mirror of
https://gitlab.com/libvirt/libvirt.git
synced 2025-02-08 12:41:29 +00:00
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.
This commit is contained in:
parent
eca81e08be
commit
44258473b8
91
HACKING
91
HACKING
@ -105,6 +105,97 @@ Usually they're in macro definitions or strings, and should be converted
|
|||||||
anyhow.
|
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
|
Preprocessor
|
||||||
============
|
============
|
||||||
For variadic macros, stick with C99 syntax:
|
For variadic macros, stick with C99 syntax:
|
||||||
|
@ -11,16 +11,15 @@
|
|||||||
early and listen to feedback.</li>
|
early and listen to feedback.</li>
|
||||||
|
|
||||||
<li><p>Post patches in unified diff format. A command similar to this
|
<li><p>Post patches in unified diff format. A command similar to this
|
||||||
should work:</p>
|
should work:</p>
|
||||||
<pre>
|
<pre>
|
||||||
diff -urp libvirt.orig/ libvirt.modified/ > libvirt-myfeature.patch
|
diff -urp libvirt.orig/ libvirt.modified/ > libvirt-myfeature.patch</pre>
|
||||||
</pre>
|
|
||||||
<p>
|
<p>
|
||||||
or:
|
or:
|
||||||
</p>
|
</p>
|
||||||
<pre>
|
<pre>
|
||||||
git diff > libvirt-myfeature.patch
|
git diff > libvirt-myfeature.patch</pre>
|
||||||
</pre></li>
|
</li>
|
||||||
<li>Split large changes into a series of smaller patches, self-contained
|
<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
|
if possible, with an explanation of each patch and an explanation of how
|
||||||
the sequence of patches fits together.</li>
|
the sequence of patches fits together.</li>
|
||||||
@ -29,16 +28,14 @@
|
|||||||
<li><p>Run the automated tests on your code before submitting any changes.
|
<li><p>Run the automated tests on your code before submitting any changes.
|
||||||
In particular, configure with compile warnings set to -Werror:</p>
|
In particular, configure with compile warnings set to -Werror:</p>
|
||||||
<pre>
|
<pre>
|
||||||
./configure --enable-compile-warnings=error
|
./configure --enable-compile-warnings=error</pre>
|
||||||
</pre>
|
|
||||||
<p>
|
<p>
|
||||||
and run the tests:
|
and run the tests:
|
||||||
</p>
|
</p>
|
||||||
<pre>
|
<pre>
|
||||||
make check
|
make check
|
||||||
make syntax-check
|
make syntax-check
|
||||||
make -C tests valgrind
|
make -C tests valgrind</pre>
|
||||||
</pre>
|
|
||||||
<p>
|
<p>
|
||||||
The latter test checks for memory leaks.
|
The latter test checks for memory leaks.
|
||||||
</p>
|
</p>
|
||||||
@ -60,6 +57,7 @@
|
|||||||
<pre>
|
<pre>
|
||||||
./qemuxml2xmltest</pre>
|
./qemuxml2xmltest</pre>
|
||||||
|
|
||||||
|
</li>
|
||||||
<li>Update tests and/or documentation, particularly if you are adding
|
<li>Update tests and/or documentation, particularly if you are adding
|
||||||
a new feature or changing the output of a program.</li>
|
a new feature or changing the output of a program.</li>
|
||||||
</ol>
|
</ol>
|
||||||
@ -124,6 +122,123 @@
|
|||||||
</p>
|
</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>
|
<h2><a href="types">Preprocessor</a></h2>
|
||||||
|
|
||||||
<p>
|
<p>
|
||||||
|
Loading…
x
Reference in New Issue
Block a user