docs: hacking: explain why using curly braces well is important

* docs/hacking.html.in: Use the "curly braces" section from coreutils'
HACKING, adapting for libvirt's different formatting style.
* HACKING: Sync from the above, still mostly manually.

diff --git a/HACKING b/HACKING
index 5486d8edcd672d5cf22d1033b2f17172793aa310..e22d73c71e433b5acd2c9e7c6219fce22a777fe1 100644 (file)
--- a/HACKING
+++ b/HACKING
@@ -105,6 +105,97 @@ Usually they're in macro definitions or strings, and should be converted
 anyhow.
 
 
+Curly braces
+============
+Omit the curly braces around an "if", "while", "for" etc. body only when that
+body occupies a single line. In every other case we require the braces. This
+ensures that it is trivially easy to identify a single-*statement* loop: each
+has only one *line* in its body.
+
+Omitting braces with a single-line body is fine:
+
+   while (expr) // one-line body -> omitting curly braces is ok
+       single_line_stmt ();
+
+However, the moment your loop/if/else body extends onto a second line, for
+whatever reason (even if it's just an added comment), then you should add
+braces. Otherwise, it would be too easy to insert a statement just before that
+comment (without adding braces), thinking it is already a multi-statement
+loop:
+
+   while (true) // BAD! multi-line body with no braces
+       /* comment... */
+       single_line_stmt ();
+
+Do this instead:
+
+   while (true) { // Always put braces around a multi-line body.
+       /* comment... */
+       single_line_stmt ();
+   }
+
+There is one exception: when the second body line is not at the same
+indentation level as the first body line:
+
+   if (expr)
+       die ("a diagnostic that would make this line"
+            " extend past the 80-column limit"));
+
+It is safe to omit the braces in the code above, since the further-indented
+second body line makes it obvious that this is still a single-statement body.
+
+
+To reiterate, don't do this:
+
+   if (expr)            // BAD: no braces around...
+       while (expr_2) { // ... a multi-line body
+           ...
+       }
+
+Do this, instead:
+
+   if (expr) {
+       while (expr_2) {
+           ...
+       }
+   }
+
+However, there is one exception in the other direction, when even a one-line
+block should have braces. That occurs when that one-line, brace-less block is
+an "else" block, and the corresponding "then" block *does* use braces. In that
+case, either put braces around the "else" block, or negate the "if"-condition
+and swap the bodies, putting the one-line block first and making the longer,
+multi-line block be the "else" block.
+
+   if (expr) {
+       ...
+       ...
+   }
+   else
+       x = y;    // BAD: braceless "else" with braced "then"
+
+This is preferred, especially when the multi-line body is more than a few
+lines long, because it is easier to read and grasp the semantics of an if-
+then-else block when the simpler block occurs first, rather than after the
+more involved block:
+
+   if (!expr)
+     x = y; // putting the smaller block first is more readable
+   else {
+       ...
+       ...
+   }
+
+If you'd rather not negate the condition, then at least add braces:
+
+   if (expr) {
+       ...
+       ...
+   } else {
+       x = y;
+   }
+
+
 Preprocessor
 ============
 For variadic macros, stick with C99 syntax:

diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 03a1bee17dfff234b846c1f04743744a615ae51e..deab530abc9abacd89525d4c012fcaece4963f7e 100644 (file)
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -11,16 +11,15 @@
         early and listen to feedback.</li>
 
       <li><p>Post patches in unified diff format.  A command similar to this
-          should work:</p>
+        should work:</p>
         <pre>
-  diff -urp libvirt.orig/ libvirt.modified/ &gt; libvirt-myfeature.patch
-</pre>
+  diff -urp libvirt.orig/ libvirt.modified/ &gt; libvirt-myfeature.patch</pre>
         <p>
           or:
         </p>
         <pre>
-  git diff > libvirt-myfeature.patch
-</pre></li>
+  git diff > libvirt-myfeature.patch</pre>
+      </li>
       <li>Split large changes into a series of smaller patches, self-contained
         if possible, with an explanation of each patch and an explanation of how
         the sequence of patches fits together.</li>
@@ -29,16 +28,14 @@
       <li><p>Run the automated tests on your code before submitting any changes.
           In particular, configure with compile warnings set to -Werror:</p>
         <pre>
-  ./configure --enable-compile-warnings=error
-</pre>
+  ./configure --enable-compile-warnings=error</pre>
         <p>
           and run the tests:
         </p>
         <pre>
   make check
   make syntax-check
-  make -C tests valgrind
-</pre>
+  make -C tests valgrind</pre>
         <p>
           The latter test checks for memory leaks.
         </p>
@@ -60,6 +57,7 @@
        <pre>
   ./qemuxml2xmltest</pre>
 
+      </li>
       <li>Update tests and/or documentation, particularly if you are adding
         a new feature or changing the output of a program.</li>
     </ol>
@@ -124,6 +122,123 @@
     </p>
 
 
+    <h2><a name="curly_braces">Curly braces</a></h2>
+
+    <p>
+      Omit the curly braces around an "if", "while", "for" etc. body only
+      when that body occupies a single line.  In every other case we require
+      the braces.  This ensures that it is trivially easy to identify a
+      single-*statement* loop: each has only one *line* in its body.
+    </p>
+    <p>
+      Omitting braces with a single-line body is fine:
+    </p>
+
+    <pre>
+  while (expr) // one-line body -> omitting curly braces is ok
+      single_line_stmt ();</pre>
+
+    <p>
+      However, the moment your loop/if/else body extends onto a second
+      line, for whatever reason (even if it's just an added comment), then
+      you should add braces.  Otherwise, it would be too easy to insert a
+      statement just before that comment (without adding braces), thinking
+      it is already a multi-statement loop:
+    </p>
+
+    <pre>
+  while (true) // BAD! multi-line body with no braces
+      /* comment... */
+      single_line_stmt ();</pre>
+    <p>
+      Do this instead:
+    </p>
+    <pre>
+  while (true) { // Always put braces around a multi-line body.
+      /* comment... */
+      single_line_stmt ();
+  }</pre>
+    <p>
+      There is one exception: when the second body line is not at the same
+      indentation level as the first body line:
+    </p>
+    <pre>
+  if (expr)
+      die ("a diagnostic that would make this line"
+           " extend past the 80-column limit"));</pre>
+
+    <p>
+      It is safe to omit the braces in the code above, since the
+      further-indented second body line makes it obvious that this is still
+      a single-statement body.
+    </p>
+
+    <p>
+      To reiterate, don't do this:
+    </p>
+
+    <pre>
+  if (expr)            // BAD: no braces around...
+      while (expr_2) { // ... a multi-line body
+          ...
+      }</pre>
+
+    <p>
+      Do this, instead:
+    </p>
+
+    <pre>
+  if (expr) {
+      while (expr_2) {
+          ...
+      }
+  }</pre>
+
+    <p>
+      However, there is one exception in the other direction, when even a
+      one-line block should have braces.  That occurs when that one-line,
+      brace-less block is an "else" block, and the corresponding "then" block
+      *does* use braces.  In that case, either put braces around the "else"
+      block, or negate the "if"-condition and swap the bodies, putting the
+      one-line block first and making the longer, multi-line block be the
+      "else" block.
+    </p>
+
+    <pre>
+  if (expr) {
+      ...
+      ...
+  }
+  else
+      x = y;    // BAD: braceless "else" with braced "then"</pre>
+
+    <p>
+      This is preferred, especially when the multi-line body is more than a
+      few lines long, because it is easier to read and grasp the semantics of
+      an if-then-else block when the simpler block occurs first, rather than
+      after the more involved block:
+    </p>
+
+    <pre>
+  if (!expr)
+    x = y; // putting the smaller block first is more readable
+  else {
+      ...
+      ...
+  }</pre>
+
+    <p>
+      If you'd rather not negate the condition, then at least add braces:
+    </p>
+
+    <pre>
+  if (expr) {
+      ...
+      ...
+  } else {
+      x = y;
+  }</pre>
+
     <h2><a href="types">Preprocessor</a></h2>
 
     <p>