summaryrefslogtreecommitdiff
path: root/Documentation/internals
diff options
context:
space:
mode:
authorBen Pfaff <blp@ovn.org>2017-12-04 08:33:49 -0800
committerBen Pfaff <blp@ovn.org>2017-12-04 08:33:49 -0800
commit2450cdabf692fc2700d2bddedede9420815970a8 (patch)
tree7a1b16318673653a7b3e3f146ea816a826e95ba2 /Documentation/internals
parent99cf99597ffa9f09e66d646d04de8d1c7e6d52c8 (diff)
downloadopenvswitch-2450cdabf692fc2700d2bddedede9420815970a8.tar.gz
coding-style: Explain when to break lines before or after binary operators.
The coding style has never been explicit about this. This commit adds some explanation of why one position or the other might be favored in a given situation. Suggested-by: Flavio Leitner <fbl@sysclose.org> Suggested-at: https://mail.openvswitch.org/pipermail/ovs-dev/2017-November/341091.html Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Flavio Leitner <fbl@sysclose.org> Acked-by: Tiago Lam <tiagolam@gmail.com>
Diffstat (limited to 'Documentation/internals')
-rw-r--r--Documentation/internals/contributing/coding-style.rst52
1 files changed, 31 insertions, 21 deletions
diff --git a/Documentation/internals/contributing/coding-style.rst b/Documentation/internals/contributing/coding-style.rst
index 666e887b1..63a36812a 100644
--- a/Documentation/internals/contributing/coding-style.rst
+++ b/Documentation/internals/contributing/coding-style.rst
@@ -542,34 +542,44 @@ them, e.g.
: alpheus_output_control(dp, skb, fwd_save_skb(skb),
VIGR_ACTION));
-Do not parenthesize the operands of ``&&`` and ``||`` unless operator
-precedence makes it necessary, or unless the operands are themselves
-expressions that use ``&&`` and ``||``. Thus:
-
-::
-
- if (!isdigit((unsigned char)s[0])
- || !isdigit((unsigned char)s[1])
- || !isdigit((unsigned char)s[2])) {
- printf("string %s does not start with 3-digit code\n", s);
- }
-
-but
-
-::
+Parenthesize the operands of ``&&`` and ``||`` if operator precedence makes it
+necessary, or if the operands are themselves expressions that use ``&&`` and
+``||``, but not otherwise. Thus::
if (rule && (!best || rule->priority > best->priority)) {
best = rule;
}
-Do parenthesize a subexpression that must be split across more than one line,
-e.g.:
+but::
-::
+ if (!isdigit((unsigned char)s[0]) ||
+ !isdigit((unsigned char)s[1]) ||
+ !isdigit((unsigned char)s[2])) {
+ printf("string %s does not start with 3-digit code\n", s);
+ }
- *idxp = ((l1_idx << PORT_ARRAY_L1_SHIFT)
- | (l2_idx << PORT_ARRAY_L2_SHIFT)
- | (l3_idx << PORT_ARRAY_L3_SHIFT));
+Do parenthesize a subexpression that must be split across more than one line,
+e.g.::
+
+ *idxp = ((l1_idx << PORT_ARRAY_L1_SHIFT) |
+ (l2_idx << PORT_ARRAY_L2_SHIFT) |
+ (l3_idx << PORT_ARRAY_L3_SHIFT));
+
+Breaking a long line after a binary operator gives its operands a more
+consistent look, since each operand has the same horizontal position. This
+makes the end-of-line position a good choice when the operands naturally
+resemble each other, as in the previous two examples. On the other hand,
+breaking before a binary operator better draws the eye to the operator, which
+can help clarify code by making it more obvious what's happening, such as in
+the following example::
+
+ if (!ctx.freezing
+ && xbridge->has_in_band
+ && in_band_must_output_to_local_port(flow)
+ && !actions_output_to_local_port(&ctx)) {
+
+Thus, decide whether to break before or after a binary operator separately in
+each situation, based on which of these factors appear to be more important.
Try to avoid casts. Don't cast the return value of malloc().