diff --git a/HACKING b/HACKING index 5486d8edcd..e22d73c71e 100644 --- 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 03a1bee17d..deab530abc 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -11,16 +11,15 @@ early and listen to feedback.
Post patches in unified diff format. A command similar to this - should work:
+ should work:- diff -urp libvirt.orig/ libvirt.modified/ > libvirt-myfeature.patch -+ diff -urp libvirt.orig/ libvirt.modified/ > libvirt-myfeature.patch
or:
- git diff > libvirt-myfeature.patch -
Run the automated tests on your code before submitting any changes. In particular, configure with compile warnings set to -Werror:
- ./configure --enable-compile-warnings=error -+ ./configure --enable-compile-warnings=error
and run the tests:
make check make syntax-check - make -C tests valgrind -+ make -C tests valgrind
The latter test checks for memory leaks.
@@ -60,6 +57,7 @@./qemuxml2xmltest+
+ 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; + }+