mirror of
https://gitlab.com/libvirt/libvirt.git
synced 2024-12-23 14:15:28 +00:00
490 lines
19 KiB
HTML
490 lines
19 KiB
HTML
|
<?xml version="1.0" encoding="ISO-8859-1"?>
|
||
|
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
|
||
|
<html xmlns="http://www.w3.org/1999/xhtml">
|
||
|
<!--
|
||
|
This file is autogenerated from hacking.html.in
|
||
|
Do not edit this file. Changes will be lost.
|
||
|
-->
|
||
|
<head>
|
||
|
<meta http-equiv="Content-Type" content="text/html; charset=ISO-8859-1" />
|
||
|
<link rel="stylesheet" type="text/css" href="main.css" />
|
||
|
<link rel="SHORTCUT ICON" href="32favicon.png" />
|
||
|
<title>libvirt: Contributor guidelines</title>
|
||
|
<meta name="description" content="libvirt, virtualization, virtualization API" />
|
||
|
</head>
|
||
|
<body>
|
||
|
<div id="header">
|
||
|
<div id="headerLogo"></div>
|
||
|
<div id="headerSearch">
|
||
|
<form action="search.php" enctype="application/x-www-form-urlencoded" method="get"><div>
|
||
|
<input id="query" name="query" type="text" size="12" value="" />
|
||
|
<input id="submit" name="submit" type="submit" value="Search" />
|
||
|
</div></form>
|
||
|
</div>
|
||
|
</div>
|
||
|
<div id="body">
|
||
|
<div id="menu">
|
||
|
<ul class="l0"><li>
|
||
|
<div>
|
||
|
<a title="Front page of the libvirt website" class="inactive" href="index.html">Home</a>
|
||
|
</div>
|
||
|
</li><li>
|
||
|
<div>
|
||
|
<a title="Details of new features and bugs fixed in each release" class="inactive" href="news.html">News</a>
|
||
|
</div>
|
||
|
</li><li>
|
||
|
<div>
|
||
|
<a title="Get the latest source releases, binary builds and get access to the source repository" class="inactive" href="downloads.html">Downloads</a>
|
||
|
</div>
|
||
|
</li><li>
|
||
|
<div>
|
||
|
<a title="Information for users, administrators and developers" class="active" href="docs.html">Documentation</a>
|
||
|
<ul class="l1"><li>
|
||
|
<div>
|
||
|
<a title="Information about deploying and using libvirt" class="inactive" href="deployment.html">Deployment</a>
|
||
|
</div>
|
||
|
</li><li>
|
||
|
<div>
|
||
|
<a title="Overview of the logical subsystems in the libvirt API" class="inactive" href="intro.html">Architecture</a>
|
||
|
</div>
|
||
|
</li><li>
|
||
|
<div>
|
||
|
<a title="Description of the XML formats used in libvirt" class="inactive" href="format.html">XML format</a>
|
||
|
</div>
|
||
|
</li><li>
|
||
|
<div>
|
||
|
<a title="Hypervisor specific driver information" class="inactive" href="drivers.html">Drivers</a>
|
||
|
</div>
|
||
|
</li><li>
|
||
|
<div>
|
||
|
<a title="Reference manual for the C public API" class="inactive" href="html/index.html">API reference</a>
|
||
|
</div>
|
||
|
</li><li>
|
||
|
<div>
|
||
|
<a title="Bindings of the libvirt API for other languages" class="inactive" href="bindings.html">Language bindings</a>
|
||
|
</div>
|
||
|
</li><li>
|
||
|
<div>
|
||
|
<a title="Working on the internals of libvirt API, driver and daemon code" class="active" href="internals.html">Internals</a>
|
||
|
<ul class="l2"><li>
|
||
|
<div>
|
||
|
<span class="active">Contributor guidelines</span>
|
||
|
</div>
|
||
|
</li><li>
|
||
|
<div>
|
||
|
<a title="Adding new public libvirt APIs" class="inactive" href="api_extension.html">API extensions</a>
|
||
|
</div>
|
||
|
</li></ul>
|
||
|
</div>
|
||
|
</li></ul>
|
||
|
</div>
|
||
|
</li><li>
|
||
|
<div>
|
||
|
<a title="User contributed content" class="inactive" href="http://wiki.libvirt.org">Wiki</a>
|
||
|
</div>
|
||
|
</li><li>
|
||
|
<div>
|
||
|
<a title="Frequently asked questions" class="inactive" href="FAQ.html">FAQ</a>
|
||
|
</div>
|
||
|
</li><li>
|
||
|
<div>
|
||
|
<a title="How and where to report bugs and request features" class="inactive" href="bugs.html">Bug reports</a>
|
||
|
</div>
|
||
|
</li><li>
|
||
|
<div>
|
||
|
<a title="How to contact the developers via email and IRC" class="inactive" href="contact.html">Contact</a>
|
||
|
</div>
|
||
|
</li><li>
|
||
|
<div>
|
||
|
<a title="Miscellaneous links of interest related to libvirt" class="inactive" href="relatedlinks.html">Related Links</a>
|
||
|
</div>
|
||
|
</li><li>
|
||
|
<div>
|
||
|
<a title="Overview of all content on the website" class="inactive" href="sitemap.html">Sitemap</a>
|
||
|
</div>
|
||
|
</li></ul>
|
||
|
</div>
|
||
|
<div id="content">
|
||
|
<h1>Contributor guidelines</h1>
|
||
|
<ul><li>
|
||
|
<a href="#patches">General tips for contributing patches</a>
|
||
|
</li><li>
|
||
|
<a href="#indent">Code indentation</a>
|
||
|
</li><li>
|
||
|
<a href="#formatting">Code formatting (especially for new code)</a>
|
||
|
</li><li>
|
||
|
<a href="#">C types</a>
|
||
|
</li><li>
|
||
|
<a href="#memalloc">Low level memory management</a>
|
||
|
</li><li>
|
||
|
<a href="#string">String comparisons</a>
|
||
|
</li><li>
|
||
|
<a href="#strbuf">Variable length string buffer</a>
|
||
|
</li><li>
|
||
|
<a href="#includes">Include files</a>
|
||
|
</li><li>
|
||
|
<a href="#printf">Printf-style functions</a>
|
||
|
</li><li>
|
||
|
<a href="#committers">Libvirt commiters guidelines</a>
|
||
|
</li></ul>
|
||
|
<h2>
|
||
|
<a name="patches" id="patches">General tips for contributing patches</a>
|
||
|
</h2>
|
||
|
<ol><li>Discuss any large changes on the mailing list first. Post patches
|
||
|
early and listen to feedback.</li><li><p>Post patches in unified diff format. A command similar to this
|
||
|
should work:</p>
|
||
|
<pre>
|
||
|
diff -urp libvirt.orig/ libvirt.modified/ > libvirt-myfeature.patch
|
||
|
</pre>
|
||
|
<p>
|
||
|
or:
|
||
|
</p>
|
||
|
<pre>
|
||
|
cvs diff -up > 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><li>Make sure your patches apply against libvirt CVS. Developers
|
||
|
only follow CVS and don't care much about released versions.</li><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>
|
||
|
<p>
|
||
|
and run the tests:
|
||
|
</p>
|
||
|
<pre>
|
||
|
make check
|
||
|
make syntax-check
|
||
|
make -C tests valgrind
|
||
|
</pre>
|
||
|
<p>
|
||
|
The latter test checks for memory leaks.
|
||
|
</p>
|
||
|
|
||
|
</li><li>Update tests and/or documentation, particularly if you are adding
|
||
|
a new feature or changing the output of a program.</li></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/">
|
||
|
Richard Jones' guide to working with open source projects</a>
|
||
|
</p>
|
||
|
<h2>
|
||
|
<a name="indent" id="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&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&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
|
||
|
'(lambda () (if (string-match "/libvirt" (buffer-file-name))
|
||
|
(libvirt-c-mode))))
|
||
|
</pre>
|
||
|
<h2>
|
||
|
<a name="formatting" id="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
|
||
|
unsigned type.</li><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";
|
||
|
(on all but oddball embedded systems, you can assume that that
|
||
|
type is at least four bytes wide).</li><li>If a variable has boolean semantics, give it the "bool" type
|
||
|
and use the corresponding "true" and "false" macros. It's ok
|
||
|
to include <stdbool.h>, since libvirt's use of gnulib ensures
|
||
|
that it exists and is usable.</li><li>In the unusual event that you require a specific width, use a
|
||
|
standard type like int32_t, uint32_t, uint64_t, etc.</li><li>While using "bool" is good for readability, it comes with minor caveats:
|
||
|
<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></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" id="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) < 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) < 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) < 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) < 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" id="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>
|
||
|
<pre>
|
||
|
STREQ(a,b)
|
||
|
STRNEQ(a,b)
|
||
|
</pre>
|
||
|
</li><li><p>For case sensitive equality:</p>
|
||
|
<pre>
|
||
|
STRCASEEQ(a,b)
|
||
|
STRCASENEQ(a,b)
|
||
|
</pre>
|
||
|
</li><li><p>For strict equality of a substring:</p>
|
||
|
|
||
|
<pre>
|
||
|
STREQLEN(a,b,n)
|
||
|
STRNEQLEN(a,b,n)
|
||
|
</pre>
|
||
|
</li><li><p>For case sensitive equality of a substring:</p>
|
||
|
|
||
|
<pre>
|
||
|
STRCASEEQLEN(a,b,n)
|
||
|
STRCASENEQLEN(a,b,n)
|
||
|
</pre>
|
||
|
</li><li><p>For strict equality of a prefix:</p>
|
||
|
|
||
|
<pre>
|
||
|
STRPREFIX(a,b)
|
||
|
</pre>
|
||
|
</li></ul>
|
||
|
<h2>
|
||
|
<a name="strbuf" id="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(&buf, "<domain>\n");
|
||
|
virBufferVSprint(&buf, " <memory>%d</memory>\n", memory);
|
||
|
...
|
||
|
virBufferAddLit(&buf, "</domain>\n");
|
||
|
|
||
|
....
|
||
|
|
||
|
if (virBufferError(&buf)) {
|
||
|
__virRaiseError(...);
|
||
|
return NULL;
|
||
|
}
|
||
|
|
||
|
return virBufferContentAndReset(&buf);
|
||
|
}
|
||
|
</pre>
|
||
|
<h2>
|
||
|
<a name="includes" id="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 <config.h> Must come first in every file.
|
||
|
|
||
|
#include <stdio.h> Any system includes you need.
|
||
|
#include <string.h>
|
||
|
#include <limits.h>
|
||
|
|
||
|
#if HAVE_NUMACTL Some system includes aren't supported
|
||
|
#include <numa.h> 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" id="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, ...)
|
||
|
ATTRIBUTE_FORMAT(printf, 2, 3);
|
||
|
</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" id="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
|
||
|
or for a given driver then it's fine to commit a minimal fix
|
||
|
directly without getting the review feedback first</li><li>if make check or make syntax-chek breaks, if there is
|
||
|
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><li>
|
||
|
fixes for documentation and code comments can be managed
|
||
|
in the same way, but still make sure they get reviewed if non-trivial.
|
||
|
</li></ul>
|
||
|
</div>
|
||
|
</div>
|
||
|
<div id="footer">
|
||
|
<p id="sponsor">
|
||
|
Sponsored by:<br /><a href="http://et.redhat.com/"><img src="et.png" alt="Project sponsored by Red Hat Emerging Technology" /></a></p>
|
||
|
</div>
|
||
|
</body>
|
||
|
</html>
|