docs: Prepare hacking.html.in to generate HACKING from it

Tweak pre tags to achieve proper indentation of their
plaintext representation. Also use more b/i/code tags.
This commit is contained in:
Matthias Bolte 2010-11-12 15:33:00 +01:00
parent 4302b3f8f0
commit d39620e367

View File

@ -5,20 +5,21 @@
<ul id="toc"></ul> <ul id="toc"></ul>
<h2><a name="patches">General tips for contributing patches</a></h2> <h2><a name="patches">General tips for contributing patches</a></h2>
<ol> <ol>
<li>Discuss any large changes on the mailing list first. Post patches <li>Discuss any large changes on the mailing list first. Post patches
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/ &gt; libvirt-myfeature.patch</pre> diff -urp libvirt.orig/ libvirt.modified/ &gt; libvirt-myfeature.patch
</pre>
<p> <p>
or: or:
</p> </p>
<pre> <pre>
git diff > libvirt-myfeature.patch</pre> git diff > libvirt-myfeature.patch
</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
@ -27,35 +28,39 @@
only follow GIT and don't care much about released versions.</li> only follow GIT and don't care much about released versions.</li>
<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</pre> ./configure --enable-compile-warnings=error
</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</pre> make -C tests valgrind
</pre>
<p> <p>
The latter test checks for memory leaks. The latter test checks for memory leaks.
</p> </p>
<p> <p>
If you encounter any failing tests, the VIR_TEST_DEBUG If you encounter any failing tests, the VIR_TEST_DEBUG
environment variable may provide extra information to debug environment variable may provide extra information to debug
the failures. Larger values of VIR_TEST_DEBUG may provide the failures. Larger values of VIR_TEST_DEBUG may provide
larger amounts of information: larger amounts of information:
</p> </p>
<pre> <pre>
VIR_TEST_DEBUG=1 make check (or) VIR_TEST_DEBUG=1 make check (or)
VIR_TEST_DEBUG=2 make check</pre> VIR_TEST_DEBUG=2 make check
<p> </pre>
Also, individual tests can be run from inside the 'tests/' <p>
directory, like: Also, individual tests can be run from inside the <code>tests/</code>
</p> directory, like:
<pre> </p>
./qemuxml2xmltest</pre> <pre>
./qemuxml2xmltest
</pre>
</li> </li>
<li>Update tests and/or documentation, particularly if you are adding <li>Update tests and/or documentation, particularly if you are adding
@ -82,7 +87,7 @@
If you use Emacs, add the following to one of one of your start-up files If you use Emacs, add the following to one of one of your start-up files
(e.g., ~/.emacs), to help ensure that you get indentation right: (e.g., ~/.emacs), to help ensure that you get indentation right:
</p> </p>
<pre> <pre>
;;; When editing C sources in libvirt, use this style. ;;; When editing C sources in libvirt, use this style.
(defun libvirt-c-mode () (defun libvirt-c-mode ()
"C mode with adjusted defaults for use with libvirt." "C mode with adjusted defaults for use with libvirt."
@ -105,7 +110,7 @@
around operators and keywords: around operators and keywords:
</p> </p>
<pre> <pre>
indent-libvirt() indent-libvirt()
{ {
indent -bad -bap -bbb -bli4 -br -ce -brs -cs -i4 -l75 -lc75 \ indent -bad -bap -bbb -bli4 -br -ce -brs -cs -i4 -l75 -lc75 \
@ -116,7 +121,7 @@
<p> <p>
Note that sometimes you'll have to post-process that output further, by Note that sometimes you'll have to post-process that output further, by
piping it through "expand -i", since some leading TABs can get through. piping it through <code>expand -i</code>, since some leading TABs can get through.
Usually they're in macro definitions or strings, and should be converted Usually they're in macro definitions or strings, and should be converted
anyhow. anyhow.
</p> </p>
@ -125,18 +130,20 @@
<h2><a name="curly_braces">Curly braces</a></h2> <h2><a name="curly_braces">Curly braces</a></h2>
<p> <p>
Omit the curly braces around an "if", "while", "for" etc. body only Omit the curly braces around an <code>if</code>, <code>while</code>,
<code>for</code> etc. body only
when that body occupies a single line. In every other case we require 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 the braces. This ensures that it is trivially easy to identify a
single-*statement* loop: each has only one *line* in its body. single-<i>statement</i> loop: each has only one <i>line</i> in its body.
</p> </p>
<p> <p>
Omitting braces with a single-line body is fine: Omitting braces with a single-line body is fine:
</p> </p>
<pre> <pre>
while (expr) // one-line body -> omitting curly braces is ok while (expr) // one-line body -> omitting curly braces is ok
single_line_stmt ();</pre> single_line_stmt();
</pre>
<p> <p>
However, the moment your loop/if/else body extends onto a second However, the moment your loop/if/else body extends onto a second
@ -146,26 +153,29 @@
it is already a multi-statement loop: it is already a multi-statement loop:
</p> </p>
<pre> <pre>
while (true) // BAD! multi-line body with no braces while (true) // BAD! multi-line body with no braces
/* comment... */ /* comment... */
single_line_stmt ();</pre> single_line_stmt();
</pre>
<p> <p>
Do this instead: Do this instead:
</p> </p>
<pre> <pre>
while (true) { // Always put braces around a multi-line body. while (true) { // Always put braces around a multi-line body.
/* comment... */ /* comment... */
single_line_stmt (); single_line_stmt();
}</pre> }
</pre>
<p> <p>
There is one exception: when the second body line is not at the same There is one exception: when the second body line is not at the same
indentation level as the first body line: indentation level as the first body line:
</p> </p>
<pre> <pre>
if (expr) if (expr)
die ("a diagnostic that would make this line" die("a diagnostic that would make this line"
" extend past the 80-column limit"));</pre> " extend past the 80-column limit"));
</pre>
<p> <p>
It is safe to omit the braces in the code above, since the It is safe to omit the braces in the code above, since the
@ -177,40 +187,44 @@
To reiterate, don't do this: To reiterate, don't do this:
</p> </p>
<pre> <pre>
if (expr) // BAD: no braces around... if (expr) // BAD: no braces around...
while (expr_2) { // ... a multi-line body while (expr_2) { // ... a multi-line body
... ...
}</pre> }
</pre>
<p> <p>
Do this, instead: Do this, instead:
</p> </p>
<pre> <pre>
if (expr) { if (expr) {
while (expr_2) { while (expr_2) {
... ...
} }
}</pre> }
</pre>
<p> <p>
However, there is one exception in the other direction, when even a However, there is one exception in the other direction, when even a
one-line block should have braces. That occurs when that one-line, one-line block should have braces. That occurs when that one-line,
brace-less block is an "else" block, and the corresponding "then" block brace-less block is an <code>else</code> block, and the corresponding
*does* use braces. In that case, either put braces around the "else" <code>then</code> block <b>does</b> use braces. In that case, either
block, or negate the "if"-condition and swap the bodies, putting the put braces around the <code>else</code> block, or negate the
<code>if</code>-condition and swap the bodies, putting the
one-line block first and making the longer, multi-line block be the one-line block first and making the longer, multi-line block be the
"else" block. <code>else</code> block.
</p> </p>
<pre> <pre>
if (expr) { if (expr) {
... ...
... ...
} }
else else
x = y; // BAD: braceless "else" with braced "then"</pre> x = y; // BAD: braceless "else" with braced "then"
</pre>
<p> <p>
This is preferred, especially when the multi-line body is more than a This is preferred, especially when the multi-line body is more than a
@ -219,43 +233,45 @@
after the more involved block: after the more involved block:
</p> </p>
<pre> <pre>
if (!expr) if (!expr)
x = y; // putting the smaller block first is more readable x = y; // putting the smaller block first is more readable
else { else {
... ...
... ...
}</pre> }
</pre>
<p> <p>
If you'd rather not negate the condition, then at least add braces: If you'd rather not negate the condition, then at least add braces:
</p> </p>
<pre> <pre>
if (expr) { if (expr) {
... ...
... ...
} else { } else {
x = y; x = y;
}</pre> }
</pre>
<h2><a href="types">Preprocessor</a></h2> <h2><a href="types">Preprocessor</a></h2>
<p> <p>
For variadic macros, stick with C99 syntax: For variadic macros, stick with C99 syntax:
</p> </p>
<pre> <pre>
#define vshPrint(_ctl, ...) fprintf(stdout, __VA_ARGS__) #define vshPrint(_ctl, ...) fprintf(stdout, __VA_ARGS__)
</pre> </pre>
<p>Use parenthesis when checking if a macro is defined, and use <p>Use parenthesis when checking if a macro is defined, and use
indentation to track nesting: indentation to track nesting:
</p> </p>
<pre> <pre>
#if defined(HAVE_POSIX_FALLOCATE) &amp;&amp; !defined(HAVE_FALLOCATE) #if defined(HAVE_POSIX_FALLOCATE) &amp;&amp; !defined(HAVE_FALLOCATE)
# define fallocate(a,ignored,b,c) posix_fallocate(a,b,c) # define fallocate(a,ignored,b,c) posix_fallocate(a,b,c)
#endif #endif
</pre> </pre>
<h2><a href="types">C types</a></h2> <h2><a href="types">C types</a></h2>
@ -266,45 +282,51 @@
<h3>Scalars</h3> <h3>Scalars</h3>
<ul> <ul>
<li>If you're using "int" or "long", odds are good that there's a better type.</li> <li>If you're using <code>int</code> or <code>long</code>, odds are
good that there's a better type.</li>
<li>If a variable is counting something, be sure to declare it with an <li>If a variable is counting something, be sure to declare it with an
unsigned type.</li> unsigned type.</li>
<li>If it's memory-size-related, use size_t (use ssize_t only if required).</li> <li>If it's memory-size-related, use <code>size_t</code> (use
<li>If it's file-size related, use uintmax_t, or maybe off_t.</li> <code>ssize_t</code> only if required).</li>
<li>If it's file-offset related (i.e., signed), use off_t.</li> <li>If it's file-size related, use uintmax_t, or maybe <code>off_t</code>.</li>
<li>If it's just counting small numbers use "unsigned int"; <li>If it's file-offset related (i.e., signed), use <code>off_t</code>.</li>
<li>If it's just counting small numbers use <code>unsigned int</code>;
(on all but oddball embedded systems, you can assume that that (on all but oddball embedded systems, you can assume that that
type is at least four bytes wide).</li> type is at least four bytes wide).</li>
<li>If a variable has boolean semantics, give it the "bool" type <li>If a variable has boolean semantics, give it the <code>bool</code> type
and use the corresponding "true" and "false" macros. It's ok and use the corresponding <code>true</code> and <code>false</code> macros.
to include &lt;stdbool.h&gt;, since libvirt's use of gnulib ensures It's ok to include &lt;stdbool.h&gt;, since libvirt's use of gnulib ensures
that it exists and is usable.</li> that it exists and is usable.</li>
<li>In the unusual event that you require a specific width, use a <li>In the unusual event that you require a specific width, use a
standard type like int32_t, uint32_t, uint64_t, etc.</li> standard type like <code>int32_t</code>, <code>uint32_t</code>,
<li>While using "bool" is good for readability, it comes with minor caveats: <code>uint64_t</code>, etc.</li>
<li>While using <code>bool</code> is good for readability, it comes with
minor caveats:
<ul> <ul>
<li>Don't use "bool" in places where the type size must be constant across <li>Don't use <code>bool</code> in places where the type size must be constant across
all systems, like public interfaces and on-the-wire protocols. Note all systems, like public interfaces and on-the-wire protocols. Note
that it would be possible (albeit wasteful) to use "bool" in libvirt's that it would be possible (albeit wasteful) to use <code>bool</code> in libvirt's
logical wire protocol, since XDR maps that to its lower-level bool_t logical wire protocol, since XDR maps that to its lower-level <code>bool_t</code>
type, which *is* fixed-size.</li> type, which <b>is</b> fixed-size.</li>
<li>Don't compare a bool variable against the literal, "true", <li>Don't compare a bool variable against the literal, <code>true</code>,
since a value with a logical non-false value need not be "1". since a value with a logical non-false value need not be <code>1</code>.
I.e., don't write "if (seen == true) ...". Rather, write "if (seen)...".</li> I.e., don't write <code>if (seen == true) ...</code>. Rather,
write <code>if (seen)...</code>.</li>
</ul> </ul>
</li> </li>
</ul> </ul>
<p> <p>
Of course, take all of the above with a grain of salt. If you're about Of course, take all of the above with a grain of salt. If you're about
to use some system interface that requires a type like size_t, pid_t or to use some system interface that requires a type like <code>size_t</code>,
off_t, use matching types for any corresponding variables. <code>pid_t</code> or <code>off_t</code>, use matching types for any
corresponding variables.
</p> </p>
<p> <p>
Also, if you try to use e.g., "unsigned int" as a type, and that Also, if you try to use e.g., <code>unsigned int</code> as a type, and that
conflicts with the signedness of a related variable, sometimes conflicts with the signedness of a related variable, sometimes
it's best just to use the *wrong* type, if "pulling the thread" it's best just to use the <b>wrong</b> type, if <i>pulling the thread</i>
and fixing all related variables would be too invasive. and fixing all related variables would be too invasive.
</p> </p>
@ -317,9 +339,9 @@
<h3>Pointers</h3> <h3>Pointers</h3>
<p> <p>
Ensure that all of your pointers are "const-correct". Ensure that all of your pointers are <i>const-correct</i>.
Unless a pointer is used to modify the pointed-to storage, Unless a pointer is used to modify the pointed-to storage,
give it the "const" attribute. That way, the reader knows give it the <code>const</code> attribute. That way, the reader knows
up-front that this is a read-only pointer. Perhaps more up-front that this is a read-only pointer. Perhaps more
importantly, if we're diligent about this, when you see a non-const importantly, if we're diligent about this, when you see a non-const
pointer, you're guaranteed that it is used to modify the storage pointer, you're guaranteed that it is used to modify the storage
@ -336,57 +358,57 @@
</p> </p>
<ul> <ul>
<li><p>eg to allocate a single object:</p> <li><p>e.g. to allocate a single object:</p>
<pre> <pre>
virDomainPtr domain; virDomainPtr domain;
if (VIR_ALLOC(domain) &lt; 0) { if (VIR_ALLOC(domain) &lt; 0) {
virReportOOMError(); virReportOOMError();
return NULL; return NULL;
} }
</pre></li> </pre>
</li>
<li><p>eg to allocate an array of objects</p>
<li><p>e.g. to allocate an array of objects</p>
<pre> <pre>
virDomainPtr domains; virDomainPtr domains;
int ndomains = 10; int ndomains = 10;
if (VIR_ALLOC_N(domains, ndomains) &lt; 0) { if (VIR_ALLOC_N(domains, ndomains) &lt; 0) {
virReportOOMError(); virReportOOMError();
return NULL; return NULL;
} }
</pre></li> </pre>
</li>
<li><p>eg to allocate an array of object pointers</p>
<li><p>e.g. to allocate an array of object pointers</p>
<pre> <pre>
virDomainPtr *domains; virDomainPtr *domains;
int ndomains = 10; int ndomains = 10;
if (VIR_ALLOC_N(domains, ndomains) &lt; 0) { if (VIR_ALLOC_N(domains, ndomains) &lt; 0) {
virReportOOMError(); virReportOOMError();
return NULL; return NULL;
} }
</pre></li> </pre>
</li>
<li><p>eg to re-allocate the array of domains to be longer</p>
<li><p>e.g. to re-allocate the array of domains to be longer</p>
<pre> <pre>
ndomains = 20 ndomains = 20
if (VIR_REALLOC_N(domains, ndomains) &lt; 0) { if (VIR_REALLOC_N(domains, ndomains) &lt; 0) {
virReportOOMError(); virReportOOMError();
return NULL; return NULL;
} }
</pre></li> </pre>
</li>
<li><p>eg to free the domain</p>
<li><p>e.g. to free the domain</p>
<pre> <pre>
VIR_FREE(domain); VIR_FREE(domain);
</pre></li> </pre>
</li>
</ul> </ul>
<h2><a name="file_handling">File handling</a></h2> <h2><a name="file_handling">File handling</a></h2>
@ -398,20 +420,21 @@
</p> </p>
<ul> <ul>
<li><p>eg close a file descriptor</p> <li><p>e.g. close a file descriptor</p>
<pre> <pre>
if (VIR_CLOSE(fd) &lt; 0) { if (VIR_CLOSE(fd) &lt; 0) {
virReportSystemError(errno, _("failed to close file")); virReportSystemError(errno, _("failed to close file"));
} }
</pre></li> </pre>
</li>
<li><p>eg close a file descriptor in an error path, without losing <li><p>eg close a file descriptor in an error path, without losing
the previous errno value</p> the previous <code>errno</code> value</p>
<pre> <pre>
VIR_FORCE_CLOSE(fd); VIR_FORCE_CLOSE(fd);
</pre></li> </pre>
</li>
</ul> </ul>
<h2><a name="string_comparision">String comparisons</a></h2> <h2><a name="string_comparision">String comparisons</a></h2>
@ -423,39 +446,36 @@
<ul> <ul>
<li><p>For strict equality:</p> <li><p>For strict equality:</p>
<pre> <pre>
STREQ(a,b) STREQ(a,b)
STRNEQ(a,b) STRNEQ(a,b)
</pre> </pre>
</li> </li>
<li><p>For case insensitive equality:</p> <li><p>For case insensitive equality:</p>
<pre> <pre>
STRCASEEQ(a,b) STRCASEEQ(a,b)
STRCASENEQ(a,b) STRCASENEQ(a,b)
</pre> </pre>
</li> </li>
<li><p>For strict equality of a substring:</p> <li><p>For strict equality of a substring:</p>
<pre>
<pre> STREQLEN(a,b,n)
STREQLEN(a,b,n) STRNEQLEN(a,b,n)
STRNEQLEN(a,b,n)
</pre> </pre>
</li> </li>
<li><p>For case insensitive equality of a substring:</p> <li><p>For case insensitive equality of a substring:</p>
<pre>
<pre> STRCASEEQLEN(a,b,n)
STRCASEEQLEN(a,b,n) STRCASENEQLEN(a,b,n)
STRCASENEQLEN(a,b,n)
</pre> </pre>
</li> </li>
<li><p>For strict equality of a prefix:</p> <li><p>For strict equality of a prefix:</p>
<pre>
<pre> STRPREFIX(a,b)
STRPREFIX(a,b)
</pre> </pre>
</li> </li>
</ul> </ul>
@ -469,7 +489,10 @@
it extremely dangerous to use. Instead, use one of the it extremely dangerous to use. Instead, use one of the
functionally equivalent functions: functionally equivalent functions:
</p> </p>
<pre>virStrncpy(char *dest, const char *src, size_t n, size_t destbytes)</pre>
<pre>
virStrncpy(char *dest, const char *src, size_t n, size_t destbytes)
</pre>
<p> <p>
The first three arguments have the same meaning as for strncpy; The first three arguments have the same meaning as for strncpy;
namely the destination, source, and number of bytes to copy, namely the destination, source, and number of bytes to copy,
@ -481,8 +504,9 @@
trailing \0 is appended. trailing \0 is appended.
</p> </p>
<pre>virStrcpy(char *dest, const char *src, size_t destbytes)</pre> <pre>
virStrcpy(char *dest, const char *src, size_t destbytes)
</pre>
<p> <p>
Use this variant if you know you want to copy the entire src Use this variant if you know you want to copy the entire src
string into dest. Note that this is a macro, so arguments could string into dest. Note that this is a macro, so arguments could
@ -490,11 +514,12 @@
virStrncpy(dest, src, strlen(src), destbytes) virStrncpy(dest, src, strlen(src), destbytes)
</p> </p>
<pre>virStrcpyStatic(char *dest, const char *src)</pre> <pre>
virStrcpyStatic(char *dest, const char *src)
</pre>
<p> <p>
Use this variant if you know you want to copy the entire src Use this variant if you know you want to copy the entire src
string into dest *and* you know that your destination string is string into dest <b>and</b> you know that your destination string is
a static string (i.e. that sizeof(dest) returns something a static string (i.e. that sizeof(dest) returns something
meaningful). Note that this is a macro, so arguments could be meaningful). Note that this is a macro, so arguments could be
evaluated more than once. This is equivalent to evaluated more than once. This is equivalent to
@ -511,9 +536,10 @@
<p>eg typical usage is as follows:</p> <p>eg typical usage is as follows:</p>
<pre> <pre>
char * char *
somefunction(...) { somefunction(...)
{
virBuffer buf = VIR_BUFFER_INITIALIZER; virBuffer buf = VIR_BUFFER_INITIALIZER;
... ...
@ -545,7 +571,7 @@
*.c source files: *.c source files:
</p> </p>
<pre> <pre>
/* /*
* Copyright notice * Copyright notice
* .... * ....
@ -561,7 +587,7 @@
#include &lt;limits.h&gt; #include &lt;limits.h&gt;
#if HAVE_NUMACTL Some system includes aren't supported #if HAVE_NUMACTL Some system includes aren't supported
# include &lt;numa.h&gt; everywhere so need these #if guards. # include &lt;numa.h&gt; everywhere so need these #if guards.
#endif #endif
#include "internal.h" Include this first, after system includes. #include "internal.h" Include this first, after system includes.
@ -569,13 +595,14 @@
#include "util.h" Any libvirt internal header files. #include "util.h" Any libvirt internal header files.
#include "buf.h" #include "buf.h"
static myInternalFunc () The actual code. static int
myInternalFunc() The actual code.
{ {
... ...
</pre> </pre>
<p> <p>
Of particular note: *DO NOT* include libvirt/libvirt.h or Of particular note: <b>Do not</b> include libvirt/libvirt.h or
libvirt/virterror.h. It is included by "internal.h" already and there libvirt/virterror.h. It is included by "internal.h" already and there
are some special reasons why you cannot include these files are some special reasons why you cannot include these files
explicitly. explicitly.
@ -591,9 +618,9 @@
the one for virAsprintf, in util.h: the one for virAsprintf, in util.h:
</p> </p>
<pre> <pre>
int virAsprintf(char **strp, const char *fmt, ...) int virAsprintf(char **strp, const char *fmt, ...)
ATTRIBUTE_FORMAT(printf, 2, 3); ATTRIBUTE_FORMAT(printf, 2, 3);
</pre> </pre>
<p> <p>
@ -654,7 +681,7 @@
Although libvirt does not encourage the Linux kernel wind/unwind Although libvirt does not encourage the Linux kernel wind/unwind
style of multiple labels, there's a good general discussion of style of multiple labels, there's a good general discussion of
the issue archived at the issue archived at
<a href=http://kerneltrap.org/node/553/2131>KernelTrap</a> <a href="http://kerneltrap.org/node/553/2131">KernelTrap</a>
</p> </p>
<p> <p>
@ -662,11 +689,12 @@
makes sense: makes sense:
</p> </p>
<pre> <pre>
error: A path only taken upon return with an error code error: A path only taken upon return with an error code
cleanup: A path taken upon return with success code + optional error cleanup: A path taken upon return with success code + optional error
no_memory: A path only taken upon return with an OOM error code no_memory: A path only taken upon return with an OOM error code
retry: If needing to jump upwards (eg retry on EINTR)</pre> retry: If needing to jump upwards (eg retry on EINTR)
</pre>
@ -691,7 +719,7 @@
configure with configure with
</p> </p>
<pre> <pre>
--enable-compile-warnings=error --enable-compile-warnings=error
</pre> </pre>
<p> <p>
which adds -Werror to compile flags, so no warnings get missed which adds -Werror to compile flags, so no warnings get missed