mirror of
https://gitlab.com/libvirt/libvirt.git
synced 2024-08-27 19:11:16 +00:00
* src/util.c (virAsprintf): Remove trailing space. * src/util.h (virAsprintf): Use ATTRIBUTE_FORMAT. * HACKING (Printf-style functions): New section.
296 lines
8.5 KiB
Plaintext
296 lines
8.5 KiB
Plaintext
Libvirt contributor guidelines
|
|
==============================
|
|
|
|
|
|
General tips for contributing patches
|
|
=====================================
|
|
|
|
(1) Discuss any large changes on the mailing list first. Post patches
|
|
early and listen to feedback.
|
|
|
|
(2) Post patches in unified diff format. A command similar to this
|
|
should work:
|
|
|
|
diff -urp libvirt.orig/ libvirt.modified/ > libvirt-myfeature.patch
|
|
|
|
or:
|
|
|
|
cvs diff -up > libvirt-myfeature.patch
|
|
|
|
(3) 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.
|
|
|
|
(4) Make sure your patches apply against libvirt CVS. Developers
|
|
only follow CVS and don't care much about released versions.
|
|
|
|
(5) 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
|
|
|
|
and run the tests:
|
|
|
|
make check
|
|
make syntax-check
|
|
make -C tests valgrind
|
|
|
|
The latter test checks for memory leaks.
|
|
|
|
(6) Update tests and/or documentation, particularly if you are adding
|
|
a new feature or changing the output of a program.
|
|
|
|
|
|
There is more on this subject, including lots of links to background
|
|
reading on the subject, on this page:
|
|
|
|
http://et.redhat.com/~rjones/how-to-supply-code-to-open-source-projects/
|
|
|
|
|
|
|
|
Code indentation
|
|
================
|
|
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.
|
|
|
|
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:
|
|
|
|
;;; 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))))
|
|
|
|
Code formatting (especially for new code)
|
|
=========================================
|
|
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:
|
|
|
|
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 "$@"
|
|
}
|
|
|
|
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.
|
|
|
|
|
|
Low level memory management
|
|
===========================
|
|
|
|
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
|
|
|
|
- eg to allocate a single object:
|
|
|
|
virDomainPtr domain;
|
|
|
|
if (VIR_ALLOC(domain) < 0) {
|
|
__virRaiseError(VIR_ERROR_NO_MEMORY)
|
|
return NULL;
|
|
}
|
|
|
|
|
|
- eg to allocate an array of objects
|
|
|
|
virDomainPtr domains;
|
|
int ndomains = 10;
|
|
|
|
if (VIR_ALLOC_N(domains, ndomains) < 0) {
|
|
__virRaiseError(VIR_ERROR_NO_MEMORY)
|
|
return NULL;
|
|
}
|
|
|
|
- eg to allocate an array of object pointers
|
|
|
|
virDomainPtr *domains;
|
|
int ndomains = 10;
|
|
|
|
if (VIR_ALLOC_N(domains, ndomains) < 0) {
|
|
__virRaiseError(VIR_ERROR_NO_MEMORY)
|
|
return NULL;
|
|
}
|
|
|
|
- eg to re-allocate the array of domains to be longer
|
|
|
|
ndomains = 20
|
|
|
|
if (VIR_REALLOC_N(domains, ndomains) < 0) {
|
|
__virRaiseError(VIR_ERROR_NO_MEMORY)
|
|
return NULL;
|
|
}
|
|
|
|
- eg to free the domain
|
|
|
|
VIR_FREE(domain);
|
|
|
|
|
|
|
|
String comparisons
|
|
==================
|
|
|
|
Do not use the strcmp, strncmp, etc functions directly. Instead use
|
|
one of the following semantically named macros
|
|
|
|
- For strict equality:
|
|
|
|
STREQ(a,b)
|
|
STRNEQ(a,b)
|
|
|
|
- For case sensitive equality:
|
|
STRCASEEQ(a,b)
|
|
STRCASENEQ(a,b)
|
|
|
|
- For strict equality of a substring:
|
|
|
|
STREQLEN(a,b,n)
|
|
STRNEQLEN(a,b,n)
|
|
|
|
- For case sensitive equality of a substring:
|
|
|
|
STRCASEEQLEN(a,b,n)
|
|
STRCASENEQLEN(a,b,n)
|
|
|
|
- For strict equality of a prefix:
|
|
|
|
STRPREFIX(a,b)
|
|
|
|
|
|
|
|
Variable length string buffer
|
|
=============================
|
|
|
|
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
|
|
|
|
eg typical usage is as follows:
|
|
|
|
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);
|
|
}
|
|
|
|
|
|
Include files
|
|
=============
|
|
|
|
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:
|
|
|
|
/*
|
|
* 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.
|
|
{
|
|
...
|
|
|
|
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.
|
|
|
|
|
|
Printf-style functions
|
|
======================
|
|
|
|
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:
|
|
|
|
int virAsprintf(char **strp, const char *fmt, ...)
|
|
ATTRIBUTE_FORMAT(printf, 2, 3);
|
|
|
|
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.
|
|
|
|
|
|
|
|
Libvirt commiters guidelines
|
|
============================
|
|
|
|
The AUTHORS files indicates the list of people with commit acces right
|
|
who can actually merge the patches.
|
|
|
|
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
|
|
|
|
Exceptions to that 'review and approval on the list first' is fixing failures
|
|
to build:
|
|
- 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
|
|
- similary 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
|
|
Similary fixes for documentation and code comments can be managed
|
|
in the same way, but still make sure they get reviewed if non-trivial.
|