External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-11-08 14:29:56 +00:00
Code Issues Packages Projects Releases Wiki Activity
048781fe71
libvirt/docs/hacking.html.in

434 lines
13 KiB
HTML
Raw Normal View History

Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
<html>
<body>
<h1>Contributor guidelines</h1>
<ul id="toc"></ul>
<h2><a name="patches">General tips for contributing patches</a></h2>
<ol>
<li>Discuss any large changes on the mailing list first. Post patches
Cleanup whitespace in docs This patch is the result of running the following command in the docs directory: sed -i 's/\t/ /g; s/\s*$//' *.html.in * docs/*.html.in:convert tabs into 8 spaces and remove trailing whitespace
2009-11-06 15:04:19 +00:00
early and listen to feedback.</li>
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
<li><p>Post patches in unified diff format. A command similar to this
Cleanup whitespace in docs This patch is the result of running the following command in the docs directory: sed -i 's/\t/ /g; s/\s*$//' *.html.in * docs/*.html.in:convert tabs into 8 spaces and remove trailing whitespace
2009-11-06 15:04:19 +00:00
should work:</p>
<pre>
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
diff -urp libvirt.orig/ libvirt.modified/ &gt; libvirt-myfeature.patch
</pre>
Cleanup whitespace in docs This patch is the result of running the following command in the docs directory: sed -i 's/\t/ /g; s/\s*$//' *.html.in * docs/*.html.in:convert tabs into 8 spaces and remove trailing whitespace
2009-11-06 15:04:19 +00:00
<p>
or:
</p>
<pre>
Replace old CVS references with GIT
2010-01-08 00:40:38 +00:00
git diff > libvirt-myfeature.patch
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
</pre></li>
<li>Split large changes into a series of smaller patches, self-contained
Cleanup whitespace in docs This patch is the result of running the following command in the docs directory: sed -i 's/\t/ /g; s/\s*$//' *.html.in * docs/*.html.in:convert tabs into 8 spaces and remove trailing whitespace
2009-11-06 15:04:19 +00:00
if possible, with an explanation of each patch and an explanation of how
the sequence of patches fits together.</li>
Replace old CVS references with GIT
2010-01-08 00:40:38 +00:00
<li>Make sure your patches apply against libvirt GIT. Developers
only follow GIT and don't care much about released versions.</li>
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
<li><p>Run the automated tests on your code before submitting any changes.
Cleanup whitespace in docs This patch is the result of running the following command in the docs directory: sed -i 's/\t/ /g; s/\s*$//' *.html.in * docs/*.html.in:convert tabs into 8 spaces and remove trailing whitespace
2009-11-06 15:04:19 +00:00
In particular, configure with compile warnings set to -Werror:</p>
<pre>
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
./configure --enable-compile-warnings=error
</pre>
Cleanup whitespace in docs This patch is the result of running the following command in the docs directory: sed -i 's/\t/ /g; s/\s*$//' *.html.in * docs/*.html.in:convert tabs into 8 spaces and remove trailing whitespace
2009-11-06 15:04:19 +00:00
<p>
and run the tests:
</p>
<pre>
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
make check
make syntax-check
make -C tests valgrind
</pre>
Cleanup whitespace in docs This patch is the result of running the following command in the docs directory: sed -i 's/\t/ /g; s/\s*$//' *.html.in * docs/*.html.in:convert tabs into 8 spaces and remove trailing whitespace
2009-11-06 15:04:19 +00:00
<p>
The latter test checks for memory leaks.
</p>
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
<li>Update tests and/or documentation, particularly if you are adding
Cleanup whitespace in docs This patch is the result of running the following command in the docs directory: sed -i 's/\t/ /g; s/\s*$//' *.html.in * docs/*.html.in:convert tabs into 8 spaces and remove trailing whitespace
2009-11-06 15:04:19 +00:00
a new feature or changing the output of a program.</li>
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
</ol>
<p>
There is more on this subject, including lots of links to background
reading on the subject, on
<a href="http://et.redhat.com/~rjones/how-to-supply-code-to-open-source-projects/">
Cleanup whitespace in docs This patch is the result of running the following command in the docs directory: sed -i 's/\t/ /g; s/\s*$//' *.html.in * docs/*.html.in:convert tabs into 8 spaces and remove trailing whitespace
2009-11-06 15:04:19 +00:00
Richard Jones' guide to working with open source projects</a>
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
</p>
<h2><a name="indent">Code indentation</a></h2>
<p>
Libvirt's C source code generally adheres to some basic code-formatting
conventions. The existing code base is not totally consistent on this
front, but we do prefer that contributed code be formatted similarly.
In short, use spaces-not-TABs for indentation, use 4 spaces for each
indentation level, and other than that, follow the K&amp;R style.
</p>
<p>
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:
</p>
<pre>
;;; When editing C sources in libvirt, use this style.
(defun libvirt-c-mode ()
"C mode with adjusted defaults for use with libvirt."
(interactive)
(c-set-style "K&amp;R")
(setq indent-tabs-mode nil) ; indent using spaces, not TABs
(setq c-indent-level 4)
(setq c-basic-offset 4))
(add-hook 'c-mode-hook
Cleanup whitespace in docs This patch is the result of running the following command in the docs directory: sed -i 's/\t/ /g; s/\s*$//' *.html.in * docs/*.html.in:convert tabs into 8 spaces and remove trailing whitespace
2009-11-06 15:04:19 +00:00
'(lambda () (if (string-match "/libvirt" (buffer-file-name))
(libvirt-c-mode))))
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
</pre>
<h2><a name="formatting">Code formatting (especially for new code)</a></h2>
<p>
With new code, we can be even more strict.
Please apply the following function (using GNU indent) to any new code.
Note that this also gives you an idea of the type of spacing we prefer
around operators and keywords:
</p>
<pre>
indent-libvirt()
{
indent -bad -bap -bbb -bli4 -br -ce -brs -cs -i4 -l75 -lc75 \
-sbi4 -psl -saf -sai -saw -sbi4 -ss -sc -cdw -cli4 -npcs -nbc \
--no-tabs "$@"
}
</pre>
<p>
Note that sometimes you'll have to postprocess that output further, by
piping it through "expand -i", since some leading TABs can get through.
Usually they're in macro definitions or strings, and should be converted
anyhow.
</p>
<h2><a href="types">C types</a></h2>
<p>
Use the right type.
</p>
<h3>Scalars</h3>
<ul>
<li>If you're using "int" or "long", odds are good that there's a better type.</li>
<li>If a variable is counting something, be sure to declare it with an
Cleanup whitespace in docs This patch is the result of running the following command in the docs directory: sed -i 's/\t/ /g; s/\s*$//' *.html.in * docs/*.html.in:convert tabs into 8 spaces and remove trailing whitespace
2009-11-06 15:04:19 +00:00
unsigned type.</li>
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
<li>If it's memory-size-related, use size_t (use ssize_t only if required).</li>
<li>If it's file-size related, use uintmax_t, or maybe off_t.</li>
<li>If it's file-offset related (i.e., signed), use off_t.</li>
<li>If it's just counting small numbers use "unsigned int";
Cleanup whitespace in docs This patch is the result of running the following command in the docs directory: sed -i 's/\t/ /g; s/\s*$//' *.html.in * docs/*.html.in:convert tabs into 8 spaces and remove trailing whitespace
2009-11-06 15:04:19 +00:00
(on all but oddball embedded systems, you can assume that that
type is at least four bytes wide).</li>
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
<li>If a variable has boolean semantics, give it the "bool" type
Cleanup whitespace in docs This patch is the result of running the following command in the docs directory: sed -i 's/\t/ /g; s/\s*$//' *.html.in * docs/*.html.in:convert tabs into 8 spaces and remove trailing whitespace
2009-11-06 15:04:19 +00:00
and use the corresponding "true" and "false" macros. It's ok
to include &lt;stdbool.h&gt;, since libvirt's use of gnulib ensures
that it exists and is usable.</li>
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
<li>In the unusual event that you require a specific width, use a
Cleanup whitespace in docs This patch is the result of running the following command in the docs directory: sed -i 's/\t/ /g; s/\s*$//' *.html.in * docs/*.html.in:convert tabs into 8 spaces and remove trailing whitespace
2009-11-06 15:04:19 +00:00
standard type like int32_t, uint32_t, uint64_t, etc.</li>
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
<li>While using "bool" is good for readability, it comes with minor caveats:
Cleanup whitespace in docs This patch is the result of running the following command in the docs directory: sed -i 's/\t/ /g; s/\s*$//' *.html.in * docs/*.html.in:convert tabs into 8 spaces and remove trailing whitespace
2009-11-06 15:04:19 +00:00
<ul>
<li>Don't use "bool" in places where the type size must be constant across
all systems, like public interfaces and on-the-wire protocols. Note
that it would be possible (albeit wasteful) to use "bool" in libvirt's
logical wire protocol, since XDR maps that to its lower-level bool_t
type, which *is* fixed-size.</li>
<li>Don't compare a bool variable against the literal, "true",
since a value with a logical non-false value need not be "1".
I.e., don't write "if (seen == true) ...". Rather, write "if (seen)...".</li>
</ul>
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
</li>
</ul>
<p>
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
off_t, use matching types for any corresponding variables.
</p>
<p>
Also, if you try to use e.g., "unsigned int" as a type, and that
conflicts with the signedness of a related variable, sometimes
it's best just to use the *wrong* type, if "pulling the thread"
and fixing all related variables would be too invasive.
</p>
<p>
Finally, while using descriptive types is important, be careful not to
go overboard. If whatever you're doing causes warnings, or requires
casts, then reconsider or ask for help.
</p>
<h3>Pointers</h3>
<p>
Ensure that all of your pointers are "const-correct".
Unless a pointer is used to modify the pointed-to storage,
give it the "const" attribute. That way, the reader knows
up-front that this is a read-only pointer. Perhaps more
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
it points to, or it is aliased to another pointer that is.
</p>
<h2><a name="memalloc">Low level memory management</a></h2>
<p>
Use of the malloc/free/realloc/calloc APIs is deprecated in the libvirt
codebase, because they encourage a number of serious coding bugs and do
not enable compile time verification of checks for NULL. Instead of these
routines, use the macros from memory.h
</p>
<ul>
<li><p>eg to allocate a single object:</p>
<pre>
virDomainPtr domain;
if (VIR_ALLOC(domain) &lt; 0) {
__virRaiseError(VIR_ERROR_NO_MEMORY)
return NULL;
}
</pre></li>
<li><p>eg to allocate an array of objects</p>
<pre>
virDomainPtr domains;
int ndomains = 10;
if (VIR_ALLOC_N(domains, ndomains) &lt; 0) {
__virRaiseError(VIR_ERROR_NO_MEMORY)
return NULL;
}
</pre></li>
<li><p>eg to allocate an array of object pointers</p>
<pre>
virDomainPtr *domains;
int ndomains = 10;
if (VIR_ALLOC_N(domains, ndomains) &lt; 0) {
__virRaiseError(VIR_ERROR_NO_MEMORY)
return NULL;
}
</pre></li>
<li><p>eg to re-allocate the array of domains to be longer</p>
<pre>
ndomains = 20
if (VIR_REALLOC_N(domains, ndomains) &lt; 0) {
__virRaiseError(VIR_ERROR_NO_MEMORY)
return NULL;
}
</pre></li>
<li><p>eg to free the domain</p>
<pre>
VIR_FREE(domain);
</pre></li>
</ul>
<h2><a name="string">String comparisons</a></h2>
<p>
Do not use the strcmp, strncmp, etc functions directly. Instead use
one of the following semantically named macros
</p>
<ul>
<li><p>For strict equality:</p>
Cleanup whitespace in docs This patch is the result of running the following command in the docs directory: sed -i 's/\t/ /g; s/\s*$//' *.html.in * docs/*.html.in:convert tabs into 8 spaces and remove trailing whitespace
2009-11-06 15:04:19 +00:00
<pre>
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
STREQ(a,b)
STRNEQ(a,b)
</pre>
</li>
<li><p>For case sensitive equality:</p>
Cleanup whitespace in docs This patch is the result of running the following command in the docs directory: sed -i 's/\t/ /g; s/\s*$//' *.html.in * docs/*.html.in:convert tabs into 8 spaces and remove trailing whitespace
2009-11-06 15:04:19 +00:00
<pre>
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
STRCASEEQ(a,b)
STRCASENEQ(a,b)
</pre>
</li>
<li><p>For strict equality of a substring:</p>
Cleanup whitespace in docs This patch is the result of running the following command in the docs directory: sed -i 's/\t/ /g; s/\s*$//' *.html.in * docs/*.html.in:convert tabs into 8 spaces and remove trailing whitespace
2009-11-06 15:04:19 +00:00
<pre>
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
STREQLEN(a,b,n)
STRNEQLEN(a,b,n)
</pre>
</li>
<li><p>For case sensitive equality of a substring:</p>
Cleanup whitespace in docs This patch is the result of running the following command in the docs directory: sed -i 's/\t/ /g; s/\s*$//' *.html.in * docs/*.html.in:convert tabs into 8 spaces and remove trailing whitespace
2009-11-06 15:04:19 +00:00
<pre>
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
STRCASEEQLEN(a,b,n)
STRCASENEQLEN(a,b,n)
</pre>
</li>
<li><p>For strict equality of a prefix:</p>
Cleanup whitespace in docs This patch is the result of running the following command in the docs directory: sed -i 's/\t/ /g; s/\s*$//' *.html.in * docs/*.html.in:convert tabs into 8 spaces and remove trailing whitespace
2009-11-06 15:04:19 +00:00
<pre>
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
STRPREFIX(a,b)
</pre>
</li>
</ul>
<h2><a name="strbuf">Variable length string buffer</a></h2>
<p>
If there is a need for complex string concatenations, avoid using
the usual sequence of malloc/strcpy/strcat/snprintf functions and
make use of the virBuffer API described in buf.h
</p>
<p>eg typical usage is as follows:</p>
<pre>
char *
somefunction(...) {
virBuffer buf = VIR_BUFFER_INITIALIZER;
...
virBufferAddLit(&amp;buf, "&lt;domain&gt;\n");
Add virBufferFreeAndReset() and replace free() Replace free(virBufferContentAndReset()) with virBufferFreeAndReset(). Update documentation and replace all remaining calls to free() with calls to VIR_FREE(). Also add missing calls to virBufferFreeAndReset() and virReportOOMError() in OOM error cases.
2009-12-09 23:00:50 +00:00
virBufferVSprint(&amp;buf, " &lt;memory&gt;%d&lt;/memory&gt;\n", memory);
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
...
virBufferAddLit(&amp;buf, "&lt;/domain&gt;\n");
Add virBufferFreeAndReset() and replace free() Replace free(virBufferContentAndReset()) with virBufferFreeAndReset(). Update documentation and replace all remaining calls to free() with calls to VIR_FREE(). Also add missing calls to virBufferFreeAndReset() and virReportOOMError() in OOM error cases.
2009-12-09 23:00:50 +00:00
...
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
if (virBufferError(&amp;buf)) {
Add virBufferFreeAndReset() and replace free() Replace free(virBufferContentAndReset()) with virBufferFreeAndReset(). Update documentation and replace all remaining calls to free() with calls to VIR_FREE(). Also add missing calls to virBufferFreeAndReset() and virReportOOMError() in OOM error cases.
2009-12-09 23:00:50 +00:00
virBufferFreeAndReset(&amp;buf);
virReportOOMError(...);
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
return NULL;
}
return virBufferContentAndReset(&amp;buf);
}
</pre>
<h2><a name="includes">Include files</a></h2>
<p>
There are now quite a large number of include files, both libvirt
internal and external, and system includes. To manage all this
complexity it's best to stick to the following general plan for all
*.c source files:
</p>
<pre>
/*
* Copyright notice
* ....
* ....
* ....
*
*/
#include &lt;config.h&gt; Must come first in every file.
#include &lt;stdio.h&gt; Any system includes you need.
#include &lt;string.h&gt;
#include &lt;limits.h&gt;
#if HAVE_NUMACTL Some system includes aren't supported
#include &lt;numa.h&gt; everywhere so need these #if defences.
#endif
#include "internal.h" Include this first, after system includes.
#include "util.h" Any libvirt internal header files.
#include "buf.h"
static myInternalFunc () The actual code.
{
...
</pre>
<p>
Of particular note: *DO NOT* include libvirt/libvirt.h or
libvirt/virterror.h. It is included by "internal.h" already and there
are some special reasons why you cannot include these files
explicitly.
</p>
<h2><a name="printf">Printf-style functions</a></h2>
<p>
Whenever you add a new printf-style function, i.e., one with a format
string argument and following "..." in its prototype, be sure to use
gcc's printf attribute directive in the prototype. For example, here's
the one for virAsprintf, in util.h:
</p>
<pre>
int virAsprintf(char **strp, const char *fmt, ...)
Cleanup whitespace in docs This patch is the result of running the following command in the docs directory: sed -i 's/\t/ /g; s/\s*$//' *.html.in * docs/*.html.in:convert tabs into 8 spaces and remove trailing whitespace
2009-11-06 15:04:19 +00:00
ATTRIBUTE_FORMAT(printf, 2, 3);
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
</pre>
<p>
This makes it so gcc's -Wformat and -Wformat-security options can do
their jobs and cross-check format strings with the number and types
of arguments.
</p>
<h2><a name="committers">Libvirt commiters guidelines</a></h2>
<p>
The AUTHORS files indicates the list of people with commit acces right
who can actually merge the patches.
</p>
<p>
The general rule for commiting patches is to make sure it has been reviewed
properly in the mailing-list first, usually if a couple of persons gave an
ACK or +1 to a patch and nobody raised an objection on the list it should
be good to go. If the patch touches a part of the code where you're not the
main maintainer or not have a very clear idea of how things work, it's better
to wait for a more authoritative feedback though. Before commiting please
also rebuild locally and run 'make check syntax-check' and make sure they
don't raise error. Try to look for warnings too for example configure with
--enable-compile-warnings=error
which adds -Werror to compile flags, so no warnings get missed
</p>
<p>
Exceptions to that 'review and approval on the list first' is fixing failures
to build:
</p>
<ul>
<li>if a recently commited patch breaks compilation on a platform
Cleanup whitespace in docs This patch is the result of running the following command in the docs directory: sed -i 's/\t/ /g; s/\s*$//' *.html.in * docs/*.html.in:convert tabs into 8 spaces and remove trailing whitespace
2009-11-06 15:04:19 +00:00
or for a given driver then it's fine to commit a minimal fix
directly without getting the review feedback first</li>
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
<li>if make check or make syntax-chek breaks, if there is
Cleanup whitespace in docs This patch is the result of running the following command in the docs directory: sed -i 's/\t/ /g; s/\s*$//' *.html.in * docs/*.html.in:convert tabs into 8 spaces and remove trailing whitespace
2009-11-06 15:04:19 +00:00
an obvious fix, it's fine to commit immediately.
The patch should still be sent to the list (or tell what the fix was if
trivial) and 'make check syntax-check' should pass too before commiting
anything</li>
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
<li>
Cleanup whitespace in docs This patch is the result of running the following command in the docs directory: sed -i 's/\t/ /g; s/\s*$//' *.html.in * docs/*.html.in:convert tabs into 8 spaces and remove trailing whitespace
2009-11-06 15:04:19 +00:00
fixes for documentation and code comments can be managed
in the same way, but still make sure they get reviewed if non-trivial.
Add HACKING doc to the website
2009-06-29 11:09:17 +00:00
</li>
</ul>
</body>
</html>
Reference in New Issue Copy Permalink