Tweak pre tags in docs/hacking.html.in to achieve proper indentation of their plaintext representation. Also use more b/i/code tags in docs/hacking.html.in. --- HACKING | 602 +++++++++++++++++++++++++++++--------------------- Makefile.am | 8 + docs/hacking.html.in | 351 ++++++++++++++++-------------- docs/hacking1.xsl | 28 +++ docs/hacking2.xsl | 146 ++++++++++++ docs/wrapstring.xsl | 56 +++++ 6 files changed, 772 insertions(+), 419 deletions(-) create mode 100644 docs/hacking1.xsl create mode 100644 docs/hacking2.xsl create mode 100644 docs/wrapstring.xsl diff --git a/HACKING b/HACKING index a9a9b49..bf563c3 100644 --- a/HACKING +++ b/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: - http://et.redhat.com/~rjones/how-to-supply-code-to-open-source-projects/ +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. - -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. +- 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. + +- 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; + } + + + +- e.g. to allocate an array of objects + + virDomainPtr domains; + int ndomains = 10; + + if (VIR_ALLOC_N(domains, ndomains) < 0) { + virReportOOMError(); + return NULL; + } + + + +- e.g. to allocate an array of object pointers + + virDomainPtr *domains; + int ndomains = 10; + + if (VIR_ALLOC_N(domains, ndomains) < 0) { + virReportOOMError(); + return NULL; + } - if (VIR_ALLOC(domain) < 0) { - virReportOOMError(); - return NULL; - } - - eg to allocate an array of objects +- e.g. to re-allocate the array of domains to be longer - virDomainPtr domains; - int ndomains = 10; + ndomains = 20 - if (VIR_ALLOC_N(domains, ndomains) < 0) { - virReportOOMError(); - return NULL; - } + if (VIR_REALLOC_N(domains, ndomains) < 0) { + virReportOOMError(); + return NULL; + } - - eg 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 free the domain - - eg to re-allocate the array of domains to be longer + 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 + +- For strict equality: + + STREQ(a,b) + STRNEQ(a,b) + + + +- For case insensitive equality: + + STRCASEEQ(a,b) + STRCASENEQ(a,b) + + + +- For strict equality of a substring: + + STREQLEN(a,b,n) + STRNEQLEN(a,b,n) -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 insensitive equality of a substring: - - For case insensitive equality: - STRCASEEQ(a,b) - STRCASENEQ(a,b) + STRCASEEQLEN(a,b,n) + STRCASENEQLEN(a,b,n) - - For strict equality of a substring: - STREQLEN(a,b,n) - STRNEQLEN(a,b,n) - - For case insensitive equality of a substring: +- For strict equality of a prefix: + + STRPREFIX(a,b) - STRCASEEQLEN(a,b,n) - STRCASENEQLEN(a,b,n) - - For strict equality of a prefix: - 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: + + virStrncpy(char *dest, const char *src, size_t n, size_t destbytes) -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: +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. - - 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. + virStrcpy(char *dest, const char *src, size_t destbytes) - - 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) +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)). + 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,114 @@ 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 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: + + 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. + + +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. -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: +- 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. - int virAsprintf(char **strp, const char *fmt, ...) - ATTRIBUTE_FMT_PRINTF(2, 3); +- 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. -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. +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 - Libvirt committer guidelines - ============================ + KernelTrap + http://kerneltrap.org/node/553/2131 -The AUTHORS files indicates the list of people with commit access right -who can actually merge the patches. +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. + + + diff --git a/Makefile.am b/Makefile.am index d05b7da..915adbe 100644 --- a/Makefile.am +++ b/Makefile.am @@ -48,6 +48,8 @@ EXTRA_DIST = \ pkgconfigdir = $(libdir)/pkgconfig pkgconfig_DATA = libvirt.pc +all: NEWS HACKING + NEWS: $(top_srcdir)/docs/news.xsl $(top_srcdir)/docs/news.html.in -@(if [ -x $(XSLTPROC) ] ; then \ $(XSLTPROC) --nonet $(top_srcdir)/docs/news.xsl \ @@ -56,6 +58,12 @@ NEWS: $(top_srcdir)/docs/news.xsl $(top_srcdir)/docs/news.html.in | perl -pe 's/[ \t]+$$//' \ +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 - \ + > $@-t && mv $@-t $@ ; fi ); rpm: clean @(unset CDPATH ; $(MAKE) dist && rpmbuild -ta $(distdir).tar.gz) diff --git a/docs/hacking.html.in b/docs/hacking.html.in index bd8b443..88db01a 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -5,20 +5,21 @@ <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 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/ &gt; libvirt-myfeature.patch</pre> +<pre> + diff -urp libvirt.orig/ libvirt.modified/ &gt; libvirt-myfeature.patch +</pre> <p> or: </p> - <pre> - git diff > libvirt-myfeature.patch</pre> +<pre> + git diff > 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 @@ -27,35 +28,39 @@ 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. In particular, configure with compile warnings set to -Werror:</p> - <pre> - ./configure --enable-compile-warnings=error</pre> +<pre> + ./configure --enable-compile-warnings=error +</pre> <p> and run the tests: </p> - <pre> +<pre> make check make syntax-check - make -C tests valgrind</pre> + make -C tests valgrind +</pre> <p> The latter test checks for memory leaks. </p> - <p> - If you encounter any failing tests, the VIR_TEST_DEBUG - environment variable may provide extra information to debug - the failures. Larger values of VIR_TEST_DEBUG may provide - larger amounts of information: - </p> + <p> + If you encounter any failing tests, the VIR_TEST_DEBUG + environment variable may provide extra information to debug + the failures. Larger values of VIR_TEST_DEBUG may provide + larger amounts of information: + </p> - <pre> +<pre> VIR_TEST_DEBUG=1 make check (or) - VIR_TEST_DEBUG=2 make check</pre> - <p> - Also, individual tests can be run from inside the 'tests/' - directory, like: - </p> - <pre> - ./qemuxml2xmltest</pre> + VIR_TEST_DEBUG=2 make check +</pre> + <p> + Also, individual tests can be run from inside the <code>tests/</code> + directory, like: + </p> +<pre> + ./qemuxml2xmltest +</pre> </li> <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 (e.g., ~/.emacs), to help ensure that you get indentation right: </p> - <pre> +<pre> ;;; When editing C sources in libvirt, use this style. (defun libvirt-c-mode () "C mode with adjusted defaults for use with libvirt." @@ -105,7 +110,7 @@ around operators and keywords: </p> - <pre> +<pre> indent-libvirt() { indent -bad -bap -bbb -bli4 -br -ce -brs -cs -i4 -l75 -lc75 \ @@ -116,7 +121,7 @@ <p> 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 anyhow. </p> @@ -125,18 +130,20 @@ <h2><a name="curly_braces">Curly braces</a></h2> <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 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> Omitting braces with a single-line body is fine: </p> - <pre> +<pre> while (expr) // one-line body -> omitting curly braces is ok - single_line_stmt ();</pre> + single_line_stmt(); +</pre> <p> However, the moment your loop/if/else body extends onto a second @@ -146,26 +153,29 @@ it is already a multi-statement loop: </p> - <pre> +<pre> while (true) // BAD! multi-line body with no braces /* comment... */ - single_line_stmt ();</pre> + single_line_stmt(); +</pre> <p> Do this instead: </p> - <pre> +<pre> while (true) { // Always put braces around a multi-line body. /* comment... */ - single_line_stmt (); - }</pre> + single_line_stmt(); + } +</pre> <p> There is one exception: when the second body line is not at the same indentation level as the first body line: </p> - <pre> +<pre> if (expr) - die ("a diagnostic that would make this line" - " extend past the 80-column limit"));</pre> + die("a diagnostic that would make this line" + " extend past the 80-column limit")); +</pre> <p> It is safe to omit the braces in the code above, since the @@ -177,40 +187,44 @@ To reiterate, don't do this: </p> - <pre> +<pre> if (expr) // BAD: no braces around... while (expr_2) { // ... a multi-line body ... - }</pre> + } +</pre> <p> Do this, instead: </p> - <pre> +<pre> if (expr) { while (expr_2) { ... } - }</pre> + } +</pre> <p> 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 an "else" block, and the corresponding "then" block - *does* use braces. In that case, either put braces around the "else" - block, or negate the "if"-condition and swap the bodies, putting the + brace-less block is an <code>else</code> block, and the corresponding + <code>then</code> block <b>does</b> use braces. In that case, either + 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 - "else" block. + <code>else</code> block. </p> - <pre> +<pre> if (expr) { ... ... } else - x = y; // BAD: braceless "else" with braced "then"</pre> + x = y; // BAD: braceless "else" with braced "then" +</pre> <p> This is preferred, especially when the multi-line body is more than a @@ -219,43 +233,45 @@ after the more involved block: </p> - <pre> +<pre> if (!expr) x = y; // putting the smaller block first is more readable else { ... ... - }</pre> + } +</pre> <p> If you'd rather not negate the condition, then at least add braces: </p> - <pre> +<pre> if (expr) { ... ... } else { x = y; - }</pre> + } +</pre> <h2><a href="types">Preprocessor</a></h2> <p> For variadic macros, stick with C99 syntax: </p> - <pre> +<pre> #define vshPrint(_ctl, ...) fprintf(stdout, __VA_ARGS__) - </pre> +</pre> <p>Use parenthesis when checking if a macro is defined, and use indentation to track nesting: </p> - <pre> +<pre> #if defined(HAVE_POSIX_FALLOCATE) &amp;&amp; !defined(HAVE_FALLOCATE) # define fallocate(a,ignored,b,c) posix_fallocate(a,b,c) #endif - </pre> +</pre> <h2><a href="types">C types</a></h2> @@ -266,45 +282,51 @@ <h3>Scalars</h3> <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 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"; + <li>If it's memory-size-related, use <code>size_t</code> (use + <code>ssize_t</code> only if required).</li> + <li>If it's file-size related, use uintmax_t, or maybe <code>off_t</code>.</li> + <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 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 &lt;stdbool.h&gt;, since libvirt's use of gnulib ensures + <li>If a variable has boolean semantics, give it the <code>bool</code> type + and use the corresponding <code>true</code> and <code>false</code> macros. + It's ok to include &lt;stdbool.h&gt;, 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: + standard type like <code>int32_t</code>, <code>uint32_t</code>, + <code>uint64_t</code>, etc.</li> + <li>While using <code>bool</code> 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 + <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 - 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> + 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 <code>bool_t</code> + type, which <b>is</b> fixed-size.</li> + <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 <code>1</code>. + I.e., don't write <code>if (seen == true) ...</code>. Rather, + write <code>if (seen)...</code>.</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. + to use some system interface that requires a type like <code>size_t</code>, + <code>pid_t</code> or <code>off_t</code>, use matching types for any + corresponding variables. </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 - 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. </p> @@ -317,9 +339,9 @@ <h3>Pointers</h3> <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, - 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 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 @@ -336,57 +358,57 @@ </p> <ul> - <li><p>eg to allocate a single object:</p> - + <li><p>e.g. to allocate a single object:</p> <pre> - virDomainPtr domain; + virDomainPtr domain; - if (VIR_ALLOC(domain) &lt; 0) { - virReportOOMError(); - return NULL; - } -</pre></li> - - <li><p>eg to allocate an array of objects</p> + if (VIR_ALLOC(domain) &lt; 0) { + virReportOOMError(); + return NULL; + } +</pre> + </li> + <li><p>e.g. to allocate an array of objects</p> <pre> - virDomainPtr domains; - int ndomains = 10; - - if (VIR_ALLOC_N(domains, ndomains) &lt; 0) { - virReportOOMError(); - return NULL; - } -</pre></li> + virDomainPtr domains; + int ndomains = 10; - <li><p>eg to allocate an array of object pointers</p> + if (VIR_ALLOC_N(domains, ndomains) &lt; 0) { + virReportOOMError(); + return NULL; + } +</pre> + </li> + <li><p>e.g. to allocate an array of object pointers</p> <pre> - virDomainPtr *domains; - int ndomains = 10; + virDomainPtr *domains; + int ndomains = 10; - if (VIR_ALLOC_N(domains, ndomains) &lt; 0) { - virReportOOMError(); - return NULL; - } -</pre></li> - - <li><p>eg to re-allocate the array of domains to be longer</p> + if (VIR_ALLOC_N(domains, ndomains) &lt; 0) { + virReportOOMError(); + return NULL; + } +</pre> + </li> + <li><p>e.g. to re-allocate the array of domains to be longer</p> <pre> - ndomains = 20 - - if (VIR_REALLOC_N(domains, ndomains) &lt; 0) { - virReportOOMError(); - return NULL; - } -</pre></li> + ndomains = 20 - <li><p>eg to free the domain</p> + if (VIR_REALLOC_N(domains, ndomains) &lt; 0) { + virReportOOMError(); + return NULL; + } +</pre> + </li> + <li><p>e.g. to free the domain</p> <pre> - VIR_FREE(domain); -</pre></li> + VIR_FREE(domain); +</pre> + </li> </ul> <h2><a name="file_handling">File handling</a></h2> @@ -398,20 +420,21 @@ </p> <ul> - <li><p>eg close a file descriptor</p> - + <li><p>e.g. close a file descriptor</p> <pre> - if (VIR_CLOSE(fd) &lt; 0) { - virReportSystemError(errno, _("failed to close file")); - } -</pre></li> + if (VIR_CLOSE(fd) &lt; 0) { + virReportSystemError(errno, _("failed to close file")); + } +</pre> + </li> <li><p>eg close a file descriptor in an error path, without losing the previous errno value</p> <pre> - VIR_FORCE_CLOSE(fd); -</pre></li> + VIR_FORCE_CLOSE(fd); +</pre> + </li> </ul> <h2><a name="string_comparision">String comparisons</a></h2> @@ -423,39 +446,36 @@ <ul> <li><p>For strict equality:</p> - <pre> - STREQ(a,b) - STRNEQ(a,b) +<pre> + STREQ(a,b) + STRNEQ(a,b) </pre> </li> <li><p>For case insensitive equality:</p> - <pre> - STRCASEEQ(a,b) - STRCASENEQ(a,b) +<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> + STREQLEN(a,b,n) + STRNEQLEN(a,b,n) </pre> </li> <li><p>For case insensitive equality of a substring:</p> - - <pre> - STRCASEEQLEN(a,b,n) - STRCASENEQLEN(a,b,n) +<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> + STRPREFIX(a,b) </pre> </li> </ul> @@ -469,7 +489,10 @@ it extremely dangerous to use. Instead, use one of the functionally equivalent functions: </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> The first three arguments have the same meaning as for strncpy; namely the destination, source, and number of bytes to copy, @@ -481,8 +504,9 @@ trailing \0 is appended. </p> - <pre>virStrcpy(char *dest, const char *src, size_t destbytes)</pre> - +<pre> + virStrcpy(char *dest, const char *src, size_t destbytes) +</pre> <p> 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 @@ -490,11 +514,12 @@ virStrncpy(dest, src, strlen(src), destbytes) </p> - <pre>virStrcpyStatic(char *dest, const char *src)</pre> - +<pre> + virStrcpyStatic(char *dest, const char *src) +</pre> <p> 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 meaningful). Note that this is a macro, so arguments could be evaluated more than once. This is equivalent to @@ -511,9 +536,10 @@ <p>eg typical usage is as follows:</p> - <pre> +<pre> char * - somefunction(...) { + somefunction(...) + { virBuffer buf = VIR_BUFFER_INITIALIZER; ... @@ -545,7 +571,7 @@ *.c source files: </p> - <pre> +<pre> /* * Copyright notice * .... @@ -561,7 +587,7 @@ #include &lt;limits.h&gt; #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 #include "internal.h" Include this first, after system includes. @@ -569,13 +595,13 @@ #include "util.h" Any libvirt internal header files. #include "buf.h" - static myInternalFunc () The actual code. + static myInternalFunc() The actual code. { - ... + ... </pre> <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 are some special reasons why you cannot include these files explicitly. @@ -591,9 +617,9 @@ the one for virAsprintf, in util.h: </p> - <pre> - int virAsprintf(char **strp, const char *fmt, ...) - ATTRIBUTE_FORMAT(printf, 2, 3); +<pre> + int virAsprintf(char **strp, const char *fmt, ...) + ATTRIBUTE_FORMAT(printf, 2, 3); </pre> <p> @@ -654,7 +680,7 @@ 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 - <a href=http://kerneltrap.org/node/553/2131>KernelTrap</a> + <a href="http://kerneltrap.org/node/553/2131">KernelTrap</a> </p> <p> @@ -662,11 +688,12 @@ makes sense: </p> - <pre> +<pre> 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)</pre> + retry: If needing to jump upwards (eg retry on EINTR) +</pre> @@ -691,7 +718,7 @@ configure with </p> <pre> - --enable-compile-warnings=error + --enable-compile-warnings=error </pre> <p> which adds -Werror to compile flags, so no warnings get missed diff --git a/docs/hacking1.xsl b/docs/hacking1.xsl new file mode 100644 index 0000000..982fa8d --- /dev/null +++ b/docs/hacking1.xsl @@ -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> diff --git a/docs/hacking2.xsl b/docs/hacking2.xsl new file mode 100644 index 0000000..89e777b --- /dev/null +++ b/docs/hacking2.xsl @@ -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(.,'&#xA;')"><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> diff --git a/docs/wrapstring.xsl b/docs/wrapstring.xsl new file mode 100644 index 0000000..b468a70 --- /dev/null +++ b/docs/wrapstring.xsl @@ -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> -- 1.7.0.4