[ovs-dev] [PATCH] coding-style: Explain when to break lines before or after binary operators.

Ben Pfaff blp at ovn.org
Wed Nov 29 00:37:04 UTC 2017


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 at sysclose.org>
Suggested-at: https://mail.openvswitch.org/pipermail/ovs-dev/2017-November/341091.html
Signed-off-by: Ben Pfaff <blp at ovn.org>
---
 .../internals/contributing/coding-style.rst        | 52 +++++++++++++---------
 1 file changed, 31 insertions(+), 21 deletions(-)

diff --git a/Documentation/internals/contributing/coding-style.rst b/Documentation/internals/contributing/coding-style.rst
index 666e887b1b68..bcc169af305b 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 before 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 after 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().
 
-- 
2.10.2



More information about the dev mailing list