[libvirt PATCH 01/10] docs: Convert hacking.html to reStructuredText
Andrea Bolognani
abologna at redhat.com
Mon Apr 6 16:20:01 UTC 2020
The conversion has been performed by using pandoc as a first pass,
and then tweaking the result manually until it looked satisfactory.
Signed-off-by: Andrea Bolognani <abologna at redhat.com>
---
build-aux/syntax-check.mk | 4 +-
docs/hacking.html.in | 1555 -------------------------------------
docs/hacking.rst | 1400 +++++++++++++++++++++++++++++++++
3 files changed, 1402 insertions(+), 1557 deletions(-)
delete mode 100644 docs/hacking.html.in
create mode 100644 docs/hacking.rst
diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index c7c938ac92..6ffea7afb9 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -2040,7 +2040,7 @@ exclude_file_name_regexp--sc_prohibit_canonicalize_file_name = \
^(build-aux/syntax-check\.mk|tests/virfilemock\.c)$$
exclude_file_name_regexp--sc_prohibit_raw_allocation = \
- ^(docs/hacking\.html\.in|src/util/viralloc\.[ch]|examples/.*|tests/(securityselinuxhelper|(vircgroup|nss)mock|commandhelper)\.c|tools/wireshark/src/packet-libvirt\.c|tools/nss/libvirt_nss(_leases|_macs)?\.c|build-aux/useless-if-before-free)$$
+ ^(docs/hacking\.rst|src/util/viralloc\.[ch]|examples/.*|tests/(securityselinuxhelper|(vircgroup|nss)mock|commandhelper)\.c|tools/wireshark/src/packet-libvirt\.c|tools/nss/libvirt_nss(_leases|_macs)?\.c|build-aux/useless-if-before-free)$$
exclude_file_name_regexp--sc_prohibit_readlink = \
^src/(util/virutil|lxc/lxc_container)\.c$$
@@ -2048,7 +2048,7 @@ exclude_file_name_regexp--sc_prohibit_readlink = \
exclude_file_name_regexp--sc_prohibit_setuid = ^src/util/virutil\.c|tools/virt-login-shell\.c$$
exclude_file_name_regexp--sc_prohibit_snprintf = \
- ^(build-aux/syntax-check\.mk|docs/hacking\.html\.in|tools/virt-login-shell\.c)$$
+ ^(build-aux/syntax-check\.mk|docs/hacking\.rst|tools/virt-login-shell\.c)$$
exclude_file_name_regexp--sc_prohibit_strtol = ^examples/.*$$
diff --git a/docs/hacking.html.in b/docs/hacking.html.in
deleted file mode 100644
index 1756e84fc4..0000000000
--- a/docs/hacking.html.in
+++ /dev/null
@@ -1,1555 +0,0 @@
-<?xml version="1.0" encoding="UTF-8"?>
-<!DOCTYPE html>
-<html xmlns="http://www.w3.org/1999/xhtml">
- <body>
- <h1>Contributor guidelines</h1>
-
- <ul id="toc"></ul>
-
- <h2><a id="patches">General tips for contributing patches</a></h2>
- <ol>
- <li>
- <p>Discuss any large changes on the mailing list first. Post patches
- early and listen to feedback.</p>
- </li>
-
- <li>
- <p>Official upstream repository is kept in git
- (<code>https://libvirt.org/git/libvirt.git</code>) and is browsable
- along with other libvirt-related repositories
- (e.g. libvirt-python) <a href="https://libvirt.org/git/">online</a>.</p>
- </li>
-
- <li>
- <p>Patches to translations are maintained via
- the <a href="https://fedora.zanata.org/">zanata project</a>.
- If you want to fix a translation in a .po file, join the
- appropriate language team. The libvirt release process
- automatically pulls the latest version of each translation
- file from zanata.</p>
- </li>
-
- <li><p>The simplest way to send patches is to use the
- <a href="https://github.com/stefanha/git-publish"><code>git-publish</code></a>
- tool. All libvirt-related repositories contain a config file that
- tells git-publish to use the correct mailing list and subject prefix.</p>
- <p>Alternatively, you may send patches using <code>git send-email</code>.</p>
- <p>Also, for code motion patches, you may find that <code>git
- diff --patience</code> provides an easier-to-read patch.
- However, the usual workflow of libvirt developer is:</p>
-<pre>
- git checkout master
- git pull
- git checkout -t origin -b workbranch
- Hack, committing any changes along the way
-</pre>
- <p>More hints on compiling can be
- found <a href="compiling.html">here</a>. When you want to
- post your patches:</p>
-<pre>
- git pull --rebase
- (fix any conflicts)
- git send-email --cover-letter --no-chain-reply-to --annotate \
- --confirm=always --to=libvir-list at redhat.com master
-</pre>
- <p>For a single patch you can omit
- <code>--cover-letter</code>, but a series of two or more
- patches needs a cover letter.</p>
- <p>Note that the <code>git send-email</code> subcommand may not
- be in the main git package and using it may require installation
- of a separate package, for example the "git-email" package in
- Fedora and Debian. If this is your first time using
- <code>git send-email</code>, you might need to configure it to
- point it to your SMTP server with something like:</p>
-<pre>
- git config --global sendemail.smtpServer stmp.youremailprovider.net
-</pre>
- <p>If you get tired of typing
- <code>--to=libvir-list at redhat.com</code> all the time, you can
- configure that to be automatically handled as well:</p>
-<pre>
- git config sendemail.to libvir-list at redhat.com
-</pre>
- <p>As a rule, patches should be sent to the mailing list only: all
- developers are subscribed to libvir-list and read it regularly, so
- <strong>please don't CC individual developers</strong> unless
- they've explicitly asked you to.</p>
- <p>Avoid using mail clients for sending patches, as most of them
- will mangle the messages in some way, making them unusable for our
- purposes. Gmail and other Web-based mail clients are particularly
- bad at this.</p>
- <p>If everything went well, your patch should show up on the
- <a href="https://www.redhat.com/archives/libvir-list/">libvir-list
- archives</a> in a matter of minutes; if you still can't find it on
- there after an hour or so, you should double-check your setup.
- <strong>Note that, if you are not already a subscriber, your very
- first post to the mailing list will be
- subject to moderation</strong>, and it's not uncommon for that to
- take around a day.</p>
- <p>Please follow this as close as you can, especially the rebase and
- <code>git send-email</code> part, as it makes life easier for other
- developers to review your patch set.</p>
- <p>One should avoid sending patches as attachments,
- but rather send them in email body along with commit message. If a
- developer is sending another version of the patch (e.g. to address
- review comments), they are advised to note differences to previous
- versions after the <code>---</code> line in the patch so that it helps
- reviewers but doesn't become part of git history. Moreover, such patch
- needs to be prefixed correctly with
- <code>--subject-prefix=PATCHv2</code> appended to <code>git
- send-email</code> (substitute <code>v2</code> with the correct
- version if needed though).</p>
- </li>
-
- <li><p>In your commit message, make the summary line reasonably
- short (60 characters is typical), followed by a blank line,
- followed by any longer description of why your patch makes
- sense. If the patch fixes a regression, and you know what
- commit introduced the problem, mentioning that is useful.
- If the patch resolves a bugzilla report, mentioning the URL
- of the bug number is useful; but also summarize the issue
- rather than making all readers follow the link. You can use
- 'git shortlog -30' to get an idea of typical summary lines.
- </p>
- </li>
-
- <li><p>Contributors to libvirt projects <strong>must</strong>
- assert that they are in compliance with the
- <a href="https://developercertificate.org/">Developer
- Certificate of Origin 1.1</a>. This is achieved by adding
- a "Signed-off-by" line containing the contributor's name
- and e-mail to every commit message. The presence
- of this line attests that the contributor has read the
- above lined DCO and agrees with its statements.
- </p></li>
-
- <li><p>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. Moreover, please keep in mind that it's required to
- be able to compile cleanly (<b>including</b> <code>make
- check</code> and <code>make syntax-check</code>) after each
- patch. A feature does not have to work until the end of a
- series, but intermediate patches must compile and not cause
- test-suite failures (this is to preserve the usefulness
- of <code>git bisect</code>, among other things).</p>
- </li>
-
- <li>
- <p>Make sure your patches apply against libvirt GIT. Developers
- only follow GIT and don't care much about released versions.</p>
- </li>
-
- <li><p>Run the automated tests on your code before submitting any changes.
- That is:
- </p>
-<pre>
- make check
- make syntax-check
- make -C tests valgrind
-</pre>
- <p><a href="http://valgrind.org/">Valgrind</a> is a test that checks
- for memory management issues, such as leaks or use of uninitialized
- variables.
- </p>
-
- <p>
- Some tests are skipped by default in a development environment,
- based on the time they take in comparison to the likelihood
- that those tests will turn up problems during incremental builds.
- These tests default to being run when building from a
- tarball or with the configure option --enable-expensive-tests;
- you can also force a one-time toggle of these tests by
- setting VIR_TEST_EXPENSIVE to 0 or 1 at make time, as in:
- </p>
-<pre>
- make check VIR_TEST_EXPENSIVE=1
-</pre>
- <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>
- VIR_TEST_DEBUG=1 make check (or)
- VIR_TEST_DEBUG=2 make check
-</pre>
-
- <p>
- When debugging failures during development, it is possible
- to focus in on just the failing subtests by using
- VIR_TEST_RANGE. I.e. to run all tests from 3 to 20 with the
- exception of tests 6 and 16, use:
- </p>
-
-<pre>
- VIR_TEST_DEBUG=1 VIR_TEST_RANGE=3-5,7-20,^16 ./run tests/qemuxml2argvtest
-</pre>
-
- <p>
- Also, individual tests can be run from inside the <code>tests/</code>
- directory, like:
- </p>
-<pre>
- ./qemuxml2xmltest
-</pre>
-
- <p>
- If you are adding new test cases, or making changes that alter
- existing test output, you can use the environment variable
- VIR_TEST_REGENERATE_OUTPUT to quickly update the saved test data.
- Of course you still need to review the changes VERY CAREFULLY to
- ensure they are correct.
- </p>
-<pre>
- VIR_TEST_REGENERATE_OUTPUT=1 ./qemuxml2argvtest
-</pre>
-
- <p>There is also a <code>./run</code> script at the top level,
- to make it easier to run programs that have not yet been
- installed, as well as to wrap invocations of various tests
- under gdb or Valgrind.
- </p>
-
- <p>When running our test suite it may happen that the test result is
- nondeterministic because of the test suite relying on a particular file
- in the system being accessible or having some specific value. To catch
- this kind of errors, the test suite has a module for that prints any
- path touched that fulfils constraints described above
- into a file. To enable it just set
- <code>VIR_TEST_FILE_ACCESS</code> environment variable.
- Then <code>VIR_TEST_FILE_ACCESS_OUTPUT</code> environment
- variable can alter location where the file is stored.</p>
-<pre>
- VIR_TEST_FILE_ACCESS=1 VIR_TEST_FILE_ACCESS_OUTPUT="/tmp/file_access.txt" ./qemuxml2argvtest
-</pre>
-
- </li>
- <li><p>The Valgrind test should produce similar output to
- <code>make check</code>. If the output has traces within libvirt
- API's, then investigation is required in order to determine the
- cause of the issue. Output such as the following indicates some
- sort of leak:
- </p>
-<pre>
-==5414== 4 bytes in 1 blocks are definitely lost in loss record 3 of 89
-==5414== at 0x4A0881C: malloc (vg_replace_malloc.c:270)
-==5414== by 0x34DE0AAB85: xmlStrndup (in /usr/lib64/libxml2.so.2.7.8)
-==5414== by 0x4CC97A6: virDomainVideoDefParseXML (domain_conf.c:7410)
-==5414== by 0x4CD581D: virDomainDefParseXML (domain_conf.c:10188)
-==5414== by 0x4CD8C73: virDomainDefParseNode (domain_conf.c:10640)
-==5414== by 0x4CD8DDB: virDomainDefParse (domain_conf.c:10590)
-==5414== by 0x41CB1D: testCompareXMLToArgvHelper (qemuxml2argvtest.c:100)
-==5414== by 0x41E20F: virtTestRun (testutils.c:161)
-==5414== by 0x41C7CB: mymain (qemuxml2argvtest.c:866)
-==5414== by 0x41E84A: virtTestMain (testutils.c:723)
-==5414== by 0x34D9021734: (below main) (in /usr/lib64/libc-2.15.so)
-</pre>
- <p>In this example, the <code>virDomainDefParseXML()</code> had
- an error path where the <code>virDomainVideoDefPtr video</code>
- pointer was not properly disposed. By simply adding a
- <code>virDomainVideoDefFree(video);</code> in the error path,
- the issue was resolved.
- </p>
-
- <p>Another common mistake is calling a printing function, such as
- <code>VIR_DEBUG()</code> without initializing a variable to be
- printed. The following example involved a call which could return
- an error, but not set variables passed by reference to the call.
- The solution was to initialize the variables prior to the call.
- </p>
-<pre>
-==4749== Use of uninitialised value of size 8
-==4749== at 0x34D904650B: _itoa_word (in /usr/lib64/libc-2.15.so)
-==4749== by 0x34D9049118: vfprintf (in /usr/lib64/libc-2.15.so)
-==4749== by 0x34D9108F60: __vasprintf_chk (in /usr/lib64/libc-2.15.so)
-==4749== by 0x4CAEEF7: virVasprintf (stdio2.h:199)
-==4749== by 0x4C8A55E: virLogVMessage (virlog.c:814)
-==4749== by 0x4C8AA96: virLogMessage (virlog.c:751)
-==4749== by 0x4DA0056: virNetTLSContextCheckCertKeyUsage (virnettlscontext.c:225)
-==4749== by 0x4DA06DB: virNetTLSContextCheckCert (virnettlscontext.c:439)
-==4749== by 0x4DA1620: virNetTLSContextNew (virnettlscontext.c:562)
-==4749== by 0x4DA26FC: virNetTLSContextNewServer (virnettlscontext.c:927)
-==4749== by 0x409C39: testTLSContextInit (virnettlscontexttest.c:467)
-==4749== by 0x40AB8F: virtTestRun (testutils.c:161)
-</pre>
- <p>Valgrind will also find some false positives or code paths
- which cannot be resolved by making changes to the libvirt code.
- For these paths, it is possible to add a filter to avoid the
- errors. For example:
- </p>
-<pre>
-==4643== 7 bytes in 1 blocks are possibly lost in loss record 4 of 20
-==4643== at 0x4A0881C: malloc (vg_replace_malloc.c:270)
-==4643== by 0x34D90853F1: strdup (in /usr/lib64/libc-2.15.so)
-==4643== by 0x34EEC2C08A: ??? (in /usr/lib64/libnl.so.1.1)
-==4643== by 0x34EEC15B81: ??? (in /usr/lib64/libnl.so.1.1)
-==4643== by 0x34D8C0EE15: call_init.part.0 (in /usr/lib64/ld-2.15.so)
-==4643== by 0x34D8C0EECF: _dl_init (in /usr/lib64/ld-2.15.so)
-==4643== by 0x34D8C01569: ??? (in /usr/lib64/ld-2.15.so)
-
-</pre>
- <p>In this instance, it is acceptable to modify the
- <code>tests/.valgrind.supp</code> file in order to add a
- suppression filter. The filter should be unique enough to
- not suppress real leaks, but it should be generic enough to
- cover multiple code paths. The format of the entry can be
- found in the documentation found at the
- <a href="http://valgrind.org/">Valgrind home page</a>.
- The following trace was added to <code>tests/.valgrind.supp</code>
- in order to suppress the warning:
- </p>
-<pre>
-{
- dlInitMemoryLeak1
- Memcheck:Leak
- fun:?alloc
- ...
- fun:call_init.part.0
- fun:_dl_init
- ...
- obj:*/lib*/ld-2.*so*
-}
-</pre>
- </li>
-
- <li>
- <p>Update tests and/or documentation, particularly if you are adding
- a new feature or changing the output of a program.</p>
- </li>
-
- <li>
- <p>Don't forget to update the <a href="news.html">release notes</a>
- by changing <code>docs/news.xml</code> if your changes are
- significant. All user-visible changes, such as adding new XML elements
- or fixing all but the most obscure bugs, must be (briefly) described
- in a release notes entry; changes that are only relevant to other
- libvirt developers, such as code refactoring, don't belong in the
- release notes. Note that <code>docs/news.xml</code> should be updated
- in its own commit not to get in the way of backports.</p>
- </li>
- </ol>
-
- <p>
- There is more on this subject, including lots of links to background
- reading on the subject, on
- <a href="http://people.redhat.com/rjones/how-to-supply-code-to-open-source-projects/">
- Richard Jones' guide to working with open source projects</a>.
- </p>
-
- <h2><a id="lang">Language Usage</a></h2>
-
- <p>
- The libvirt repository makes use of a large number of programming
- languages. It is anticipated that in the future libvirt will adopt
- use of other new languages. To reduce the overall burden on developers,
- there is thus a general desire to phase out usage of some of the
- existing languages.
- </p>
-
- <p>
- The preferred languages at this time are:
- </p>
-
- <ul>
- <li>C - for the main libvirt codebase. Dialect supported by
- GCC/CLang only.</li>
- <li>Python - for supporting build scripts / tools. Code must
- run with both version 2.7 and 3.x at this time.</li>
- </ul>
-
- <p>
- Languages that should not be used for any new contributions:
- </p>
-
- <ul>
- <li>Perl - build scripts must be written in Python instead.</li>
- <li>Shell - build scripts must be written in Python instead.</li>
- </ul>
-
- <h2><a id="tooling">Tooling</a></h2>
-
- <p>
- libvirt includes support for some useful development tools right in its
- source repository, meaning users will be able to take advantage of them
- without little or no configuration. Examples include:
- </p>
-
- <ul>
- <li>
- <a href="https://github.com/jeaye/color_coded">color_coded</a>,
- a vim plugin for libclang-powered semantic syntax highlighting;
- </li>
-
- <li>
- <a href="http://valloric.github.io/YouCompleteMe/">YouCompleteMe</a>,
- a vim plugin for libclang-powered semantic code completion.
- </li>
- </ul>
-
- <h2><a id="naming">Naming conventions</a></h2>
-
- <p>
- When reading libvirt code, a number of different naming conventions will
- be evident due to various changes in thinking over the course of the
- project's lifetime. The conventions documented below should be followed
- when creating any entirely new files in libvirt. When working on existing
- files, while it is desirable to apply these conventions, keeping a
- consistent style with existing code in that particular file is generally
- more important. The overall guiding principal is that every file, enum,
- struct, function, macro and typedef name must have a 'vir' or 'VIR' prefix.
- All local scope variable names are exempt, and global variables are exempt,
- unless exported in a header file.
- </p>
-
- <dl>
- <dt>File names</dt>
- <dd>
- <p>
- File naming varies depending on the subdirectory. The preferred
- style is to have a 'vir' prefix, followed by a name which matches
- the name of the functions / objects inside the file. For example,
- a file containing an object 'virHashtable' is stored in files
- 'virhashtable.c' and 'virhashtable.h'. Sometimes, methods which
- would otherwise be declared 'static' need to be exported for use
- by a test suite. For this purpose a second header file should be
- added with a suffix of 'priv', e.g. 'virhashtablepriv.h'. Use of
- underscores in file names is discouraged when using the 'vir'
- prefix style. The 'vir' prefix naming applies to src/util,
- src/rpc and tests/ directories. Most other directories do not
- follow this convention.
- </p>
- </dd>
- <dt>Enum type & field names</dt>
- <dd>
- <p>
- All enums should have a 'vir' prefix in their typedef name,
- and each following word should have its first letter in
- uppercase. The enum name should match the typedef name with
- a leading underscore. The enum member names should be in all
- uppercase, and use an underscore to separate each word. The
- enum member name prefix should match the enum typedef name.
- </p>
- <pre>
- typedef enum _virSocketType virSocketType;
- enum _virSocketType {
- VIR_SOCKET_TYPE_IPV4,
- VIR_SOCKET_TYPE_IPV6,
- };</pre>
- </dd>
- <dt>Struct type names</dt>
- <dd>
- <p>
- All structs should have a 'vir' prefix in their typedef name,
- and each following word should have its first letter in
- uppercase. The struct name should be the same as the typedef
- name with a leading underscore. A second typedef should be
- given for a pointer to the struct with a 'Ptr' suffix.
- </p>
- <pre>
- typedef struct _virHashTable virHashTable;
- typedef virHashTable *virHashTablePtr;
- struct _virHashTable {
- ...
- };</pre>
- </dd>
- <dt>Function names</dt>
- <dd>
- <p>
- All functions should have a 'vir' prefix in their name,
- followed by one or more words with first letter of each
- word capitalized. Underscores should not be used in function
- names. If the function is operating on an object, then the
- function name prefix should match the object typedef name,
- otherwise it should match the filename. Following this
- comes the verb / action name, and finally an optional
- subject name. For example, given an object 'virHashTable',
- all functions should have a name 'virHashTable$VERB' or
- 'virHashTable$VERB$SUBJECT", e.g. 'virHashTableLookup'
- or 'virHashTableGetValue'.
- </p>
- </dd>
- <dt>Macro names</dt>
- <dd>
- <p>
- All macros should have a "VIR" prefix in their name, followed
- by one or more uppercase words separated by underscores. The
- macro argument names should be in lowercase. Aside from having
- a "VIR" prefix there are no common practices for the rest of
- the macro name.
- </p>
- </dd>
- </dl>
-
- <h2><a id="indent">Code indentation</a></h2>
- <p>
- Libvirt's C source code generally adheres to some basic code-formatting
- conventions. The existing code base is not totally consistent on this
- front, but we do prefer that contributed code be formatted similarly.
- In short, use spaces-not-TABs for indentation, use 4 spaces for each
- indentation level, and other than that, follow the K&R style.
- </p>
-
- <p>
- If you use Emacs, the project includes a file .dir-locals.el
- that sets up the preferred indentation. If you use vim,
- append the following to your ~/.vimrc file:
- </p>
-<pre>
- set nocompatible
- filetype on
- set autoindent
- set smartindent
- set cindent
- set tabstop=8
- set shiftwidth=4
- set expandtab
- set cinoptions=(0,:0,l1,t0,L3
- filetype plugin indent on
- au FileType make setlocal noexpandtab
- au BufRead,BufNewFile *.am setlocal noexpandtab
- match ErrorMsg /\s\+$\| \+\ze\t/
-</pre>
- <p>
- Or if you don't want to mess your ~/.vimrc up, you can save the above
- into a file called .lvimrc (not .vimrc) located at the root of libvirt
- source, then install a vim script from
- http://www.vim.org/scripts/script.php?script_id=1408,
- which will load the .lvimrc only when you edit libvirt code.
- </p>
-
- <h2><a id="formatting">Code formatting (especially for new code)</a></h2>
-
- <p>
- With new code, we can be even more strict.
- Please apply the following function (using GNU indent) to any new code.
- Note that this also gives you an idea of the type of spacing we prefer
- around operators and keywords:
- </p>
-
-<pre>
- indent-libvirt()
- {
- indent -bad -bap -bbb -bli4 -br -ce -brs -cs -i4 -l75 -lc75 \
- -sbi4 -psl -saf -sai -saw -sbi4 -ss -sc -cdw -cli4 -npcs -nbc \
- --no-tabs "$@"
- }
-</pre>
-
- <p>
- Note that sometimes you'll have to post-process that output further, by
- 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>
-
- <p>
- Libvirt requires a C99 compiler for various reasons. However,
- most of the code base prefers to stick to C89 syntax unless
- there is a compelling reason otherwise. For example, it is
- preferable to use <code>/* */</code> comments rather
- than <code>//</code>. Also, when declaring local variables, the
- prevailing style has been to declare them at the beginning of a
- scope, rather than immediately before use.
- </p>
-
-
- <h2><a id="bracket_spacing">Bracket spacing</a></h2>
-
- <p>
- The keywords <code>if</code>, <code>for</code>, <code>while</code>,
- and <code>switch</code> must have a single space following them
- before the opening bracket. E.g.
- </p>
- <pre>
- if(foo) // Bad
- if (foo) // Good
-</pre>
-
- <p>
- Function implementations must <strong>not</strong> have any whitespace
- between the function name and the opening bracket. E.g.
- </p>
- <pre>
- int foo (int wizz) // Bad
- int foo(int wizz) // Good
-</pre>
-
- <p>
- Function calls must <strong>not</strong> have any whitespace
- between the function name and the opening bracket. E.g.
- </p>
- <pre>
- bar = foo (wizz); // Bad
- bar = foo(wizz); // Good
-</pre>
-
- <p>
- Function typedefs must <strong>not</strong> have any whitespace
- between the closing bracket of the function name and opening
- bracket of the arg list. E.g.
- </p>
- <pre>
- typedef int (*foo) (int wizz); // Bad
- typedef int (*foo)(int wizz); // Good
-</pre>
-
- <p>
- There must not be any whitespace immediately following any
- opening bracket, or immediately prior to any closing bracket. E.g.
- </p>
- <pre>
- int foo( int wizz ); // Bad
- int foo(int wizz); // Good
-</pre>
-
- <h2><a id="comma">Commas</a></h2>
-
- <p>
- Commas should always be followed by a space or end of line, and
- never have leading space; this is enforced during 'make
- syntax-check'.
- </p>
- <pre>
- call(a,b ,c);// Bad
- call(a, b, c); // Good
-</pre>
-
- <p>
- When declaring an enum or using a struct initializer that
- occupies more than one line, use a trailing comma. That way,
- future edits to extend the list only have to add a line, rather
- than modify an existing line to add the intermediate comma. Any
- sentinel enumerator value with a name ending in _LAST is exempt,
- since you would extend such an enum before the _LAST element.
- Another reason to favor trailing commas is that it requires less
- effort to produce via code generators. Note that the syntax
- checker is unable to enforce a style of trailing commas, so
- there are counterexamples in existing code which do not use it;
- also, while C99 allows trailing commas, remember that JSON and
- XDR do not.
- </p>
- <pre>
- enum {
- VALUE_ONE,
- VALUE_TWO // Bad
- };
- enum {
- VALUE_THREE,
- VALUE_FOUR, // Good
- };
-</pre>
-
- <h2><a id="semicolon">Semicolons</a></h2>
-
- <p>
- Semicolons should never have a space beforehand. Inside the
- condition of a <code>for</code> loop, there should always be a
- space or line break after each semicolon, except for the special
- case of an infinite loop (although more infinite loops
- use <code>while</code>). While not enforced, loop counters
- generally use post-increment.
- </p>
- <pre>
- for (i = 0 ;i < limit ; ++i) { // Bad
- for (i = 0; i < limit; i++) { // Good
- for (;;) { // ok
- while (1) { // Better
-</pre>
- <p>
- Empty loop bodies are better represented with curly braces and a
- comment, although use of a semicolon is not currently rejected.
- </p>
- <pre>
- while ((rc = waitpid(pid, &st, 0) == -1) &&
- errno == EINTR); // ok
- while ((rc = waitpid(pid, &st, 0) == -1) &&
- errno == EINTR) { // Better
- /* nothing */
- }
-</pre>
-
- <h2><a id="curly_braces">Curly braces</a></h2>
-
- <p>
- Omit the curly braces around an <code>if</code>, <code>while</code>,
- <code>for</code> etc. body only when both that body and the condition
- itself occupy a single line. In every other case we require
- the braces. This ensures that it is trivially easy to identify a
- single-<i>statement</i> loop: each has only one <i>line</i> in its body.
- </p>
-
-<pre>
- while (expr) // single line body; {} is forbidden
- single_line_stmt();
-</pre>
-
-<pre>
- while (expr(arg1,
- arg2)) // indentation makes it obvious it is single line,
- single_line_stmt(); // {} is optional (not enforced either way)
-</pre>
-
-<pre>
- while (expr1 &&
- expr2) { // multi-line, at same indentation, {} required
- single_line_stmt();
- }
-</pre>
-
- <p>
- However, the moment your loop/if/else body extends on to 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:
- </p>
-
-<pre>
- while (true) // BAD! multi-line body with no braces
- /* comment... */
- single_line_stmt();
-</pre>
- <p>
- Do this instead:
- </p>
-<pre>
- while (true) { // Always put braces around a multi-line body.
- /* comment... */
- 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>
- if (expr)
- 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
- further-indented second body line makes it obvious that this is still
- a single-statement body.
- </p>
-
- <p>
- To reiterate, don't do this:
- </p>
-
-<pre>
- if (expr) // BAD: no braces around...
- while (expr_2) { // ... a multi-line body
- ...
- }
-</pre>
-
- <p>
- Do this, instead:
- </p>
-
-<pre>
- if (expr) {
- while (expr_2) {
- ...
- }
- }
-</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 <code>if</code> or <code>else</code>
- block, and the counterpart block <b>does</b> use braces. In
- that case, put braces around both blocks. Also, if
- the <code>else</code> block is much shorter than
- the <code>if</code> block, consider negating the
- <code>if</code>-condition and swapping the bodies, putting the
- short block first and making the longer, multi-line block be the
- <code>else</code> block.
- </p>
-
-<pre>
- if (expr) {
- ...
- ...
- }
- else
- x = y; // BAD: braceless "else" with braced "then",
- // and short block last
-
- if (expr)
- x = y; // BAD: braceless "if" with braced "else"
- else {
- ...
- ...
- }
-</pre>
-
- <p>
- Keeping braces consistent and putting the short block first 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 more involved block:
- </p>
-
-<pre>
- if (!expr) {
- x = y; // putting the smaller block first is more readable
- } else {
- ...
- ...
- }
-</pre>
-
- <p>
- But if negating a complex condition is too ugly, then at least
- add braces:
- </p>
-
-<pre>
- if (complex expr not worth negating) {
- ...
- ...
- } else {
- x = y;
- }
-</pre>
-
- <p>Use hanging braces for compound statements: the opening brace
- of a compound statement should be on the same line as the
- condition being tested. Only top-level function bodies, nested
- scopes, and compound structure declarations should ever have {
- on a line by itself.
- </p>
-
-<pre>
- void
- foo(int a, int b)
- { // correct - function body
- int 2d[][] = {
- { // correct - complex initialization
- 1, 2,
- },
- };
- if (a)
- { // BAD: compound brace on its own line
- do_stuff();
- }
- { // correct - nested scope
- int tmp;
- if (a < b) { // correct - hanging brace
- tmp = b;
- b = a;
- a = tmp;
- }
- }
- }
-</pre>
-
- <h2><a id="conditions">Conditional expressions</a></h2>
- <p>For readability reasons new code should avoid shortening comparisons
- to 0 for numeric types. Boolean and pointer comparisions may be
- shortened. All long forms are okay:
- </p>
-<pre>
- virFooPtr foos = NULL;
- size nfoos = 0;
- bool hasFoos = false;
-
-GOOD:
- if (!foos)
- if (!hasFoos)
- if (nfoos == 0)
- if (foos == NULL)
- if (hasFoos == true)
-
-BAD:
- if (!nfoos)
- if (nfoos)
-</pre>
- <p>New code should avoid the ternary operator as much as possible.
- Specifically it must never span more than one line or nest:
- </p>
-<pre>
-BAD:
- char *foo = baz ?
- virDoSomethingReallyComplex(driver, vm, something, baz->foo) :
- NULL;
-
- char *foo = bar ? bar->baz ? bar->baz->foo : "nobaz" : "nobar";
-</pre>
-
- <h2><a id="preprocessor">Preprocessor</a></h2>
-
- <p>Macros defined with an ALL_CAPS name should generally be
- assumed to be unsafe with regards to arguments with side-effects
- (that is, MAX(a++, b--) might increment a or decrement b too
- many or too few times). Exceptions to this rule are explicitly
- documented for macros in viralloc.h and virstring.h.
- </p>
-
- <p>
- For variadic macros, stick with C99 syntax:
- </p>
-<pre>
- #define vshPrint(_ctl, ...) fprintf(stdout, __VA_ARGS__)
-</pre>
-
- <p>Use parenthesis when checking if a macro is defined, and use
- indentation to track nesting:
- </p>
-<pre>
- #if defined(HAVE_POSIX_FALLOCATE) && !defined(HAVE_FALLOCATE)
- # define fallocate(a, ignored, b, c) posix_fallocate(a, b, c)
- #endif
-</pre>
-
- <h2><a id="types">C types</a></h2>
-
- <p>
- Use the right type.
- </p>
-
- <h3>Scalars</h3>
-
- <ul>
- <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 <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 <code>bool</code> type
- and use the corresponding <code>true</code> and <code>false</code> macros.
- </li>
- <li>In the unusual event that you require a specific width, use a
- 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 <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 <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 <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., <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 <b>wrong</b> type, if <i>pulling the thread</i>
- and fixing all related variables would be too invasive.
- </p>
-
- <p>
- Finally, while using descriptive types is important, be careful not to
- go overboard. If whatever you're doing causes warnings, or requires
- casts, then reconsider or ask for help.
- </p>
-
- <h3>Pointers</h3>
-
- <p>
- Ensure that all of your pointers are <i>const-correct</i>.
- Unless a pointer is used to modify the pointed-to storage,
- 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
- it points to, or it is aliased to another pointer that is.
- </p>
-
- <h2><a id="attribute_annotations">Attribute annotations</a></h2>
- <p>
- Use the following annotations to help the compiler and/or static
- analysis tools understand the code better:
- </p>
-
- <table class="top_table">
- <tr><th>Macro</th><th>Meaning</th></tr>
- <tr><td><code>ATTRIBUTE_NONNULL</code></td><td>passing NULL for this parameter is not allowed</td></tr>
- <tr><td><code>ATTRIBUTE_PACKED</code></td><td>force a structure to be packed</td></tr>
- <tr><td><code>G_GNUC_FALLTHROUGH</code></td><td>allow code reuse by multiple switch cases</td></tr>
- <tr><td><code>G_GNUC_NO_INLINE</code></td><td>the function is mocked in the test suite</td></tr>
- <tr><td><code>G_GNUC_NORETURN</code></td><td>the function never returns</td></tr>
- <tr><td><code>G_GNUC_NULL_TERMINATED</code></td><td>last parameter must be NULL</td></tr>
- <tr><td><code>G_GNUC_PRINTF</code></td><td>validate that the formatting string matches parameters</td></tr>
- <tr><td><code>G_GNUC_UNUSED</code></td><td>parameter is unused in this implementation of the function</td></tr>
- <tr><td><code>G_GNUC_WARN_UNUSED_RESULT</code></td><td>the return value must be checked</td></tr>
- </table>
-
- <h2><a id="glib">Adoption of GLib APIs</a></h2>
-
- <p>
- Libvirt has adopted use of the
- <a href="https://developer.gnome.org/glib/stable/">GLib library</a>.
- Due to libvirt's long history of development, there are many APIs
- in libvirt, for which GLib provides an alternative solution. The
- general rule to follow is that the standard GLib solution will be
- preferred over historical libvirt APIs. Existing code will be
- ported over to use GLib APIs over time, but new code should use
- the GLib APIs straight away where possible.
- </p>
-
- <p>
- The following is a list of libvirt APIs that should no longer be
- used in new code, and their suggested GLib replacements:
- </p>
-
- <dl>
- <dt><code>VIR_ALLOC</code>, <code>VIR_REALLOC</code>,
- <code>VIR_RESIZE_N</code>, <code>VIR_EXPAND_N</code>,
- <code>VIR_SHRINK_N</code>, <code>VIR_FREE</code>,
- <code>VIR_APPEND_ELEMENT</code>, <code>VIR_INSERT_ELEMENT</code>,
- <code>VIR_DELETE_ELEMENT</code></dt>
- <dd>Prefer the GLib APIs <code>g_new0</code>/<code>g_renew</code>/
- <code>g_free</code> in most cases. There should rarely be a need
- to use <code>g_malloc</code>/<code>g_realloc</code>.
- Instead of using plain C arrays, it is preferrable to use
- one of the GLib types, <code>GArray</code>, <code>GPtrArray</code>
- or <code>GByteArray</code>. These
- all use a struct to track the array memory and size together
- and efficiently resize. <strong>NEVER MIX</strong> use of the
- classic libvirt memory allocation APIs and GLib APIs within
- a single method. Keep the style consistent, converting existing
- code to GLib style in a separate, prior commit.</dd>
-
- <dt><code>virStrerror</code></dt>
- <dd>The GLib <code>g_strerror()</code> function should be used instead,
- which has a simpler calling convention as an added benefit.</dd>
-
- <table class="top_table">
- <tr><th>deprecated version</th><th>GLib version</th><th>Notes</th></tr>
- <tr><td><code>VIR_ALLOC(var)</code></td><td><code>g_new0(var_t, 1)</code></td>
- <td>the type needs to be passed explicitly</td></tr>
- <tr><td><code>VIR_ALLOC_N</code></td><td><code>g_new0(var_t, n)</code></td><td></td></tr>
- <tr><td><code>VIR_REALLOC_N</code></td><td><code>g_renew(var_t, ptr, n)</code></td>
- <td>the newly added memory is not zeroed</td></tr>
- <tr><td><code>VIR_EXPAND_N</code></td><td><code>g_renew(var_t, ptr, n)</code></td>
- <td>zero the new memory manually or use an array type</td></tr>
- <tr><td><code>VIR_SHRINK_N</code></td><td><code>g_renew(var_t, ptr, n)</code></td>
- <td></td></tr>
- <tr><td><code>VIR_APPEND_ELEMENT</code></td><td><code>g_array_append_val</code></td>
- <td><code>g_ptr_array_add</code> or <code>g_byte_array_append</code></td></tr>
- <tr><td><code>VIR_INSERT_ELEMENT</code></td><td><code>g_array_insert_val</code></td>
- <td><code>g_ptr_array_insert</code></td></tr>
- <tr><td><code>VIR_DELETE_ELEMENT</code></td><td><code>g_array_remove_index</code></td>
- <td><code>g_ptr_array_remove_index</code> or <code>g_byte_array_remove_index</code></td></tr>
- <tr><td><code>VIR_FREE</code></td><td><code>g_free</code></td>
- <td><code>g_free</code> does not zero the pointer</td></tr>
- </table>
-
- <p>String allocation macros and functions:</p>
- <table class="top_table">
- <tr><th>deprecated version</th><th>GLib version</th><th>Notes</th></tr>
- <tr><td><code>virAsprintf</code></td><td><code>g_strdup_printf</code></td><td></td></tr>
- <tr><td><code>virVasprintf</code></td><td><code>g_strdup_vprint</code></td>
- <td>use <code>g_vasprintf</code> if you really need to know the returned length</td></tr>
- </table>
- </dl>
-
- <p>
- The following libvirt APIs have been deleted already:
- </p>
- <dl>
- <dt><code>VIR_AUTOPTR</code>, <code>VIR_AUTOCLEAN</code>, <code>VIR_AUTOFREE</code></dt>
- <dd>The GLib macros <code>g_autoptr</code>, <code>g_auto</code> and
- <code>g_autofree</code> must be used
- instead in all new code. In existing code, the GLib macros must
- never be mixed with libvirt macros within a method, nor should
- they be mixed with <code>VIR_FREE</code>. If introducing GLib macros to an
- existing method, any use of libvirt macros must be converted
- in an independent commit.
- </dd>
-
- <dt><code>VIR_DEFINE_AUTOPTR_FUNC</code>, <code>VIR_DEFINE_AUTOCLEAN_FUNC</code></dt>
- <dd>The GLib macros <code>G_DEFINE_AUTOPTR_CLEANUP_FUNC</code> and
- <code>G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC</code> must be used in all
- new code. Existing code should be converted to the
- new macros where relevant. It is permissible to use
- <code>g_autoptr</code>, <code>g_auto</code> on an object whose cleanup function
- is declared with the libvirt macros and vice-versa.
- </dd>
-
- <dt><code>VIR_AUTOUNREF</code></dt>
- <dd>The GLib macros <code>g_autoptr</code> and <code>G_DEFINE_AUTOPTR_CLEANUP_FUNC</code>
- should be used to manage autoclean of virObject classes.
- This matches usage with GObject classes.</dd>
-
- <dt><code>VIR_STRDUP</code>, <code>VIR_STRNDUP</code></dt>
- <dd>Prefer the GLib APIs <code>g_strdup</code> and <code>g_strndup</code>.</dd>
- </dl>
- <table class="top_table">
- <tr><th>deleted version</th><th>GLib version</th><th>Notes</th></tr>
- <tr><td><code>VIR_AUTOPTR</code></td><td><code>g_autoptr</code></td><td></td></tr>
- <tr><td><code>VIR_AUTOCLEAN</code></td><td><code>g_auto</code></td><td></td></tr>
- <tr><td><code>VIR_AUTOFREE</code></td><td><code>g_autofree</code></td><td>The GLib version does not use parentheses</td></tr>
- <tr><td><code>VIR_AUTOUNREF</code></td><td><code>g_autoptr</code></td><td>The cleanup function needs to be defined</td></tr>
- <tr><td><code>VIR_DEFINE_AUTOPTR_FUNC</code></td><td><code>G_DEFINE_AUTOPTR_CLEANUP_FUNC</code></td><td></td></tr>
- <tr><td><code>VIR_DEFINE_AUTOCLEAN_FUNC</code></td><td><code>G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC</code></td><td></td></tr>
- <tr><td><code>VIR_STEAL_PTR</code></td><td><code>g_steal_pointer</code></td>
- <td><code>a = f(&b)</code> instead of <code>f(a, b)</code></td></tr>
- <tr><td><code>VIR_RETURN_PTR</code></td><td><code>return g_steal_pointer</code></td><td></td></tr>
- <tr><td><code>ARRAY_CARDINALITY</code></td><td><code>G_N_ELEMENTS</code></td><td></td></tr>
- <tr><td><code>ATTRIBUTE_FALLTHROUGH</code></td><td><code>G_GNUC_FALLTHROUGH</code></td><td></td></tr>
- <tr><td><code>ATTRIBUTE_FMT_PRINTF</code></td><td><code>G_GNUC_PRINTF</code></td><td></td></tr>
- <tr><td><code>ATTRIBUTE_NOINLINE</code></td><td><code>G_GNUC_NO_INLINE</code></td><td></td></tr>
- <tr><td><code>ATTRIBUTE_NORETURN</code></td><td><code>G_GNUC_NORETURN</code></td><td></td></tr>
- <tr><td><code>ATTRIBUTE_RETURN_CHECK</code></td><td><code>G_GNUC_WARN_UNUSED_RESULT</code></td><td></td></tr>
- <tr><td><code>ATTRIBUTE_SENTINEL</code></td><td><code>G_GNUC_NULL_TERMINATED</code></td><td></td></tr>
- <tr><td><code>ATTRIBUTE_UNUSED</code></td><td><code>G_GNUC_UNUSED</code></td><td></td></tr>
- <tr><td><code>VIR_STRDUP</code></td><td><code>g_strdup</code></td><td></td></tr>
- <tr><td><code>VIR_STRNDUP</code></td><td><code>g_strndup</code></td><td></td></tr>
- <tr><td><code>virStrerror</code></td><td><code>g_strerror</code></td><td></td></tr>
- </table>
-
-
- <h2><a id="file_handling">File handling</a></h2>
-
- <p>
- Usage of the <code>fdopen()</code>, <code>close()</code>, <code>fclose()</code>
- APIs is deprecated in libvirt code base to help avoiding double-closing of files
- or file descriptors, which is particularly dangerous in a multi-threaded
- application. Instead of these APIs, use the macros from virfile.h
- </p>
-
- <ul>
- <li><p>Open a file from a file descriptor:</p>
-
-<pre>
- if ((file = VIR_FDOPEN(fd, "r")) == NULL) {
- virReportSystemError(errno, "%s",
- _("failed to open file from file descriptor"));
- return -1;
- }
- /* fd is now invalid; only access the file using file variable */
-</pre></li>
-
- <li><p>Close a file descriptor:</p>
-<pre>
- if (VIR_CLOSE(fd) < 0) {
- virReportSystemError(errno, "%s", _("failed to close file"));
- }
-</pre></li>
-
- <li><p>Close a file:</p>
-
-<pre>
- if (VIR_FCLOSE(file) < 0) {
- virReportSystemError(errno, "%s", _("failed to close file"));
- }
-</pre></li>
-
- <li><p>Close a file or file descriptor in an error path, without losing
- the previous <code>errno</code> value:</p>
-
-<pre>
- VIR_FORCE_CLOSE(fd);
- VIR_FORCE_FCLOSE(file);
-</pre>
- </li>
- </ul>
-
- <h2><a id="string_comparision">String comparisons</a></h2>
-
- <p>
- Do not use the strcmp, strncmp, etc functions directly. Instead use
- one of the following semantically named macros
- </p>
-
- <ul>
- <li><p>For strict equality:</p>
-<pre>
- STREQ(a,b)
- STRNEQ(a,b)
-</pre>
- </li>
-
- <li><p>For case insensitive equality:</p>
-<pre>
- STRCASEEQ(a,b)
- STRCASENEQ(a,b)
-</pre>
- </li>
-
- <li><p>For strict equality of a substring:</p>
-<pre>
- STREQLEN(a,b,n)
- STRNEQLEN(a,b,n)
-</pre>
- </li>
-
- <li><p>For case insensitive equality of a substring:</p>
-<pre>
- STRCASEEQLEN(a,b,n)
- STRCASENEQLEN(a,b,n)
-</pre>
- </li>
-
- <li><p>For strict equality of a prefix:</p>
-<pre>
- STRPREFIX(a,b)
-</pre>
- </li>
- <li><p>To avoid having to check if a or b are NULL:</p>
-<pre>
- STREQ_NULLABLE(a, b)
- STRNEQ_NULLABLE(a, b)
-</pre>
- </li>
- </ul>
-
-
- <h2><a id="string_copying">String copying</a></h2>
-
- <p>
- Do not use the strncpy function. According to the man page, it
- does <b>not</b> guarantee a NULL-terminated buffer, which makes
- it extremely dangerous to use. Instead, use one of the replacement
- functions provided by libvirt:
- </p>
-
-<pre>
- virStrncpy(char *dest, const char *src, size_t n, size_t destbytes)
-</pre>
- <p>
- The first two arguments have the same meaning as for strncpy,
- namely the destination and source of the copy operation. Unlike
- strncpy, the function will always copy exactly the number of bytes
- requested and make sure the destination is NULL-terminated, as the
- source is required to be; sanity checks are performed to ensure the
- size of the destination, as specified by the last argument, is
- sufficient for the operation to succeed. On success, 0 is returned;
- on failure, a value <0 is returned instead.
- </p>
-
-<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.
- </p>
-
-<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 <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.
- </p>
-
-<pre>
- dst = g_strdup(src);
- dst = g_strndup(src, n);
-</pre>
- <p>
- You should avoid using strdup or strndup directly as they do not handle
- out-of-memory errors, and do not allow a NULL source.
- Use <code>g_strdup</code> and <code>g_strndup</code> from GLib which
- abort on OOM and handle NULL source by returning NULL.
- </p>
-
- <h2><a id="strbuf">Variable length string buffer</a></h2>
-
- <p>
- If there is a need for complex string concatenations, avoid using
- the usual sequence of malloc/strcpy/strcat/snprintf functions and
- make use of either the
- <a href="https://developer.gnome.org/glib/stable/glib-Strings.html">GString</a>
- type from GLib or the virBuffer API.
- If formatting XML or QEMU command line is needed, use the virBuffer
- API described in virbuffer.h, since it has helper functions for those.
- </p>
-
- <p>Typical usage is as follows:</p>
-
-<pre>
- char *
- somefunction(...)
- {
- g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
-
- ...
-
- virBufferAddLit(&buf, "<domain>\n");
- virBufferAsprintf(&buf, " <memory>%d</memory>\n", memory);
- if (some_error)
- return NULL; /* g_auto will free the memory used so far */
- ...
- virBufferAddLit(&buf, "</domain>\n");
-
- ...
-
- if (virBufferCheckError(&buf) < 0)
- return NULL;
-
- return virBufferContentAndReset(&buf);
- }
-</pre>
-
-
- <h2><a id="includes">Include files</a></h2>
-
- <p>
- There are now quite a large number of include files, both libvirt
- internal and external, and system includes. To manage all this
- complexity it's best to stick to the following general plan for all
- *.c source files:
- </p>
-
-<pre>
- /*
- * Copyright notice
- * ....
- * ....
- * ....
- *
- */
-
- #include <config.h> Must come first in every file.
-
- #include <stdio.h> Any system includes you need.
- #include <string.h>
- #include <limits.h>
-
- #if WITH_NUMACTL Some system includes aren't supported
- # include <numa.h> everywhere so need these #if guards.
- #endif
-
- #include "internal.h" Include this first, after system includes.
-
- #include "util.h" Any libvirt internal header files.
- #include "buf.h"
-
- static int
- myInternalFunc() The actual code.
- {
- ...
-</pre>
-
- <p>
- Of particular note: <b>Do not</b> include libvirt/libvirt.h,
- libvirt/virterror.h, libvirt/libvirt-qemu.h, or libvirt/libvirt-lxc.h.
- They are included by "internal.h" already and there are some special reasons
- why you cannot include these files explicitly. One of the special cases,
- "libvirt/libvirt.h" is included prior to "internal.h" in "remote_protocol.x",
- to avoid exposing *_LAST enum elements.
- </p>
-
-
- <h2><a id="printf">Printf-style functions</a></h2>
-
- <p>
- Whenever you add a new printf-style function, i.e., one with a format
- string argument and following "..." in its prototype, be sure to use
- gcc's printf attribute directive in the prototype. For example, here's
- the one for virCommandAddEnvFormat in vircommand.h:
- </p>
-
-<pre>
- void virCommandAddEnvFormat(virCommandPtr cmd, const char *format, ...)
- G_GNUC_PRINTF(2, 3);
-</pre>
-
- <p>
- This makes it so gcc's -Wformat and -Wformat-security options can do
- their jobs and cross-check format strings with the number and types
- of arguments.
- </p>
-
- <p>
- When printing to a string, consider using GString or virBuffer for
- incremental allocations, g_strdup_printf for a one-shot allocation,
- and g_snprintf for fixed-width buffers. Only use g_sprintf,
- if you can prove the buffer won't overflow.
- </p>
-
- <h2><a id="errors">Error message format</a></h2>
-
- <p>
- Error messages visible to the user should be short and descriptive. All
- error messages are translated using gettext and thus must be wrapped in
- <code>_()</code> macro. To simplify the translation work, the error message
- must not be concatenated from various parts. To simplify searching for
- the error message in the code the strings should not be broken even
- if they result into a line longer than 80 columns and any formatting
- modifier should be enclosed by quotes or other obvious separator.
- If a string used with <code>%s</code> can be NULL the NULLSTR macro must
- be used.
- </p>
-
-<pre>
- GOOD: virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to connect to remote host '%s'"), hostname)
-
- BAD: virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to %s to remote host '%s'"),
- "connect", hostname);
-
- BAD: virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to connect "
- "to remote host '%s'),
- hostname);
-</pre>
-
- <h2><a id="goto">Use of goto</a></h2>
-
- <p>
- 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.
- </p>
-
- <p>
- 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. g_free() and most of the functions named XXXFree() in
- libvirt is required to handle NULL as its arg. This does not
- apply to libvirt's public APIs. 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.
- Note that most of libvirt's type declarations can be marked with
- either <code>g_autofree</code> or <code>g_autoptr</code> which uses
- the compiler's <code>__attribute__((cleanup))</code> that calls
- the appropriate free function when the variable goes out of scope.
- </p>
-
- <p>
- There are a couple of signs that a particular use of goto is not
- ok:
- </p>
-
- <ul>
- <li>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.</li>
- <li>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.</li>
- <li>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.</li>
- </ul>
-
- <p>
- 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>
- </p>
-
- <p>
- When using goto, please use one of these standard labels if it
- makes sense:
- </p>
-
-<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 (e.g., retry on EINTR)
-</pre>
-
- <p>
- Top-level labels should be indented by one space (putting them on
- the beginning of the line confuses function context detection in git):
- </p>
-
-<pre>
-int foo()
-{
- /* ... do stuff ... */
- cleanup:
- /* ... do other stuff ... */
-}
-</pre>
-
-
-
- <h2><a id="committers">Libvirt committer guidelines</a></h2>
-
- <p>
- The AUTHORS files indicates the list of people with commit access right
- who can actually merge the patches.
- </p>
-
- <p>
- 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 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.
- </p>
-
- <p>
- An exception to 'review and approval on the list first' is fixing failures
- to build:
- </p>
- <ul>
- <li>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</li>
- <li>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</li>
- <li>
- fixes for documentation and code comments can be managed
- in the same way, but still make sure they get reviewed if non-trivial.
- </li>
- <li>(ir)regular pulls from other repositories or automated updates, such
- as the keycodemap submodule updates, pulling in new translations or updating
- the container images for the CI system
- </li>
- </ul>
- </body>
-</html>
diff --git a/docs/hacking.rst b/docs/hacking.rst
new file mode 100644
index 0000000000..ac02e9ab73
--- /dev/null
+++ b/docs/hacking.rst
@@ -0,0 +1,1400 @@
+======================
+Contributor guidelines
+======================
+
+.. contents::
+
+General tips for contributing patches
+=====================================
+
+#. Discuss any large changes on the mailing list first. Post
+ patches early and listen to feedback.
+
+#. Official upstream repository is kept in git
+ (``https://libvirt.org/git/libvirt.git``) and is browsable
+ along with other libvirt-related repositories (e.g.
+ libvirt-python) `online <https://libvirt.org/git/>`__.
+
+#. Patches to translations are maintained via the `zanata
+ project <https://fedora.zanata.org/>`__. If you want to fix a
+ translation in a .po file, join the appropriate language team.
+ The libvirt release process automatically pulls the latest
+ version of each translation file from zanata.
+
+#. The simplest way to send patches is to use the
+ `git-publish <https://github.com/stefanha/git-publish>`__
+ tool. All libvirt-related repositories contain a config file
+ that tells git-publish to use the correct mailing list and
+ subject prefix.
+
+ Alternatively, you may send patches using ``git send-email``.
+
+ Also, for code motion patches, you may find that
+ ``git diff --patience`` provides an easier-to-read
+ patch. However, the usual workflow of libvirt developer is:
+
+ ::
+
+ git checkout master
+ git pull
+ git checkout -t origin -b workbranch
+ Hack, committing any changes along the way
+
+ More hints on compiling can be found `here <compiling.html>`__.
+ When you want to post your patches:
+
+ ::
+
+ git pull --rebase
+ (fix any conflicts)
+ git send-email --cover-letter --no-chain-reply-to --annotate \
+ --confirm=always --to=libvir-list at redhat.com master
+
+ For a single patch you can omit ``--cover-letter``, but a
+ series of two or more patches needs a cover letter.
+
+ Note that the ``git send-email`` subcommand may not be in the
+ main git package and using it may require installation of a
+ separate package, for example the "git-email" package in Fedora
+ and Debian. If this is your first time using
+ ``git send-email``, you might need to configure it to point it
+ to your SMTP server with something like:
+
+ ::
+
+ git config --global sendemail.smtpServer stmp.youremailprovider.net
+
+ If you get tired of typing ``--to=libvir-list at redhat.com`` all
+ the time, you can configure that to be automatically handled as
+ well:
+
+ ::
+
+ git config sendemail.to libvir-list at redhat.com
+
+ As a rule, patches should be sent to the mailing list only: all
+ developers are subscribed to libvir-list and read it regularly,
+ so **please don't CC individual developers** unless they've
+ explicitly asked you to.
+
+ Avoid using mail clients for sending patches, as most of them
+ will mangle the messages in some way, making them unusable for
+ our purposes. Gmail and other Web-based mail clients are
+ particularly bad at this.
+
+ If everything went well, your patch should show up on the
+ `libvir-list
+ archives <https://www.redhat.com/archives/libvir-list/>`__ in a
+ matter of minutes; if you still can't find it on there after an
+ hour or so, you should double-check your setup. **Note that, if
+ you are not already a subscriber, your very first post to the
+ mailing list will be subject to moderation**, and it's not
+ uncommon for that to take around a day.
+
+ Please follow this as close as you can, especially the rebase
+ and ``git send-email`` part, as it makes life easier for other
+ developers to review your patch set.
+
+ One should avoid sending patches as attachments, but rather
+ send them in email body along with commit message. If a
+ developer is sending another version of the patch (e.g. to
+ address review comments), they are advised to note differences
+ to previous versions after the ``---`` line in the patch so
+ that it helps reviewers but doesn't become part of git history.
+ Moreover, such patch needs to be prefixed correctly with
+ ``--subject-prefix=PATCHv2`` appended to
+ ``git send-email`` (substitute ``v2`` with the
+ correct version if needed though).
+
+#. In your commit message, make the summary line reasonably short
+ (60 characters is typical), followed by a blank line, followed
+ by any longer description of why your patch makes sense. If the
+ patch fixes a regression, and you know what commit introduced
+ the problem, mentioning that is useful. If the patch resolves a
+ bugzilla report, mentioning the URL of the bug number is
+ useful; but also summarize the issue rather than making all
+ readers follow the link. You can use 'git shortlog -30' to get
+ an idea of typical summary lines.
+
+#. Contributors to libvirt projects **must** assert that they are
+ in compliance with the `Developer Certificate of Origin
+ 1.1 <https://developercertificate.org/>`__. This is achieved by
+ adding a "Signed-off-by" line containing the contributor's name
+ and e-mail to every commit message. The presence of this line
+ attests that the contributor has read the above lined DCO and
+ agrees with its statements.
+
+#. 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. Moreover, please keep in mind that it's required to
+ be able to compile cleanly (**including**
+ ``make check`` and ``make syntax-check``) after each
+ patch. A feature does not have to work until the end of a
+ series, but intermediate patches must compile and not cause
+ test-suite failures (this is to preserve the usefulness of
+ ``git bisect``, among other things).
+
+#. Make sure your patches apply against libvirt GIT. Developers
+ only follow GIT and don't care much about released versions.
+
+#. Run the automated tests on your code before submitting any
+ changes. That is:
+
+ ::
+
+ make check
+ make syntax-check
+ make -C tests valgrind
+
+ `Valgrind <http://valgrind.org/>`__ is a test that checks for
+ memory management issues, such as leaks or use of uninitialized
+ variables.
+
+ Some tests are skipped by default in a development environment,
+ based on the time they take in comparison to the likelihood
+ that those tests will turn up problems during incremental
+ builds. These tests default to being run when building from a
+ tarball or with the configure option --enable-expensive-tests;
+ you can also force a one-time toggle of these tests by setting
+ VIR_TEST_EXPENSIVE to 0 or 1 at make time, as in:
+
+ ::
+
+ make check VIR_TEST_EXPENSIVE=1
+
+ 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:
+
+ ::
+
+ VIR_TEST_DEBUG=1 make check (or)
+ VIR_TEST_DEBUG=2 make check
+
+ When debugging failures during development, it is possible to
+ focus in on just the failing subtests by using VIR_TEST_RANGE.
+ I.e. to run all tests from 3 to 20 with the exception of tests
+ 6 and 16, use:
+
+ ::
+
+ VIR_TEST_DEBUG=1 VIR_TEST_RANGE=3-5,7-20,^16 ./run tests/qemuxml2argvtest
+
+ Also, individual tests can be run from inside the ``tests/``
+ directory, like:
+
+ ::
+
+ ./qemuxml2xmltest
+
+ If you are adding new test cases, or making changes that alter
+ existing test output, you can use the environment variable
+ VIR_TEST_REGENERATE_OUTPUT to quickly update the saved test
+ data. Of course you still need to review the changes VERY
+ CAREFULLY to ensure they are correct.
+
+ ::
+
+ VIR_TEST_REGENERATE_OUTPUT=1 ./qemuxml2argvtest
+
+ There is also a ``./run`` script at the top level, to make it
+ easier to run programs that have not yet been installed, as
+ well as to wrap invocations of various tests under gdb or
+ Valgrind.
+
+ When running our test suite it may happen that the test result
+ is nondeterministic because of the test suite relying on a
+ particular file in the system being accessible or having some
+ specific value. To catch this kind of errors, the test suite
+ has a module for that prints any path touched that fulfils
+ constraints described above into a file. To enable it just set
+ ``VIR_TEST_FILE_ACCESS`` environment variable. Then
+ ``VIR_TEST_FILE_ACCESS_OUTPUT`` environment variable can alter
+ location where the file is stored.
+
+ ::
+
+ VIR_TEST_FILE_ACCESS=1 VIR_TEST_FILE_ACCESS_OUTPUT="/tmp/file_access.txt" ./qemuxml2argvtest
+
+#. The Valgrind test should produce similar output to
+ ``make check``. If the output has traces within libvirt API's,
+ then investigation is required in order to determine the cause
+ of the issue. Output such as the following indicates some sort
+ of leak:
+
+ ::
+
+ ==5414== 4 bytes in 1 blocks are definitely lost in loss record 3 of 89
+ ==5414== at 0x4A0881C: malloc (vg_replace_malloc.c:270)
+ ==5414== by 0x34DE0AAB85: xmlStrndup (in /usr/lib64/libxml2.so.2.7.8)
+ ==5414== by 0x4CC97A6: virDomainVideoDefParseXML (domain_conf.c:7410)
+ ==5414== by 0x4CD581D: virDomainDefParseXML (domain_conf.c:10188)
+ ==5414== by 0x4CD8C73: virDomainDefParseNode (domain_conf.c:10640)
+ ==5414== by 0x4CD8DDB: virDomainDefParse (domain_conf.c:10590)
+ ==5414== by 0x41CB1D: testCompareXMLToArgvHelper (qemuxml2argvtest.c:100)
+ ==5414== by 0x41E20F: virtTestRun (testutils.c:161)
+ ==5414== by 0x41C7CB: mymain (qemuxml2argvtest.c:866)
+ ==5414== by 0x41E84A: virtTestMain (testutils.c:723)
+ ==5414== by 0x34D9021734: (below main) (in /usr/lib64/libc-2.15.so)
+
+ In this example, the ``virDomainDefParseXML()`` had an error
+ path where the ``virDomainVideoDefPtr video`` pointer was not
+ properly disposed. By simply adding a
+ ``virDomainVideoDefFree(video);`` in the error path, the issue
+ was resolved.
+
+ Another common mistake is calling a printing function, such as
+ ``VIR_DEBUG()`` without initializing a variable to be printed.
+ The following example involved a call which could return an
+ error, but not set variables passed by reference to the call.
+ The solution was to initialize the variables prior to the call.
+
+ ::
+
+ ==4749== Use of uninitialised value of size 8
+ ==4749== at 0x34D904650B: _itoa_word (in /usr/lib64/libc-2.15.so)
+ ==4749== by 0x34D9049118: vfprintf (in /usr/lib64/libc-2.15.so)
+ ==4749== by 0x34D9108F60: __vasprintf_chk (in /usr/lib64/libc-2.15.so)
+ ==4749== by 0x4CAEEF7: virVasprintf (stdio2.h:199)
+ ==4749== by 0x4C8A55E: virLogVMessage (virlog.c:814)
+ ==4749== by 0x4C8AA96: virLogMessage (virlog.c:751)
+ ==4749== by 0x4DA0056: virNetTLSContextCheckCertKeyUsage (virnettlscontext.c:225)
+ ==4749== by 0x4DA06DB: virNetTLSContextCheckCert (virnettlscontext.c:439)
+ ==4749== by 0x4DA1620: virNetTLSContextNew (virnettlscontext.c:562)
+ ==4749== by 0x4DA26FC: virNetTLSContextNewServer (virnettlscontext.c:927)
+ ==4749== by 0x409C39: testTLSContextInit (virnettlscontexttest.c:467)
+ ==4749== by 0x40AB8F: virtTestRun (testutils.c:161)
+
+ Valgrind will also find some false positives or code paths
+ which cannot be resolved by making changes to the libvirt code.
+ For these paths, it is possible to add a filter to avoid the
+ errors. For example:
+
+ ::
+
+ ==4643== 7 bytes in 1 blocks are possibly lost in loss record 4 of 20
+ ==4643== at 0x4A0881C: malloc (vg_replace_malloc.c:270)
+ ==4643== by 0x34D90853F1: strdup (in /usr/lib64/libc-2.15.so)
+ ==4643== by 0x34EEC2C08A: ??? (in /usr/lib64/libnl.so.1.1)
+ ==4643== by 0x34EEC15B81: ??? (in /usr/lib64/libnl.so.1.1)
+ ==4643== by 0x34D8C0EE15: call_init.part.0 (in /usr/lib64/ld-2.15.so)
+ ==4643== by 0x34D8C0EECF: _dl_init (in /usr/lib64/ld-2.15.so)
+ ==4643== by 0x34D8C01569: ??? (in /usr/lib64/ld-2.15.so)
+
+ In this instance, it is acceptable to modify the
+ ``tests/.valgrind.supp`` file in order to add a suppression
+ filter. The filter should be unique enough to not suppress real
+ leaks, but it should be generic enough to cover multiple code
+ paths. The format of the entry can be found in the
+ documentation found at the `Valgrind home
+ page <http://valgrind.org/>`__. The following trace was added
+ to ``tests/.valgrind.supp`` in order to suppress the warning:
+
+ ::
+
+ {
+ dlInitMemoryLeak1
+ Memcheck:Leak
+ fun:?alloc
+ ...
+ fun:call_init.part.0
+ fun:_dl_init
+ ...
+ obj:*/lib*/ld-2.*so*
+ }
+
+#. Update tests and/or documentation, particularly if you are
+ adding a new feature or changing the output of a program.
+
+#. Don't forget to update the `release notes <news.html>`__ by
+ changing ``docs/news.xml`` if your changes are significant. All
+ user-visible changes, such as adding new XML elements or fixing
+ all but the most obscure bugs, must be (briefly) described in a
+ release notes entry; changes that are only relevant to other
+ libvirt developers, such as code refactoring, don't belong in
+ the release notes. Note that ``docs/news.xml`` should be
+ updated in its own commit not to get in the way of backports.
+
+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://people.redhat.com/rjones/how-to-supply-code-to-open-source-projects/>`__.
+
+Language Usage
+==============
+
+The libvirt repository makes use of a large number of programming
+languages. It is anticipated that in the future libvirt will adopt
+use of other new languages. To reduce the overall burden on
+developers, there is thus a general desire to phase out usage of
+some of the existing languages.
+
+The preferred languages at this time are:
+
+- C - for the main libvirt codebase. Dialect supported by
+ GCC/CLang only.
+- Python - for supporting build scripts / tools. Code must run
+ with both version 2.7 and 3.x at this time.
+
+Languages that should not be used for any new contributions:
+
+- Perl - build scripts must be written in Python instead.
+- Shell - build scripts must be written in Python instead.
+
+Tooling
+=======
+
+libvirt includes support for some useful development tools right
+in its source repository, meaning users will be able to take
+advantage of them without little or no configuration. Examples
+include:
+
+- `color_coded <https://github.com/jeaye/color_coded>`__, a vim
+ plugin for libclang-powered semantic syntax highlighting;
+- `YouCompleteMe <http://valloric.github.io/YouCompleteMe/>`__, a
+ vim plugin for libclang-powered semantic code completion.
+
+Naming conventions
+==================
+
+When reading libvirt code, a number of different naming
+conventions will be evident due to various changes in thinking
+over the course of the project's lifetime. The conventions
+documented below should be followed when creating any entirely new
+files in libvirt. When working on existing files, while it is
+desirable to apply these conventions, keeping a consistent style
+with existing code in that particular file is generally more
+important. The overall guiding principal is that every file, enum,
+struct, function, macro and typedef name must have a 'vir' or
+'VIR' prefix. All local scope variable names are exempt, and
+global variables are exempt, unless exported in a header file.
+
+File names
+ File naming varies depending on the subdirectory. The preferred
+ style is to have a 'vir' prefix, followed by a name which
+ matches the name of the functions / objects inside the file.
+ For example, a file containing an object 'virHashtable' is
+ stored in files 'virhashtable.c' and 'virhashtable.h'.
+ Sometimes, methods which would otherwise be declared 'static'
+ need to be exported for use by a test suite. For this purpose a
+ second header file should be added with a suffix of 'priv',
+ e.g. 'virhashtablepriv.h'. Use of underscores in file names is
+ discouraged when using the 'vir' prefix style. The 'vir' prefix
+ naming applies to src/util, src/rpc and tests/ directories.
+ Most other directories do not follow this convention.
+
+Enum type & field names
+ All enums should have a 'vir' prefix in their typedef name, and
+ each following word should have its first letter in uppercase.
+ The enum name should match the typedef name with a leading
+ underscore. The enum member names should be in all uppercase,
+ and use an underscore to separate each word. The enum member
+ name prefix should match the enum typedef name.
+
+ ::
+
+ typedef enum _virSocketType virSocketType;
+ enum _virSocketType {
+ VIR_SOCKET_TYPE_IPV4,
+ VIR_SOCKET_TYPE_IPV6,
+ };
+
+Struct type names
+ All structs should have a 'vir' prefix in their typedef name,
+ and each following word should have its first letter in
+ uppercase. The struct name should be the same as the typedef
+ name with a leading underscore. A second typedef should be
+ given for a pointer to the struct with a 'Ptr' suffix.
+
+ ::
+
+ typedef struct _virHashTable virHashTable;
+ typedef virHashTable *virHashTablePtr;
+ struct _virHashTable {
+ ...
+ };
+
+Function names
+ All functions should have a 'vir' prefix in their name,
+ followed by one or more words with first letter of each word
+ capitalized. Underscores should not be used in function names.
+ If the function is operating on an object, then the function
+ name prefix should match the object typedef name, otherwise it
+ should match the filename. Following this comes the verb /
+ action name, and finally an optional subject name. For example,
+ given an object 'virHashTable', all functions should have a
+ name 'virHashTable$VERB' or 'virHashTable$VERB$SUBJECT", e.g.
+ 'virHashTableLookup' or 'virHashTableGetValue'.
+
+Macro names
+ All macros should have a "VIR" prefix in their name, followed
+ by one or more uppercase words separated by underscores. The
+ macro argument names should be in lowercase. Aside from having
+ a "VIR" prefix there are no common practices for the rest of
+ the macro name.
+
+Code indentation
+================
+
+Libvirt's C source code generally adheres to some basic
+code-formatting conventions. The existing code base is not totally
+consistent on this front, but we do prefer that contributed code
+be formatted similarly. In short, use spaces-not-TABs for
+indentation, use 4 spaces for each indentation level, and other
+than that, follow the K&R style.
+
+If you use Emacs, the project includes a file .dir-locals.el that
+sets up the preferred indentation. If you use vim, append the
+following to your ~/.vimrc file:
+
+::
+
+ set nocompatible
+ filetype on
+ set autoindent
+ set smartindent
+ set cindent
+ set tabstop=8
+ set shiftwidth=4
+ set expandtab
+ set cinoptions=(0,:0,l1,t0,L3
+ filetype plugin indent on
+ au FileType make setlocal noexpandtab
+ au BufRead,BufNewFile *.am setlocal noexpandtab
+ match ErrorMsg /\s\+$\| \+\ze\t/
+
+Or if you don't want to mess your ~/.vimrc up, you can save the
+above into a file called .lvimrc (not .vimrc) located at the root
+of libvirt source, then install a vim script from
+http://www.vim.org/scripts/script.php?script_id=1408, which will
+load the .lvimrc only when you edit libvirt code.
+
+Code formatting (especially for new code)
+=========================================
+
+With new code, we can be even more strict. Please apply the
+following function (using GNU indent) to any new code. Note that
+this also gives you an idea of the type of spacing we prefer
+around operators and keywords:
+
+::
+
+ indent-libvirt()
+ {
+ indent -bad -bap -bbb -bli4 -br -ce -brs -cs -i4 -l75 -lc75 \
+ -sbi4 -psl -saf -sai -saw -sbi4 -ss -sc -cdw -cli4 -npcs -nbc \
+ --no-tabs "$@"
+ }
+
+Note that sometimes you'll have to 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.
+
+Libvirt requires a C99 compiler for various reasons. However, most
+of the code base prefers to stick to C89 syntax unless there is a
+compelling reason otherwise. For example, it is preferable to use
+``/* */`` comments rather than ``//``. Also, when declaring local
+variables, the prevailing style has been to declare them at the
+beginning of a scope, rather than immediately before use.
+
+Bracket spacing
+---------------
+
+The keywords ``if``, ``for``, ``while``, and ``switch`` must have
+a single space following them before the opening bracket. E.g.
+
+::
+
+ if(foo) // Bad
+ if (foo) // Good
+
+Function implementations must **not** have any whitespace between
+the function name and the opening bracket. E.g.
+
+::
+
+ int foo (int wizz) // Bad
+ int foo(int wizz) // Good
+
+Function calls must **not** have any whitespace between the
+function name and the opening bracket. E.g.
+
+::
+
+ bar = foo (wizz); // Bad
+ bar = foo(wizz); // Good
+
+Function typedefs must **not** have any whitespace between the
+closing bracket of the function name and opening bracket of the
+arg list. E.g.
+
+::
+
+ typedef int (*foo) (int wizz); // Bad
+ typedef int (*foo)(int wizz); // Good
+
+There must not be any whitespace immediately following any opening
+bracket, or immediately prior to any closing bracket. E.g.
+
+::
+
+ int foo( int wizz ); // Bad
+ int foo(int wizz); // Good
+
+Commas
+------
+
+Commas should always be followed by a space or end of line, and
+never have leading space; this is enforced during 'make
+syntax-check'.
+
+::
+
+ call(a,b ,c);// Bad
+ call(a, b, c); // Good
+
+When declaring an enum or using a struct initializer that occupies
+more than one line, use a trailing comma. That way, future edits
+to extend the list only have to add a line, rather than modify an
+existing line to add the intermediate comma. Any sentinel
+enumerator value with a name ending in \_LAST is exempt, since you
+would extend such an enum before the \_LAST element. Another
+reason to favor trailing commas is that it requires less effort to
+produce via code generators. Note that the syntax checker is
+unable to enforce a style of trailing commas, so there are
+counterexamples in existing code which do not use it; also, while
+C99 allows trailing commas, remember that JSON and XDR do not.
+
+::
+
+ enum {
+ VALUE_ONE,
+ VALUE_TWO // Bad
+ };
+ enum {
+ VALUE_THREE,
+ VALUE_FOUR, // Good
+ };
+
+Semicolons
+----------
+
+Semicolons should never have a space beforehand. Inside the
+condition of a ``for`` loop, there should always be a space or
+line break after each semicolon, except for the special case of an
+infinite loop (although more infinite loops use ``while``). While
+not enforced, loop counters generally use post-increment.
+
+::
+
+ for (i = 0 ;i < limit ; ++i) { // Bad
+ for (i = 0; i < limit; i++) { // Good
+ for (;;) { // ok
+ while (1) { // Better
+
+Empty loop bodies are better represented with curly braces and a
+comment, although use of a semicolon is not currently rejected.
+
+::
+
+ while ((rc = waitpid(pid, &st, 0) == -1) &&
+ errno == EINTR); // ok
+ while ((rc = waitpid(pid, &st, 0) == -1) &&
+ errno == EINTR) { // Better
+ /* nothing */
+ }
+
+Curly braces
+------------
+
+Omit the curly braces around an ``if``, ``while``, ``for`` etc.
+body only when both that body and the condition itself occupy 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.
+
+::
+
+ while (expr) // single line body; {} is forbidden
+ single_line_stmt();
+
+::
+
+ while (expr(arg1,
+ arg2)) // indentation makes it obvious it is single line,
+ single_line_stmt(); // {} is optional (not enforced either way)
+
+::
+
+ while (expr1 &&
+ expr2) { // multi-line, at same indentation, {} required
+ single_line_stmt();
+ }
+
+However, the moment your loop/if/else body extends on to 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:
+
+::
+
+ 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();
+ }
+
+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"));
+
+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
+ ...
+ }
+
+Do this, instead:
+
+::
+
+ 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 an ``if`` or ``else`` block, and the
+counterpart block **does** use braces. In that case, put braces
+around both blocks. Also, if the ``else`` block is much shorter
+than the ``if`` block, consider negating the ``if``-condition and
+swapping the bodies, putting the short block first and making the
+longer, multi-line block be the ``else`` block.
+
+::
+
+ if (expr) {
+ ...
+ ...
+ }
+ else
+ x = y; // BAD: braceless "else" with braced "then",
+ // and short block last
+
+ if (expr)
+ x = y; // BAD: braceless "if" with braced "else"
+ else {
+ ...
+ ...
+ }
+
+Keeping braces consistent and putting the short block first 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 more involved block:
+
+::
+
+ if (!expr) {
+ x = y; // putting the smaller block first is more readable
+ } else {
+ ...
+ ...
+ }
+
+But if negating a complex condition is too ugly, then at least add
+braces:
+
+::
+
+ if (complex expr not worth negating) {
+ ...
+ ...
+ } else {
+ x = y;
+ }
+
+Use hanging braces for compound statements: the opening brace of a
+compound statement should be on the same line as the condition
+being tested. Only top-level function bodies, nested scopes, and
+compound structure declarations should ever have { on a line by
+itself.
+
+::
+
+ void
+ foo(int a, int b)
+ { // correct - function body
+ int 2d[][] = {
+ { // correct - complex initialization
+ 1, 2,
+ },
+ };
+ if (a)
+ { // BAD: compound brace on its own line
+ do_stuff();
+ }
+ { // correct - nested scope
+ int tmp;
+ if (a < b) { // correct - hanging brace
+ tmp = b;
+ b = a;
+ a = tmp;
+ }
+ }
+ }
+
+Conditional expressions
+-----------------------
+
+For readability reasons new code should avoid shortening
+comparisons to 0 for numeric types. Boolean and pointer
+comparisions may be shortened. All long forms are okay:
+
+::
+
+ virFooPtr foos = NULL;
+ size nfoos = 0;
+ bool hasFoos = false;
+
+ GOOD:
+ if (!foos)
+ if (!hasFoos)
+ if (nfoos == 0)
+ if (foos == NULL)
+ if (hasFoos == true)
+
+ BAD:
+ if (!nfoos)
+ if (nfoos)
+
+New code should avoid the ternary operator as much as possible.
+Specifically it must never span more than one line or nest:
+
+::
+
+ BAD:
+ char *foo = baz ?
+ virDoSomethingReallyComplex(driver, vm, something, baz->foo) :
+ NULL;
+
+ char *foo = bar ? bar->baz ? bar->baz->foo : "nobaz" : "nobar";
+
+Preprocessor
+------------
+
+Macros defined with an ALL_CAPS name should generally be assumed
+to be unsafe with regards to arguments with side-effects (that is,
+MAX(a++, b--) might increment a or decrement b too many or too few
+times). Exceptions to this rule are explicitly documented for
+macros in viralloc.h and virstring.h.
+
+For variadic macros, stick with C99 syntax:
+
+::
+
+ #define vshPrint(_ctl, ...) fprintf(stdout, __VA_ARGS__)
+
+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
+
+C types
+-------
+
+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.
+- 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.
+
+Attribute annotations
+---------------------
+
+Use the following annotations to help the compiler and/or static
+analysis tools understand the code better:
+
++-------------------------------+------------------------------------------------------------+
+| Macro | Meaning |
++===============================+============================================================+
+| ``ATTRIBUTE_NONNULL`` | passing NULL for this parameter is not allowed |
++-------------------------------+------------------------------------------------------------+
+| ``ATTRIBUTE_PACKED`` | force a structure to be packed |
++-------------------------------+------------------------------------------------------------+
+| ``G_GNUC_FALLTHROUGH`` | allow code reuse by multiple switch cases |
++-------------------------------+------------------------------------------------------------+
+| ``G_GNUC_NO_INLINE`` | the function is mocked in the test suite |
++-------------------------------+------------------------------------------------------------+
+| ``G_GNUC_NORETURN`` | the function never returns |
++-------------------------------+------------------------------------------------------------+
+| ``G_GNUC_NULL_TERMINATED`` | last parameter must be NULL |
++-------------------------------+------------------------------------------------------------+
+| ``G_GNUC_PRINTF`` | validate that the formatting string matches parameters |
++-------------------------------+------------------------------------------------------------+
+| ``G_GNUC_UNUSED`` | parameter is unused in this implementation of the function |
++-------------------------------+------------------------------------------------------------+
+| ``G_GNUC_WARN_UNUSED_RESULT`` | the return value must be checked |
++-------------------------------+------------------------------------------------------------+
+
+Adoption of GLib APIs
+---------------------
+
+Libvirt has adopted use of the `GLib
+library <https://developer.gnome.org/glib/stable/>`__. Due to
+libvirt's long history of development, there are many APIs in
+libvirt, for which GLib provides an alternative solution. The
+general rule to follow is that the standard GLib solution will be
+preferred over historical libvirt APIs. Existing code will be
+ported over to use GLib APIs over time, but new code should use
+the GLib APIs straight away where possible.
+
+The following is a list of libvirt APIs that should no longer be
+used in new code, and their suggested GLib replacements:
+
+``VIR_ALLOC``, ``VIR_REALLOC``, ``VIR_RESIZE_N``, ``VIR_EXPAND_N``, ``VIR_SHRINK_N``, ``VIR_FREE``, ``VIR_APPEND_ELEMENT``, ``VIR_INSERT_ELEMENT``, ``VIR_DELETE_ELEMENT``
+ Prefer the GLib APIs ``g_new0``/``g_renew``/ ``g_free`` in most
+ cases. There should rarely be a need to use
+ ``g_malloc``/``g_realloc``. Instead of using plain C arrays, it
+ is preferrable to use one of the GLib types, ``GArray``,
+ ``GPtrArray`` or ``GByteArray``. These all use a struct to
+ track the array memory and size together and efficiently
+ resize. **NEVER MIX** use of the classic libvirt memory
+ allocation APIs and GLib APIs within a single method. Keep the
+ style consistent, converting existing code to GLib style in a
+ separate, prior commit.
+``virStrerror``
+ The GLib ``g_strerror()`` function should be used instead,
+ which has a simpler calling convention as an added benefit.
+
+The following libvirt APIs have been deleted already:
+
+``VIR_AUTOPTR``, ``VIR_AUTOCLEAN``, ``VIR_AUTOFREE``
+ The GLib macros ``g_autoptr``, ``g_auto`` and ``g_autofree``
+ must be used instead in all new code. In existing code, the
+ GLib macros must never be mixed with libvirt macros within a
+ method, nor should they be mixed with ``VIR_FREE``. If
+ introducing GLib macros to an existing method, any use of
+ libvirt macros must be converted in an independent commit.
+``VIR_DEFINE_AUTOPTR_FUNC``, ``VIR_DEFINE_AUTOCLEAN_FUNC``
+ The GLib macros ``G_DEFINE_AUTOPTR_CLEANUP_FUNC`` and
+ ``G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC`` must be used in all new
+ code. Existing code should be converted to the new macros where
+ relevant. It is permissible to use ``g_autoptr``, ``g_auto`` on
+ an object whose cleanup function is declared with the libvirt
+ macros and vice-versa.
+``VIR_AUTOUNREF``
+ The GLib macros ``g_autoptr`` and
+ ``G_DEFINE_AUTOPTR_CLEANUP_FUNC`` should be used to manage
+ autoclean of virObject classes. This matches usage with GObject
+ classes.
+``VIR_STRDUP``, ``VIR_STRNDUP``
+ Prefer the GLib APIs ``g_strdup`` and ``g_strndup``.
+
++-------------------------------+--------------------------------------+-------------------------------------------+
+| deleted version | GLib version | Notes |
++===============================+======================================+===========================================+
+| ``VIR_AUTOPTR`` | ``g_autoptr`` | |
++-------------------------------+--------------------------------------+-------------------------------------------+
+| ``VIR_AUTOCLEAN`` | ``g_auto`` | |
++-------------------------------+--------------------------------------+-------------------------------------------+
+| ``VIR_AUTOFREE`` | ``g_autofree`` | The GLib version does not use parentheses |
++-------------------------------+--------------------------------------+-------------------------------------------+
+| ``VIR_AUTOUNREF`` | ``g_autoptr`` | The cleanup function needs to be defined |
++-------------------------------+--------------------------------------+-------------------------------------------+
+| ``VIR_DEFINE_AUTOPTR_FUNC`` | ``G_DEFINE_AUTOPTR_CLEANUP_FUNC`` | |
++-------------------------------+--------------------------------------+-------------------------------------------+
+| ``VIR_DEFINE_AUTOCLEAN_FUNC`` | ``G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC`` | |
++-------------------------------+--------------------------------------+-------------------------------------------+
+| ``VIR_STEAL_PTR`` | ``g_steal_pointer`` | ``a = f(&b)`` instead of ``f(a, b)`` |
++-------------------------------+--------------------------------------+-------------------------------------------+
+| ``VIR_RETURN_PTR`` | ``return g_steal_pointer`` | |
++-------------------------------+--------------------------------------+-------------------------------------------+
+| ``ARRAY_CARDINALITY`` | ``G_N_ELEMENTS`` | |
++-------------------------------+--------------------------------------+-------------------------------------------+
+| ``ATTRIBUTE_FALLTHROUGH`` | ``G_GNUC_FALLTHROUGH`` | |
++-------------------------------+--------------------------------------+-------------------------------------------+
+| ``ATTRIBUTE_FMT_PRINTF`` | ``G_GNUC_PRINTF`` | |
++-------------------------------+--------------------------------------+-------------------------------------------+
+| ``ATTRIBUTE_NOINLINE`` | ``G_GNUC_NO_INLINE`` | |
++-------------------------------+--------------------------------------+-------------------------------------------+
+| ``ATTRIBUTE_NORETURN`` | ``G_GNUC_NORETURN`` | |
++-------------------------------+--------------------------------------+-------------------------------------------+
+| ``ATTRIBUTE_RETURN_CHECK`` | ``G_GNUC_WARN_UNUSED_RESULT`` | |
++-------------------------------+--------------------------------------+-------------------------------------------+
+| ``ATTRIBUTE_SENTINEL`` | ``G_GNUC_NULL_TERMINATED`` | |
++-------------------------------+--------------------------------------+-------------------------------------------+
+| ``ATTRIBUTE_UNUSED`` | ``G_GNUC_UNUSED`` | |
++-------------------------------+--------------------------------------+-------------------------------------------+
+| ``VIR_STRDUP`` | ``g_strdup`` | |
++-------------------------------+--------------------------------------+-------------------------------------------+
+| ``VIR_STRNDUP`` | ``g_strndup`` | |
++-------------------------------+--------------------------------------+-------------------------------------------+
+| ``virStrerror`` | ``g_strerror`` | |
++-------------------------------+--------------------------------------+-------------------------------------------+
+
+File handling
+-------------
+
+Usage of the ``fdopen()``, ``close()``, ``fclose()`` APIs is
+deprecated in libvirt code base to help avoiding double-closing of
+files or file descriptors, which is particularly dangerous in a
+multi-threaded application. Instead of these APIs, use the macros
+from virfile.h
+
+- Open a file from a file descriptor:
+
+ ::
+
+ if ((file = VIR_FDOPEN(fd, "r")) == NULL) {
+ virReportSystemError(errno, "%s",
+ _("failed to open file from file descriptor"));
+ return -1;
+ }
+ /* fd is now invalid; only access the file using file variable */
+
+- Close a file descriptor:
+
+ ::
+
+ if (VIR_CLOSE(fd) < 0) {
+ virReportSystemError(errno, "%s", _("failed to close file"));
+ }
+
+- Close a file:
+
+ ::
+
+ if (VIR_FCLOSE(file) < 0) {
+ virReportSystemError(errno, "%s", _("failed to close file"));
+ }
+
+- Close a file or file descriptor in an error path, without
+ losing the previous ``errno`` value:
+
+ ::
+
+ VIR_FORCE_CLOSE(fd);
+ VIR_FORCE_FCLOSE(file);
+
+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)
+
+- For case insensitive equality of a substring:
+
+ ::
+
+ STRCASEEQLEN(a,b,n)
+ STRCASENEQLEN(a,b,n)
+
+- For strict equality of a prefix:
+
+ ::
+
+ STRPREFIX(a,b)
+
+- To avoid having to check if a or b are NULL:
+
+ ::
+
+ STREQ_NULLABLE(a, b)
+ STRNEQ_NULLABLE(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 replacement
+functions provided by libvirt:
+
+::
+
+ virStrncpy(char *dest, const char *src, size_t n, size_t destbytes)
+
+The first two arguments have the same meaning as for strncpy,
+namely the destination and source of the copy operation. Unlike
+strncpy, the function will always copy exactly the number of bytes
+requested and make sure the destination is NULL-terminated, as the
+source is required to be; sanity checks are performed to ensure
+the size of the destination, as specified by the last argument, is
+sufficient for the operation to succeed. On success, 0 is
+returned; on failure, a value <0 is returned instead.
+
+::
+
+ 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.
+
+::
+
+ 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.
+
+::
+
+ dst = g_strdup(src);
+ dst = g_strndup(src, n);
+
+You should avoid using strdup or strndup directly as they do not
+handle out-of-memory errors, and do not allow a NULL source. Use
+``g_strdup`` and ``g_strndup`` from GLib which abort on OOM and
+handle NULL source by returning NULL.
+
+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 either the
+`GString <https://developer.gnome.org/glib/stable/glib-Strings.html>`__
+type from GLib or the virBuffer API. If formatting XML or QEMU
+command line is needed, use the virBuffer API described in
+virbuffer.h, since it has helper functions for those.
+
+Typical usage is as follows:
+
+::
+
+ char *
+ somefunction(...)
+ {
+ g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+
+ ...
+
+ virBufferAddLit(&buf, "<domain>\n");
+
+ ...
+
+ if (some_error)
+ return NULL; /* g_auto will free the memory used so far */
+
+ ...
+
+ virBufferAddLit(&buf, "</domain>\n");
+
+ ...
+
+ if (virBufferCheckError(&buf) < 0)
+ return NULL;
+
+ return virBufferContentAndReset(&buf);
+ }
+
+Include files
+-------------
+
+There are now quite a large number of include files, both libvirt
+internal and external, and system includes. To manage all this
+complexity it's best to stick to the following general plan for
+all \*.c source files:
+
+::
+
+ /*
+ * Copyright notice
+ * ....
+ * ....
+ * ....
+ *
+ */
+
+ #include <config.h> Must come first in every file.
+
+ #include <stdio.h> Any system includes you need.
+ #include <string.h>
+ #include <limits.h>
+
+ #if WITH_NUMACTL Some system includes aren't supported
+ # include <numa.h> everywhere so need these #if guards.
+ #endif
+
+ #include "internal.h" Include this first, after system includes.
+
+ #include "util.h" Any libvirt internal header files.
+ #include "buf.h"
+
+ static int
+ myInternalFunc() The actual code.
+ {
+ ...
+
+Of particular note: **Do not** include libvirt/libvirt.h,
+libvirt/virterror.h, libvirt/libvirt-qemu.h, or
+libvirt/libvirt-lxc.h. They are included by "internal.h" already
+and there are some special reasons why you cannot include these
+files explicitly. One of the special cases, "libvirt/libvirt.h" is
+included prior to "internal.h" in "remote_protocol.x", to avoid
+exposing \*_LAST enum elements.
+
+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 virCommandAddEnvFormat in
+vircommand.h:
+
+::
+
+ void virCommandAddEnvFormat(virCommandPtr cmd, const char *format, ...)
+ G_GNUC_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.
+
+When printing to a string, consider using GString or virBuffer for
+incremental allocations, g_strdup_printf for a one-shot
+allocation, and g_snprintf for fixed-width buffers. Only use
+g_sprintf, if you can prove the buffer won't overflow.
+
+Error message format
+--------------------
+
+Error messages visible to the user should be short and
+descriptive. All error messages are translated using gettext and
+thus must be wrapped in ``_()`` macro. To simplify the translation
+work, the error message must not be concatenated from various
+parts. To simplify searching for the error message in the code the
+strings should not be broken even if they result into a line
+longer than 80 columns and any formatting modifier should be
+enclosed by quotes or other obvious separator. If a string used
+with ``%s`` can be NULL the NULLSTR macro must be used.
+
+::
+
+ GOOD: virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to connect to remote host '%s'"), hostname)
+
+ BAD: virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to %s to remote host '%s'"),
+ "connect", hostname);
+
+ BAD: virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to connect "
+ "to remote host '%s'),
+ hostname);
+
+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. g_free()
+and most of the functions named XXXFree() in libvirt is required
+to handle NULL as its arg. This does not apply to libvirt's public
+APIs. 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. Note that most of libvirt's type declarations can
+be marked with either ``g_autofree`` or ``g_autoptr`` which uses
+the compiler's ``__attribute__((cleanup))`` that calls the
+appropriate free function when the variable goes out of scope.
+
+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.
+
+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
+`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 (e.g., retry on EINTR)
+
+Top-level labels should be indented by one space (putting them on
+the beginning of the line confuses function context detection in
+git):
+
+::
+
+ int foo()
+ {
+ /* ... do stuff ... */
+ cleanup:
+ /* ... do other stuff ... */
+ }
+
+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 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.
+
+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.
+- (ir)regular pulls from other repositories or automated updates,
+ such as the keycodemap submodule updates, pulling in new
+ translations or updating the container images for the CI system
--
2.25.1
More information about the libvir-list
mailing list