mirror of
https://gitlab.com/libvirt/libvirt.git
synced 2024-12-22 13:45:38 +00:00
Generate HACKING from docs/hacking.html.in
This commit is contained in:
parent
d39620e367
commit
34ed4e9809
592
HACKING
592
HACKING
@ -1,18 +1,19 @@
|
||||
-*- buffer-read-only: t -*- vi: set ro:
|
||||
DO NOT EDIT THIS FILE! IT IS GENERATED AUTOMATICALLY!
|
||||
|
||||
Libvirt contributor guidelines
|
||||
==============================
|
||||
|
||||
|
||||
Contributor guidelines
|
||||
======================
|
||||
|
||||
|
||||
|
||||
General tips for contributing patches
|
||||
=====================================
|
||||
(1) Discuss any large changes on the mailing list first. Post patches early and
|
||||
listen to feedback.
|
||||
|
||||
(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:
|
||||
(2) Post patches in unified diff format. A command similar to this should work:
|
||||
|
||||
diff -urp libvirt.orig/ libvirt.modified/ > libvirt-myfeature.patch
|
||||
|
||||
@ -20,15 +21,15 @@ or:
|
||||
|
||||
git diff > 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.
|
||||
(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 GIT. Developers
|
||||
only follow GIT and don't care much about released versions.
|
||||
(4) Make sure your patches apply against libvirt GIT. Developers only follow GIT
|
||||
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:
|
||||
(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
|
||||
|
||||
@ -47,28 +48,29 @@ VIR_TEST_DEBUG may provide larger amounts of information:
|
||||
VIR_TEST_DEBUG=1 make check (or)
|
||||
VIR_TEST_DEBUG=2 make check
|
||||
|
||||
Also, individual tests can be run from inside the 'tests/' directory, like:
|
||||
Also, individual tests can be run from inside the "tests/" directory, like:
|
||||
|
||||
./qemuxml2xmltest
|
||||
|
||||
(6) Update tests and/or documentation, particularly if you are adding
|
||||
a new feature or changing the output of a program.
|
||||
(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:
|
||||
|
||||
There is more on this subject, including lots of links to background reading
|
||||
on the subject, on
|
||||
|
||||
Richard Jones' guide to working with open source projects
|
||||
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.
|
||||
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:
|
||||
@ -82,15 +84,15 @@ If you use Emacs, add the following to one of one of your start-up files
|
||||
(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))))
|
||||
'(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:
|
||||
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()
|
||||
{
|
||||
@ -99,66 +101,63 @@ around operators and keywords:
|
||||
--no-tabs "$@"
|
||||
}
|
||||
|
||||
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.
|
||||
Usually they're in macro definitions or strings, and should be converted
|
||||
anyhow.
|
||||
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. Usually
|
||||
they're in macro definitions or strings, and should be converted 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.
|
||||
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 ();
|
||||
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:
|
||||
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 ();
|
||||
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 ();
|
||||
}
|
||||
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"));
|
||||
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
|
||||
...
|
||||
}
|
||||
if (expr) // BAD: no braces around...
|
||||
while (expr_2) { // ... a multi-line body
|
||||
...
|
||||
}
|
||||
|
||||
Do this, instead:
|
||||
|
||||
if (expr) {
|
||||
while (expr_2) {
|
||||
...
|
||||
}
|
||||
}
|
||||
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
|
||||
@ -167,47 +166,47 @@ 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"
|
||||
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
|
||||
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 (!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;
|
||||
}
|
||||
if (expr) {
|
||||
...
|
||||
...
|
||||
} else {
|
||||
x = y;
|
||||
}
|
||||
|
||||
|
||||
Preprocessor
|
||||
============
|
||||
For variadic macros, stick with C99 syntax:
|
||||
|
||||
#define vshPrint(_ctl, ...) fprintf(stdout, __VA_ARGS__)
|
||||
#define vshPrint(_ctl, ...) fprintf(stdout, __VA_ARGS__)
|
||||
|
||||
Use parenthesis when checking if a macro is defined, and use
|
||||
indentation to track nesting:
|
||||
Use parenthesis when checking if a macro is defined, and use indentation to
|
||||
track nesting:
|
||||
|
||||
#if defined(HAVE_POSIX_FALLOCATE) && !defined(HAVE_FALLOCATE)
|
||||
# define fallocate(a,ignored,b,c) posix_fallocate(a,b,c)
|
||||
#endif
|
||||
#if defined(HAVE_POSIX_FALLOCATE) && !defined(HAVE_FALLOCATE)
|
||||
# define fallocate(a,ignored,b,c) posix_fallocate(a,b,c)
|
||||
#endif
|
||||
|
||||
|
||||
C types
|
||||
@ -216,199 +215,235 @@ Use the right type.
|
||||
|
||||
Scalars
|
||||
-------
|
||||
If you're using "int" or "long", odds are good that there's a better type.
|
||||
If a variable is counting something, be sure to declare it with an
|
||||
unsigned type.
|
||||
If it's memory-size-related, use size_t (use ssize_t only if required).
|
||||
If it's file-size related, use uintmax_t, or maybe off_t.
|
||||
If it's file-offset related (i.e., signed), use off_t.
|
||||
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).
|
||||
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.
|
||||
In the unusual event that you require a specific width, use a
|
||||
standard type like int32_t, uint32_t, uint64_t, etc.
|
||||
- If you're using "int" or "long", odds are good that there's a better type.
|
||||
|
||||
While using "bool" is good for readability, it comes with minor caveats:
|
||||
- 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.
|
||||
- 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)...".
|
||||
- If a variable is counting something, be sure to declare it with an unsigned
|
||||
type.
|
||||
|
||||
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.
|
||||
- If it's memory-size-related, use "size_t" (use "ssize_t" only if required).
|
||||
|
||||
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.
|
||||
- If it's file-size related, use uintmax_t, or maybe "off_t".
|
||||
|
||||
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.
|
||||
- If it's file-offset related (i.e., signed), use "off_t".
|
||||
|
||||
- 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).
|
||||
|
||||
- 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.
|
||||
|
||||
- In the unusual event that you require a specific width, use a standard type
|
||||
like "int32_t", "uint32_t", "uint64_t", etc.
|
||||
|
||||
- While using "bool" is good for readability, it comes with minor caveats:
|
||||
|
||||
-- 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.
|
||||
|
||||
-- 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)...".
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
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.
|
||||
|
||||
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.
|
||||
|
||||
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.
|
||||
|
||||
Pointers
|
||||
--------
|
||||
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.
|
||||
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.
|
||||
|
||||
|
||||
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
|
||||
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:
|
||||
- e.g. to allocate a single object:
|
||||
|
||||
virDomainPtr domain;
|
||||
virDomainPtr domain;
|
||||
|
||||
if (VIR_ALLOC(domain) < 0) {
|
||||
virReportOOMError();
|
||||
return NULL;
|
||||
}
|
||||
if (VIR_ALLOC(domain) < 0) {
|
||||
virReportOOMError();
|
||||
return NULL;
|
||||
}
|
||||
|
||||
|
||||
- eg to allocate an array of objects
|
||||
|
||||
virDomainPtr domains;
|
||||
int ndomains = 10;
|
||||
- e.g. to allocate an array of objects
|
||||
|
||||
if (VIR_ALLOC_N(domains, ndomains) < 0) {
|
||||
virReportOOMError();
|
||||
return NULL;
|
||||
}
|
||||
virDomainPtr domains;
|
||||
int ndomains = 10;
|
||||
|
||||
- eg to allocate an array of object pointers
|
||||
if (VIR_ALLOC_N(domains, ndomains) < 0) {
|
||||
virReportOOMError();
|
||||
return NULL;
|
||||
}
|
||||
|
||||
virDomainPtr *domains;
|
||||
int ndomains = 10;
|
||||
|
||||
if (VIR_ALLOC_N(domains, ndomains) < 0) {
|
||||
virReportOOMError();
|
||||
return NULL;
|
||||
}
|
||||
|
||||
- eg to re-allocate the array of domains to be longer
|
||||
- e.g. to allocate an array of object pointers
|
||||
|
||||
virDomainPtr *domains;
|
||||
int ndomains = 10;
|
||||
|
||||
if (VIR_ALLOC_N(domains, ndomains) < 0) {
|
||||
virReportOOMError();
|
||||
return NULL;
|
||||
}
|
||||
|
||||
|
||||
|
||||
- e.g. to re-allocate the array of domains to be longer
|
||||
|
||||
ndomains = 20
|
||||
|
||||
if (VIR_REALLOC_N(domains, ndomains) < 0) {
|
||||
virReportOOMError();
|
||||
return NULL;
|
||||
}
|
||||
|
||||
|
||||
|
||||
- e.g. to free the domain
|
||||
|
||||
VIR_FREE(domain);
|
||||
|
||||
ndomains = 20
|
||||
|
||||
if (VIR_REALLOC_N(domains, ndomains) < 0) {
|
||||
virReportOOMError();
|
||||
return NULL;
|
||||
}
|
||||
|
||||
- eg to free the domain
|
||||
|
||||
VIR_FREE(domain);
|
||||
|
||||
|
||||
File handling
|
||||
=============
|
||||
|
||||
Use of the close() API is deprecated in libvirt code base to help avoiding
|
||||
double-closing of a file descriptor. Instead of this API, use the macro from
|
||||
files.h
|
||||
|
||||
- eg close a file descriptor
|
||||
- e.g. close a file descriptor
|
||||
|
||||
if (VIR_CLOSE(fd) < 0) {
|
||||
virReportSystemError(errno, _("failed to close file"));
|
||||
}
|
||||
|
||||
|
||||
|
||||
- eg close a file descriptor in an error path, without losing the previous
|
||||
"errno" value
|
||||
|
||||
VIR_FORCE_CLOSE(fd);
|
||||
|
||||
|
||||
|
||||
if (VIR_CLOSE(fd) < 0) {
|
||||
virReportSystemError(errno, _("failed to close file"));
|
||||
}
|
||||
|
||||
- eg close a file descriptor in an error path, without losing the previous
|
||||
errno value
|
||||
|
||||
VIR_FORCE_CLOSE(fd);
|
||||
|
||||
String comparisons
|
||||
==================
|
||||
Do not use the strcmp, strncmp, etc functions directly. Instead use one of the
|
||||
following semantically named macros
|
||||
|
||||
Do not use the strcmp, strncmp, etc functions directly. Instead use
|
||||
one of the following semantically named macros
|
||||
- For strict equality:
|
||||
|
||||
- For strict equality:
|
||||
STREQ(a,b)
|
||||
STRNEQ(a,b)
|
||||
|
||||
STREQ(a,b)
|
||||
STRNEQ(a,b)
|
||||
|
||||
- For case insensitive equality:
|
||||
STRCASEEQ(a,b)
|
||||
STRCASENEQ(a,b)
|
||||
|
||||
- For strict equality of a substring:
|
||||
- For case insensitive equality:
|
||||
|
||||
STREQLEN(a,b,n)
|
||||
STRNEQLEN(a,b,n)
|
||||
STRCASEEQ(a,b)
|
||||
STRCASENEQ(a,b)
|
||||
|
||||
- For case insensitive equality of a substring:
|
||||
|
||||
STRCASEEQLEN(a,b,n)
|
||||
STRCASENEQLEN(a,b,n)
|
||||
|
||||
- For strict equality of a prefix:
|
||||
- For strict equality of a substring:
|
||||
|
||||
STREQLEN(a,b,n)
|
||||
STRNEQLEN(a,b,n)
|
||||
|
||||
|
||||
|
||||
- For case insensitive equality of a substring:
|
||||
|
||||
STRCASEEQLEN(a,b,n)
|
||||
STRCASENEQLEN(a,b,n)
|
||||
|
||||
|
||||
|
||||
- For strict equality of a prefix:
|
||||
|
||||
STRPREFIX(a,b)
|
||||
|
||||
|
||||
|
||||
STRPREFIX(a,b)
|
||||
|
||||
|
||||
|
||||
String copying
|
||||
==============
|
||||
Do not use the strncpy function. According to the man page, it does *not*
|
||||
guarantee a NULL-terminated buffer, which makes it extremely dangerous to use.
|
||||
Instead, use one of the functionally equivalent functions:
|
||||
|
||||
Do not use the strncpy function. According to the man page, it does
|
||||
*not* guarantee a NULL-terminated buffer, which makes it extremely dangerous
|
||||
to use. Instead, use one of the functionally equivalent functions:
|
||||
virStrncpy(char *dest, const char *src, size_t n, size_t destbytes)
|
||||
|
||||
- virStrncpy(char *dest, const char *src, size_t n, size_t destbytes)
|
||||
The first three arguments have the same meaning as for strncpy; namely the
|
||||
destination, source, and number of bytes to copy, respectively. The last
|
||||
argument is the number of bytes available in the destination string; if a
|
||||
copy of the source string (including a \0) will not fit into the
|
||||
destination, no bytes are copied and the routine returns NULL.
|
||||
Otherwise, n bytes from the source are copied into the destination and a
|
||||
trailing \0 is appended.
|
||||
The first three arguments have the same meaning as for strncpy; namely the
|
||||
destination, source, and number of bytes to copy, respectively. The last
|
||||
argument is the number of bytes available in the destination string; if a copy
|
||||
of the source string (including a \0) will not fit into the destination, no
|
||||
bytes are copied and the routine returns NULL. Otherwise, n bytes from the
|
||||
source are copied into the destination and a trailing \0 is appended.
|
||||
|
||||
- virStrcpy(char *dest, const char *src, size_t destbytes)
|
||||
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 be
|
||||
evaluated more than once. This is equivalent to
|
||||
virStrncpy(dest, src, strlen(src), destbytes)
|
||||
virStrcpy(char *dest, const char *src, size_t destbytes)
|
||||
|
||||
- virStrcpyStatic(char *dest, const char *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 a static string
|
||||
(i.e. that sizeof(dest) returns something meaningful). Note that
|
||||
this is a macro, so arguments could be evaluated more than once. This is
|
||||
equivalent to virStrncpy(dest, src, strlen(src), sizeof(dest)).
|
||||
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 be evaluated more than once.
|
||||
This is equivalent to virStrncpy(dest, src, strlen(src), destbytes)
|
||||
|
||||
virStrcpyStatic(char *dest, const char *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 a static string (i.e. that
|
||||
sizeof(dest) returns something meaningful). Note that this is a macro, so
|
||||
arguments could be evaluated more than once. This is equivalent to
|
||||
virStrncpy(dest, src, strlen(src), sizeof(dest)).
|
||||
|
||||
|
||||
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
|
||||
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(...) {
|
||||
somefunction(...)
|
||||
{
|
||||
virBuffer buf = VIR_BUFFER_INITIALIZER;
|
||||
|
||||
...
|
||||
@ -432,11 +467,9 @@ eg typical usage is as follows:
|
||||
|
||||
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:
|
||||
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
|
||||
@ -461,59 +494,112 @@ complexity it's best to stick to the following general plan for all
|
||||
#include "util.h" Any libvirt internal header files.
|
||||
#include "buf.h"
|
||||
|
||||
static myInternalFunc () The actual code.
|
||||
static int
|
||||
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.
|
||||
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:
|
||||
|
||||
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);
|
||||
|
||||
int virAsprintf(char **strp, const char *fmt, ...)
|
||||
ATTRIBUTE_FMT_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.
|
||||
|
||||
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.
|
||||
|
||||
Use of goto
|
||||
===========
|
||||
The use of goto is not forbidden, and goto is widely used throughout libvirt.
|
||||
While the uncontrolled use of goto will quickly lead to unmaintainable code,
|
||||
there is a place for it in well structured code where its use increases
|
||||
readability and maintainability. In general, if goto is used for error
|
||||
recovery, it's likely to be ok, otherwise, be cautious or avoid it all
|
||||
together.
|
||||
|
||||
The typical use of goto is to jump to cleanup code in the case of a long list
|
||||
of actions, any of which may fail and cause the entire operation to fail. In
|
||||
this case, a function will have a single label at the end of the function.
|
||||
It's almost always ok to use this style. In particular, if the cleanup code
|
||||
only involves free'ing memory, then having multiple labels is overkill.
|
||||
VIR_FREE() and every function named XXXFree() in libvirt is required to handle
|
||||
NULL as its arg. Thus you can safely call free on all the variables even if
|
||||
they were not yet allocated (yes they have to have been initialized to NULL).
|
||||
This is much simpler and clearer than having multiple labels.
|
||||
|
||||
There are a couple of signs that a particular use of goto is not ok:
|
||||
|
||||
- You're using multiple labels. If you find yourself using multiple labels,
|
||||
you're strongly encouraged to rework your code to eliminate all but one of
|
||||
them.
|
||||
|
||||
- The goto jumps back up to a point above the current line of code being
|
||||
executed. Please use some combination of looping constructs to re-execute code
|
||||
instead; it's almost certainly going to be more understandable by others. One
|
||||
well-known exception to this rule is restarting an i/o operation following
|
||||
EINTR.
|
||||
|
||||
- The goto jumps down to an arbitrary place in the middle of a function followed
|
||||
by further potentially failing calls. You should almost certainly be using a
|
||||
conditional and a block instead of a goto. Perhaps some of your function's
|
||||
logic would be better pulled out into a helper function.
|
||||
|
||||
|
||||
|
||||
Libvirt committer guidelines
|
||||
============================
|
||||
Although libvirt does not encourage the Linux kernel wind/unwind style of
|
||||
multiple labels, there's a good general discussion of the issue archived at
|
||||
|
||||
The AUTHORS files indicates the list of people with commit access right
|
||||
who can actually merge the patches.
|
||||
KernelTrap
|
||||
http://kerneltrap.org/node/553/2131
|
||||
|
||||
When using goto, please use one of these standard labels if it makes sense:
|
||||
|
||||
error: A path only taken upon return with an error code
|
||||
cleanup: A path taken upon return with success code + optional error
|
||||
no_memory: A path only taken upon return with an OOM error code
|
||||
retry: If needing to jump upwards (eg retry on EINTR)
|
||||
|
||||
|
||||
Libvirt committer guidelines
|
||||
============================
|
||||
The AUTHORS files indicates the list of people with commit access right who
|
||||
can actually merge the patches.
|
||||
|
||||
The general rule for committing a patch is to make sure it has been reviewed
|
||||
properly in the mailing-list first, usually if a couple of people 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 committing 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
|
||||
properly in the mailing-list first, usually if a couple of people 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 where you do not have a very clear idea of how things work,
|
||||
it's better to wait for a more authoritative feedback though. Before
|
||||
committing, please also rebuild locally, run 'make check syntax-check', and
|
||||
make sure you don't raise errors. 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 committed 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
|
||||
- similarly, if make check or make syntax-check 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 committing
|
||||
anything
|
||||
Similar fixes for documentation and code comments can be managed
|
||||
in the same way, but still make sure they get reviewed if non-trivial.
|
||||
An exception to 'review and approval on the list first' is fixing failures to
|
||||
build:
|
||||
|
||||
- if a recently committed 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
|
||||
|
||||
- if make check or make syntax-check 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 committing anything
|
||||
|
||||
- fixes for documentation and code comments can be managed in the same way, but
|
||||
still make sure they get reviewed if non-trivial.
|
||||
|
@ -56,6 +56,13 @@ NEWS: $(top_srcdir)/docs/news.xsl $(top_srcdir)/docs/news.html.in
|
||||
| perl -pe 's/[ \t]+$$//' \
|
||||
> $@-t && mv $@-t $@ ; fi );
|
||||
|
||||
$(top_srcdir)/HACKING: $(top_srcdir)/docs/hacking1.xsl $(top_srcdir)/docs/hacking2.xsl \
|
||||
$(top_srcdir)/docs/wrapstring.xsl $(top_srcdir)/docs/hacking.html.in
|
||||
-@(if [ -x $(XSLTPROC) ] ; then \
|
||||
$(XSLTPROC) --nonet $(top_srcdir)/docs/hacking1.xsl $(top_srcdir)/docs/hacking.html.in | \
|
||||
$(XSLTPROC) --nonet $(top_srcdir)/docs/hacking2.xsl - \
|
||||
| perl -0777 -pe 's/\n\n+$$/\n/' \
|
||||
> $@-t && mv $@-t $@ ; fi );
|
||||
|
||||
rpm: clean
|
||||
@(unset CDPATH ; $(MAKE) dist && rpmbuild -ta $(distdir).tar.gz)
|
||||
|
3
cfg.mk
3
cfg.mk
@ -486,3 +486,6 @@ _autogen:
|
||||
|
||||
# Exempt @...@ uses of these symbols.
|
||||
_makefile_at_at_check_exceptions = ' && !/(SCHEMA|SYSCONF)DIR/'
|
||||
|
||||
# regenerate HACKING as part of the syntax-check
|
||||
syntax-check: $(top_srcdir)/HACKING
|
||||
|
28
docs/hacking1.xsl
Normal file
28
docs/hacking1.xsl
Normal file
@ -0,0 +1,28 @@
|
||||
<?xml version="1.0"?>
|
||||
<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
|
||||
|
||||
<xsl:output method="xml" encoding="UTF-8" indent="no"/>
|
||||
|
||||
|
||||
|
||||
<xsl:template match="/">
|
||||
<xsl:apply-templates/>
|
||||
</xsl:template>
|
||||
|
||||
|
||||
|
||||
<xsl:template match="@*|node()">
|
||||
<xsl:copy>
|
||||
<xsl:apply-templates select="@*|node()"/>
|
||||
</xsl:copy>
|
||||
</xsl:template>
|
||||
|
||||
|
||||
|
||||
<!-- resolve b/i/code tags in a first pass, because they interfere with line
|
||||
wrapping in the second pass -->
|
||||
<xsl:template match="b">*<xsl:apply-templates/>*</xsl:template>
|
||||
<xsl:template match="i">'<xsl:apply-templates/>'</xsl:template>
|
||||
<xsl:template match="code">"<xsl:apply-templates/>"</xsl:template>
|
||||
|
||||
</xsl:stylesheet>
|
146
docs/hacking2.xsl
Normal file
146
docs/hacking2.xsl
Normal file
@ -0,0 +1,146 @@
|
||||
<?xml version="1.0"?>
|
||||
<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
|
||||
|
||||
<xsl:import href="wrapstring.xsl"/>
|
||||
|
||||
<xsl:output method="text" encoding="UTF-8" indent="no"/>
|
||||
|
||||
<xsl:strip-space elements="*"/>
|
||||
|
||||
|
||||
|
||||
<xsl:variable name="newline">
|
||||
<xsl:text>
|
||||
</xsl:text>
|
||||
</xsl:variable>
|
||||
|
||||
|
||||
|
||||
<xsl:template match="/">
|
||||
<xsl:text>-*- buffer-read-only: t -*- vi: set ro:
|
||||
DO NOT EDIT THIS FILE! IT IS GENERATED AUTOMATICALLY!
|
||||
|
||||
|
||||
|
||||
</xsl:text>
|
||||
<xsl:apply-templates/>
|
||||
</xsl:template>
|
||||
|
||||
|
||||
|
||||
<!-- title -->
|
||||
<xsl:template match="h1">
|
||||
<xsl:text> </xsl:text>
|
||||
<xsl:value-of select="normalize-space(.)"/>
|
||||
<xsl:text>
|
||||
</xsl:text>======================
|
||||
</xsl:template>
|
||||
|
||||
|
||||
|
||||
<!-- output the current text node underlined -->
|
||||
<xsl:template name="underline">
|
||||
<xsl:param name="text" select="normalize-space(.)"/>
|
||||
<xsl:param name="text-length" select="string-length($text)"/>
|
||||
<xsl:param name="char" select="'='"/>
|
||||
<xsl:param name="line" select="$char"/>
|
||||
<xsl:choose>
|
||||
<xsl:when test="$text-length > 1">
|
||||
<xsl:call-template name="underline">
|
||||
<xsl:with-param name="text" select="$text"/>
|
||||
<xsl:with-param name="text-length" select="$text-length - 1"/>
|
||||
<xsl:with-param name="char" select="$char"/>
|
||||
<xsl:with-param name="line" select="concat($line,$char)"/>
|
||||
</xsl:call-template>
|
||||
</xsl:when>
|
||||
<xsl:otherwise>
|
||||
<xsl:value-of select="$text"/>
|
||||
<xsl:value-of select="$newline"/>
|
||||
<xsl:value-of select="$line"/>
|
||||
<xsl:value-of select="$newline"/>
|
||||
</xsl:otherwise>
|
||||
</xsl:choose>
|
||||
</xsl:template>
|
||||
|
||||
|
||||
|
||||
<xsl:template match="h2">
|
||||
<xsl:value-of select="$newline"/>
|
||||
<xsl:call-template name="underline"/>
|
||||
</xsl:template>
|
||||
|
||||
|
||||
|
||||
<xsl:template match="h3">
|
||||
<xsl:call-template name="underline">
|
||||
<xsl:with-param name="char" select="'-'"/>
|
||||
</xsl:call-template>
|
||||
</xsl:template>
|
||||
|
||||
|
||||
|
||||
<!-- output text line wrapped at 80 chars -->
|
||||
<xsl:template match="text()">
|
||||
<xsl:call-template name="wrap-string">
|
||||
<xsl:with-param name="str" select="normalize-space(.)"/>
|
||||
<xsl:with-param name="wrap-col" select="80"/>
|
||||
<xsl:with-param name="break-mark" select="$newline"/>
|
||||
</xsl:call-template>
|
||||
</xsl:template>
|
||||
|
||||
|
||||
|
||||
<xsl:template match="ol|ul|p">
|
||||
<xsl:apply-templates/><xsl:value-of select="$newline"/><xsl:value-of select="$newline"/>
|
||||
</xsl:template>
|
||||
|
||||
|
||||
|
||||
<xsl:template match="ol/li">
|
||||
<xsl:choose>
|
||||
<xsl:when test=".//node()[position()=last()]/self::pre">(<xsl:value-of select="position()"/>) <xsl:apply-templates/>
|
||||
</xsl:when>
|
||||
<!-- only append two newlines when the last element isn't a pre element -->
|
||||
<xsl:otherwise>(<xsl:value-of select="position()"/>) <xsl:apply-templates/><xsl:value-of select="$newline"/><xsl:value-of select="$newline"/>
|
||||
</xsl:otherwise>
|
||||
</xsl:choose>
|
||||
</xsl:template>
|
||||
|
||||
|
||||
|
||||
<xsl:template match="ul/li">- <xsl:apply-templates/><xsl:value-of select="$newline"/><xsl:value-of select="$newline"/>
|
||||
</xsl:template>
|
||||
|
||||
|
||||
|
||||
<xsl:template match="li/ul/li">-- <xsl:apply-templates/><xsl:value-of select="$newline"/><xsl:value-of select="$newline"/>
|
||||
</xsl:template>
|
||||
|
||||
|
||||
|
||||
<!-- add newline before nested <ul> -->
|
||||
<xsl:template match="li/ul"><xsl:value-of select="$newline"/><xsl:value-of select="$newline"/><xsl:apply-templates/>
|
||||
</xsl:template>
|
||||
|
||||
|
||||
|
||||
<xsl:template match="pre">
|
||||
<xsl:choose>
|
||||
<xsl:when test="starts-with(.,'
')"><xsl:value-of select="substring(.,2)"/><xsl:value-of select="$newline"/>
|
||||
</xsl:when>
|
||||
<xsl:otherwise>
|
||||
<xsl:value-of select="."/><xsl:value-of select="$newline"/>
|
||||
</xsl:otherwise>
|
||||
</xsl:choose>
|
||||
</xsl:template>
|
||||
|
||||
|
||||
|
||||
<xsl:template match="a">
|
||||
<xsl:value-of select="$newline"/><xsl:value-of select="$newline"/>
|
||||
<xsl:text> </xsl:text><xsl:apply-templates/>
|
||||
<xsl:value-of select="$newline"/>
|
||||
<xsl:text> </xsl:text><xsl:value-of select="@href"/>
|
||||
</xsl:template>
|
||||
|
||||
</xsl:stylesheet>
|
56
docs/wrapstring.xsl
Normal file
56
docs/wrapstring.xsl
Normal file
@ -0,0 +1,56 @@
|
||||
<?xml version="1.0"?>
|
||||
<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
|
||||
|
||||
<!-- based on http://plasmasturm.org/log/xslwordwrap/ -->
|
||||
<!-- Copyright 2010 Aristotle Pagaltzis; under the MIT licence -->
|
||||
<!-- http://www.opensource.org/licenses/mit-license.php -->
|
||||
<xsl:template name="wrap-string">
|
||||
<xsl:param name="str" />
|
||||
<xsl:param name="wrap-col" />
|
||||
<xsl:param name="break-mark" />
|
||||
<xsl:param name="pos" select="0" />
|
||||
<xsl:choose>
|
||||
<xsl:when test="contains( $str, ' ' )">
|
||||
<xsl:variable name="first-word" select="substring-before( $str, ' ' )" />
|
||||
<xsl:variable name="pos-now" select="$pos + 1 + string-length( $first-word )" />
|
||||
<xsl:choose>
|
||||
<xsl:when test="$pos > 0 and $pos-now >= $wrap-col">
|
||||
<xsl:copy-of select="$break-mark" />
|
||||
<xsl:call-template name="wrap-string">
|
||||
<xsl:with-param name="str" select="$str" />
|
||||
<xsl:with-param name="wrap-col" select="$wrap-col" />
|
||||
<xsl:with-param name="break-mark" select="$break-mark" />
|
||||
<xsl:with-param name="pos" select="0" />
|
||||
</xsl:call-template>
|
||||
</xsl:when>
|
||||
<xsl:otherwise>
|
||||
<xsl:if test="$pos > 0">
|
||||
<xsl:text> </xsl:text>
|
||||
</xsl:if>
|
||||
<xsl:value-of select="$first-word" />
|
||||
<xsl:call-template name="wrap-string">
|
||||
<xsl:with-param name="str" select="substring-after( $str, ' ' )" />
|
||||
<xsl:with-param name="wrap-col" select="$wrap-col" />
|
||||
<xsl:with-param name="break-mark" select="$break-mark" />
|
||||
<xsl:with-param name="pos" select="$pos-now" />
|
||||
</xsl:call-template>
|
||||
</xsl:otherwise>
|
||||
</xsl:choose>
|
||||
</xsl:when>
|
||||
<xsl:otherwise>
|
||||
<xsl:choose>
|
||||
<xsl:when test="$pos + string-length( $str ) >= $wrap-col">
|
||||
<xsl:copy-of select="$break-mark" />
|
||||
</xsl:when>
|
||||
<xsl:otherwise>
|
||||
<xsl:if test="$pos > 0">
|
||||
<xsl:text> </xsl:text>
|
||||
</xsl:if>
|
||||
</xsl:otherwise>
|
||||
</xsl:choose>
|
||||
<xsl:value-of select="$str" />
|
||||
</xsl:otherwise>
|
||||
</xsl:choose>
|
||||
</xsl:template>
|
||||
|
||||
</xsl:stylesheet>
|
Loading…
Reference in New Issue
Block a user