From nobody Mon Feb 9 11:29:34 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 205.139.110.120 as permitted sender) client-ip=205.139.110.120; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-1.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 205.139.110.120 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1586190039; cv=none; d=zohomail.com; s=zohoarc; b=ln6bkU/ozj/VqfPp9//dM+mWv++RdE6oBXba46wKTbjTAsA49X32AXCW4mNcz7jCNjaqgfkNwxCXTw0lhjpRIbSWM+uqvvr5n36PmUvhIyUQAPQ0oW8G14wbUUHb3IbMv2S85v0XMnDXMCvssbJzIaOwaEbZj1Z7cJLcZrH1LVU= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1586190039; h=Content-Type:Content-Transfer-Encoding:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=VQsIp9FFI3AVzswx2T82CPu5GjHNb7yWlhNAzOTEh3s=; b=gmOHIwO28VYHw3xb0DpQ70HZYCl2MdcmXPOl1v5MbSmj3HL3LvR7ZWnyIGiPNaurHkwsrxR6Xg+3uhGu1mdvXigMLCdalBCHlxqt/pZsWwoqi5K1wliRIbJky9rSAd0YgyMfGmlp5U86/RNjGP5U7PU/lGg7CKcyzUbOGWG22kg= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 205.139.110.120 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) header.from= Return-Path: Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by mx.zohomail.com with SMTPS id 1586190038993818.2302612162125; Mon, 6 Apr 2020 09:20:38 -0700 (PDT) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-28-yKsFHQguPhyAvKG6mqN_Zw-1; Mon, 06 Apr 2020 12:20:31 -0400 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 83A72800D50; Mon, 6 Apr 2020 16:20:25 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 14B52100034E; Mon, 6 Apr 2020 16:20:25 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 2814418089CD; Mon, 6 Apr 2020 16:20:23 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 036GKLBo027882 for ; Mon, 6 Apr 2020 12:20:21 -0400 Received: by smtp.corp.redhat.com (Postfix) id C6594B19D3; Mon, 6 Apr 2020 16:20:21 +0000 (UTC) Received: from kinshicho.usersys.redhat.com (unknown [10.40.196.71]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7DE67B19CB for ; Mon, 6 Apr 2020 16:20:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1586190037; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=VQsIp9FFI3AVzswx2T82CPu5GjHNb7yWlhNAzOTEh3s=; b=SIA0vHPz7OT4m3tsvml9Qy41XC2BdOMpQulbq1Mcshl7nypRLep2hVYnP4yHGCpq5d+o1b tvZ2Jzh/2AsNxNl5c35sbPeIFtVQ7Wd0DnDUgym7WIXTMZQCBX//TyVkDJ2Vtd/V7Lk6B2 RieXBMKGUVW09ga2RUypt/PajaPnx2A= X-MC-Unique: yKsFHQguPhyAvKG6mqN_Zw-1 From: Andrea Bolognani To: libvir-list@redhat.com Subject: [libvirt PATCH 01/10] docs: Convert hacking.html to reStructuredText Date: Mon, 6 Apr 2020 18:20:01 +0200 Message-Id: <20200406162010.332813-2-abologna@redhat.com> In-Reply-To: <20200406162010.332813-1-abologna@redhat.com> References: <20200406162010.332813-1-abologna@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-loop: libvir-list@redhat.com X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) Content-Type: text/plain; charset="utf-8" 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 Reviewed-by: Daniel P. Berrang=C3=A9 --- 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_fi= le_name =3D \ ^(build-aux/syntax-check\.mk|tests/virfilemock\.c)$$ =20 exclude_file_name_regexp--sc_prohibit_raw_allocation =3D \ - ^(docs/hacking\.html\.in|src/util/viralloc\.[ch]|examples/.*|tests/(secu= rityselinuxhelper|(vircgroup|nss)mock|commandhelper)\.c|tools/wireshark/src= /packet-libvirt\.c|tools/nss/libvirt_nss(_leases|_macs)?\.c|build-aux/usele= ss-if-before-free)$$ + ^(docs/hacking\.rst|src/util/viralloc\.[ch]|examples/.*|tests/(securitys= elinuxhelper|(vircgroup|nss)mock|commandhelper)\.c|tools/wireshark/src/pack= et-libvirt\.c|tools/nss/libvirt_nss(_leases|_macs)?\.c|build-aux/useless-if= -before-free)$$ =20 exclude_file_name_regexp--sc_prohibit_readlink =3D \ ^src/(util/virutil|lxc/lxc_container)\.c$$ @@ -2048,7 +2048,7 @@ exclude_file_name_regexp--sc_prohibit_readlink =3D \ exclude_file_name_regexp--sc_prohibit_setuid =3D ^src/util/virutil\.c|tool= s/virt-login-shell\.c$$ =20 exclude_file_name_regexp--sc_prohibit_snprintf =3D \ - ^(build-aux/syntax-check\.mk|docs/hacking\.html\.in|tools/virt-login-she= ll\.c)$$ + ^(build-aux/syntax-check\.mk|docs/hacking\.rst|tools/virt-login-shell\.c= )$$ =20 exclude_file_name_regexp--sc_prohibit_strtol =3D ^examples/.*$$ =20 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 @@ - - - - -

Contributor guidelines

- -
    - -

    General tips for contributing patches

    -
      -
    1. -

      Discuss any large changes on the mailing list first. Post patc= hes - early and listen to feedback.

      -
      2. - -
    2. -

      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<= /a>.

      -
      3. - -
    3. -

      Patches to translations are maintained via - the zanata project. - 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.

      -
      4. - -

    4. The simplest way to send patches is to use the - git-publ= ish - tool. All libvirt-related repositories contain a config file that - tells git-publish to use the correct mailing list and subject pref= ix.

      -

      Alternatively, you may send patches using git send-email<= /code>.

      -

      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. When you want to - post your patches:

      -
      -  git pull --rebase
-  (fix any conflicts)
-  git send-email --cover-letter --no-chain-reply-to --annotate \
-                 --confirm=3Dalways --to=3Dlibvir-list@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=3Dlibvir-list@redhat.com all the time, you can - configure that to be automatically handled as well:

      -
      -  git config sendemail.to libvir-list@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-li= st - archives 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 a= nd - 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 h= elps - reviewers but doesn't become part of git history. Moreover, such p= atch - needs to be prefixed correctly with - --subject-prefix=3DPATCHv2 appended to git - send-email (substitute v2 with the correct - version if needed though).

      -
      5. - -

    5. 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. -

      -
      6. - -

    6. Contributors to libvirt projects must - assert that they are in compliance with the - Developer - Certificate of Origin 1.1. 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. -

      7. - -

    7. 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).

      -
      8. - -
    8. -

      Make sure your patches apply against libvirt GIT. Developers - only follow GIT and don't care much about released versions.

      -
      9. - -

    9. Run the automated tests on your code before submitting any ch= anges. - That is: -

      -
      -  make check
-  make syntax-check
-  make -C tests valgrind
-
      -

      Valgrind is a test that ch= ecks - 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=3D1
-
      -

      - 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=3D1 make check    (or)
-  VIR_TEST_DEBUG=3D2 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=3D1 VIR_TEST_RANGE=3D3-5,7-20,^16 ./run tests/qemuxml2arg=
vtest
-
      - -

      - 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=3D1 ./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 c= atch - this kind of errors, the test suite has a module for that prints a= ny - 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=3D1 VIR_TEST_FILE_ACCESS_OUTPUT=3D"/tmp/file_access=
.txt" ./qemuxml2argvtest
-
      - -
      10. -

    10. 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: -

      -
      -=3D=3D5414=3D=3D 4 bytes in 1 blocks are definitely lost in loss record 3 =
of 89
-=3D=3D5414=3D=3D    at 0x4A0881C: malloc (vg_replace_malloc.c:270)
-=3D=3D5414=3D=3D    by 0x34DE0AAB85: xmlStrndup (in /usr/lib64/libxml2.so.=
2.7.8)
-=3D=3D5414=3D=3D    by 0x4CC97A6: virDomainVideoDefParseXML (domain_conf.c=
:7410)
-=3D=3D5414=3D=3D    by 0x4CD581D: virDomainDefParseXML (domain_conf.c:1018=
8)
-=3D=3D5414=3D=3D    by 0x4CD8C73: virDomainDefParseNode (domain_conf.c:106=
40)
-=3D=3D5414=3D=3D    by 0x4CD8DDB: virDomainDefParse (domain_conf.c:10590)
-=3D=3D5414=3D=3D    by 0x41CB1D: testCompareXMLToArgvHelper (qemuxml2argvt=
est.c:100)
-=3D=3D5414=3D=3D    by 0x41E20F: virtTestRun (testutils.c:161)
-=3D=3D5414=3D=3D    by 0x41C7CB: mymain (qemuxml2argvtest.c:866)
-=3D=3D5414=3D=3D    by 0x41E84A: virtTestMain (testutils.c:723)
-=3D=3D5414=3D=3D    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 ret= urn - an error, but not set variables passed by reference to the call. - The solution was to initialize the variables prior to the call. -

      -
      -=3D=3D4749=3D=3D Use of uninitialised value of size 8
-=3D=3D4749=3D=3D    at 0x34D904650B: _itoa_word (in /usr/lib64/libc-2.15.s=
o)
-=3D=3D4749=3D=3D    by 0x34D9049118: vfprintf (in /usr/lib64/libc-2.15.so)
-=3D=3D4749=3D=3D    by 0x34D9108F60: __vasprintf_chk (in /usr/lib64/libc-2=
.15.so)
-=3D=3D4749=3D=3D    by 0x4CAEEF7: virVasprintf (stdio2.h:199)
-=3D=3D4749=3D=3D    by 0x4C8A55E: virLogVMessage (virlog.c:814)
-=3D=3D4749=3D=3D    by 0x4C8AA96: virLogMessage (virlog.c:751)
-=3D=3D4749=3D=3D    by 0x4DA0056: virNetTLSContextCheckCertKeyUsage (virne=
ttlscontext.c:225)
-=3D=3D4749=3D=3D    by 0x4DA06DB: virNetTLSContextCheckCert (virnettlscont=
ext.c:439)
-=3D=3D4749=3D=3D    by 0x4DA1620: virNetTLSContextNew (virnettlscontext.c:=
562)
-=3D=3D4749=3D=3D    by 0x4DA26FC: virNetTLSContextNewServer (virnettlscont=
ext.c:927)
-=3D=3D4749=3D=3D    by 0x409C39: testTLSContextInit (virnettlscontexttest.=
c:467)
-=3D=3D4749=3D=3D    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: -

      -
      -=3D=3D4643=3D=3D 7 bytes in 1 blocks are possibly lost in loss record 4 of=
 20
-=3D=3D4643=3D=3D    at 0x4A0881C: malloc (vg_replace_malloc.c:270)
-=3D=3D4643=3D=3D    by 0x34D90853F1: strdup (in /usr/lib64/libc-2.15.so)
-=3D=3D4643=3D=3D    by 0x34EEC2C08A: ??? (in /usr/lib64/libnl.so.1.1)
-=3D=3D4643=3D=3D    by 0x34EEC15B81: ??? (in /usr/lib64/libnl.so.1.1)
-=3D=3D4643=3D=3D    by 0x34D8C0EE15: call_init.part.0 (in /usr/lib64/ld-2.=
15.so)
-=3D=3D4643=3D=3D    by 0x34D8C0EECF: _dl_init (in /usr/lib64/ld-2.15.so)
-=3D=3D4643=3D=3D    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. - 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*
-}
-
      -
      11. - -
    11. -

      Update tests and/or documentation, particularly if you are addi= ng - a new feature or changing the output of a program.

      -
      12. - -
    12. -

      Don't forget to update the release notes<= /a> - by changing docs/news.xml if your changes are - significant. All user-visible changes, such as adding new XML elem= ents - or fixing all but the most obscure bugs, must be (briefly) describ= ed - 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 upda= ted - in its own commit not to get in the way of backports.

      -
      13. -
    - -

    - 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. -

    - -

    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 develope= rs, - 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 t= hem - without little or no configuration. Examples include: -

    - -
      -
    • - color_coded, - a vim plugin for libclang-powered semantic syntax highlighting; -
      • - -
    • - 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 follo= wed - when creating any entirely new files in libvirt. When working on exi= sting - files, while it is desirable to apply these conventions, keeping a - consistent style with existing code in that particular file is gener= ally - more important. The overall guiding principal is that every file, en= um, - 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-formatt= ing - conventions. The existing code base is not totally consistent on th= is - 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=3D8
-  set shiftwidth=3D4
-  set expandtab
-  set cinoptions=3D(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 libvi= rt - source, then install a vim script from - http://www.vim.org/scripts/script.php?script_id=3D1408, - 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 co= de. - Note that this also gives you an idea of the type of spacing we pref= er - 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 ca= n get through. - Usually they're in macro definitions or strings, and should be conve= rted - 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 whitespa= ce - 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 =3D foo (wizz);  // Bad
-      bar =3D 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 =3D 0 ;i < limit ; ++i) { // Bad
-      for (i =3D 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 =3D waitpid(pid, &st, 0) =3D=3D -1) &&
-             errno =3D=3D EINTR); // ok
-      while ((rc =3D waitpid(pid, &st, 0) =3D=3D -1) &&
-             errno =3D=3D 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 b= ody. -

    - -
    -  while (expr)             // single line body; {} is forbidden
-      single_line_stmt();
-
    - -
    -  while (expr(arg1,
-              arg2))      // indentation makes it obvious it is single lin=
e,
-      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 =3D y;    // BAD: braceless "else" with braced "then",
-                // and short block last
-
-  if (expr)
-      x =3D 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 th= an - after the more involved block: -

    - -
    -  if (!expr) {
-    x =3D 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 =3D 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[][] =3D {
-        {                    // 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 =3D b;
-              b =3D a;
-              a =3D tmp;
-          }
-      }
-  }
-
    - -

    Conditional expressions

    -

    For readability reasons new code should avoid shortening comparis= ons - to 0 for numeric types. Boolean and pointer comparisions may be - shortened. All long forms are okay: -

    -
    -   virFooPtr foos =3D NULL;
-   size nfoos =3D 0;
-   bool hasFoos =3D false;
-
-GOOD:
-    if (!foos)
-    if (!hasFoos)
-    if (nfoos =3D=3D 0)
-    if (foos =3D=3D NULL)
-    if (hasFoos =3D=3D 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 =3D baz ?
-                virDoSomethingReallyComplex(driver, vm, something, baz->fo=
o) :
-                NULL;
-
-    char *foo =3D 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 mu= st be constant across - all systems, like public interfaces and on-the-wire protocols.= Note - that it would be possible (albeit wasteful) to use bool<= /code> 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, tru= e, - since a value with a logical non-false value need not be 1. - I.e., don't write if (seen =3D=3D true) .... Rat= her, - write if (seen)....
        • -
      -
      • -
    - -

    - Of course, take all of the above with a grain of salt. If you're ab= out - to use some system interface that requires a type like size_t<= /code>, - 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, a= nd that - conflicts with the signedness of a related variable, sometimes - it's best just to use the wrong type, if pulling the threa= d - 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: -

    - - - - - - - - - - - - -
    MacroMeaning
    ATTRIBUTE_NONNULLpassing NULL for th= is parameter is not allowed
    ATTRIBUTE_PACKEDforce a structure to= be packed
    G_GNUC_FALLTHROUGHallow code reuse b= y multiple switch cases
    G_GNUC_NO_INLINEthe function is mock= ed in the test suite
    G_GNUC_NORETURNthe function never re= turns
    G_GNUC_NULL_TERMINATEDlast parameter= must be NULL
    G_GNUC_PRINTFvalidate that the forma= tting string matches parameters
    G_GNUC_UNUSEDparameter is unused in = this implementation of the function
    G_GNUC_WARN_UNUSED_RESULTthe return = value must be checked
    - -

    Adoption of GLib APIs

    - -

    - Libvirt has adopted use of the - GLib library. - 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 inste= ad, - which has a simpler calling convention as an added benefit.
    - - - = - - - - - - - - - - - - - - - - - -
    deprecated versionGLib versionNotes
    VIR_ALLOC(var)g_new0(var_t, 1)= the type needs to be passed explicitly
    VIR_ALLOC_Ng_new0(var_t, n)
    VIR_REALLOC_Ng_renew(var_t, pt= r, n)the newly added memory is not zeroed
    VIR_EXPAND_Ng_renew(var_t, ptr= , n)zero the new memory manually or use an array type
    VIR_SHRINK_Ng_renew(var_t, ptr= , n)
    VIR_APPEND_ELEMENTg_array_appe= nd_valg_ptr_array_add or g_byte_array_append<= /code>
    VIR_INSERT_ELEMENTg_array_inse= rt_valg_ptr_array_insert
    VIR_DELETE_ELEMENTg_array_remo= ve_indexg_ptr_array_remove_index or g_byte_arra= y_remove_index
    VIR_FREEg_freeg_free does not zero the pointer
    - -

    String allocation macros and functions:

    - - = - - - -
    deprecated versionGLib versionNotes
    virAsprintfg_strdup_printf
    virVasprintfg_strdup_vprintuse g_vasprintf if you really need to know th= e returned length
    -
    - -

    - 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 macr= os 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 cle= anup function - is declared with the libvirt macros and vice-versa. -
    - -
    VIR_AUTOUNREF
    -
    The GLib macros g_autoptr and G_DEFINE_AUTOPT= R_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 versionGLib versionNotes
    VIR_AUTOPTRg_autoptr
    VIR_AUTOCLEANg_auto
    VIR_AUTOFREEg_autofree<= /td>The GLib version does not use parentheses
    VIR_AUTOUNREFg_autoptr<= /td>The cleanup function needs to be defined
    VIR_DEFINE_AUTOPTR_FUNCG_DEFIN= E_AUTOPTR_CLEANUP_FUNC
    VIR_DEFINE_AUTOCLEAN_FUNCG_DEF= INE_AUTO_CLEANUP_CLEAR_FUNC
    VIR_STEAL_PTRg_steal_pointera =3D f(&b) instead of f(a, b)
    VIR_RETURN_PTRreturn g_steal_p= ointer
    ARRAY_CARDINALITYG_N_ELEMENTS<= /code>
    ATTRIBUTE_FALLTHROUGHG_GNUC_FA= LLTHROUGH
    ATTRIBUTE_FMT_PRINTFG_GNUC_PRI= NTF
    ATTRIBUTE_NOINLINEG_GNUC_NO_IN= LINE
    ATTRIBUTE_NORETURNG_GNUC_NORET= URN
    ATTRIBUTE_RETURN_CHECKG_GNUC_W= ARN_UNUSED_RESULT
    ATTRIBUTE_SENTINELG_GNUC_NULL_= TERMINATED
    ATTRIBUTE_UNUSEDG_GNUC_UNUSED<= /code>
    VIR_STRDUPg_strdup
    VIR_STRNDUPg_strndup
    virStrerrorg_strerror
    - - -

    File handling

    - -

    - Usage of the fdopen(), close(), fclo= se() - APIs is deprecated in libvirt code base to help avoiding double-clos= ing of files - or file descriptors, which is particularly dangerous in a multi-thre= aded - application. Instead of these APIs, use the macros from virfile.h -

    - -
      -

    • Open a file from a file descriptor:

      - -
      -  if ((file =3D VIR_FDOPEN(fd, "r")) =3D=3D 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 los= ing - 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 =3D g_strdup(src);
-    dst =3D g_strndup(src, n);
-
    -

    - You should avoid using strdup or strndup directly as they do not han= dle - 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 - 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 thos= e. -

    - -

    Typical usage is as follows:

    - -
    -  char *
-  somefunction(...)
-  {
-     g_auto(virBuffer) buf =3D VIR_BUFFER_INITIALIZER;
-
-     ...
-
-     virBufferAddLit(&buf, "<domain>\n");
-     virBufferAsprintf(&buf, "  <memory>%d</memory>\n", me=
mory);
-     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 guard=
s.
-  #endif
-
-  #include "internal.h"           Include this first, after system include=
s.
-
-  #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 ca= ses, - "libvirt/libvirt.h" is included prior to "internal.h" in "remote_pro= tocol.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, her= e'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'"), hostnam=
e)
-
-  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 -

    - -

    - 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 ri= ght - 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 ga= ve an - ACK or +1 to a patch and nobody raised an objection on the list it s= hould - 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 y= ou - don't raise errors. -

    - -

    - An exception to 'review and approval on the list first' is fixing fa= ilures - 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 w= as if - trivial), and 'make check syntax-check' should pass too, before co= mmitting - anything
      • -
    • - fixes for documentation and code comments can be managed - in the same way, but still make sure they get reviewed if non-triv= ial. -
      • -
    • (ir)regular pulls from other repositories or automated updates, = such - as the keycodemap submodule updates, pulling in new translations o= r updating - the container images for the CI system -
      • -
    - - 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 @@ +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D +Contributor guidelines +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D + +.. contents:: + +General tips for contributing patches +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D + +#. 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 `__. + +#. Patches to translations are maintained via the `zanata + project `__. 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 `__ + 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 `__. + When you want to post your patches: + + :: + + git pull --rebase + (fix any conflicts) + git send-email --cover-letter --no-chain-reply-to --annotate \ + --confirm=3Dalways --to=3Dlibvir-list@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=3Dlibvir-list@redhat.com`` all + the time, you can configure that to be automatically handled as + well: + + :: + + git config sendemail.to libvir-list@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 `__ 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=3DPATCHv2`` 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 `__. 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 `__ 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=3D1 + + 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=3D1 make check (or) + VIR_TEST_DEBUG=3D2 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=3D1 VIR_TEST_RANGE=3D3-5,7-20,^16 ./run tests/qemuxml2= argvtest + + 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=3D1 ./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=3D1 VIR_TEST_FILE_ACCESS_OUTPUT=3D"/tmp/file_acc= ess.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: + + :: + + =3D=3D5414=3D=3D 4 bytes in 1 blocks are definitely lost in loss reco= rd 3 of 89 + =3D=3D5414=3D=3D at 0x4A0881C: malloc (vg_replace_malloc.c:270) + =3D=3D5414=3D=3D by 0x34DE0AAB85: xmlStrndup (in /usr/lib64/libxml= 2.so.2.7.8) + =3D=3D5414=3D=3D by 0x4CC97A6: virDomainVideoDefParseXML (domain_c= onf.c:7410) + =3D=3D5414=3D=3D by 0x4CD581D: virDomainDefParseXML (domain_conf.c= :10188) + =3D=3D5414=3D=3D by 0x4CD8C73: virDomainDefParseNode (domain_conf.= c:10640) + =3D=3D5414=3D=3D by 0x4CD8DDB: virDomainDefParse (domain_conf.c:10= 590) + =3D=3D5414=3D=3D by 0x41CB1D: testCompareXMLToArgvHelper (qemuxml2= argvtest.c:100) + =3D=3D5414=3D=3D by 0x41E20F: virtTestRun (testutils.c:161) + =3D=3D5414=3D=3D by 0x41C7CB: mymain (qemuxml2argvtest.c:866) + =3D=3D5414=3D=3D by 0x41E84A: virtTestMain (testutils.c:723) + =3D=3D5414=3D=3D 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. + + :: + + =3D=3D4749=3D=3D Use of uninitialised value of size 8 + =3D=3D4749=3D=3D at 0x34D904650B: _itoa_word (in /usr/lib64/libc-2= .15.so) + =3D=3D4749=3D=3D by 0x34D9049118: vfprintf (in /usr/lib64/libc-2.1= 5.so) + =3D=3D4749=3D=3D by 0x34D9108F60: __vasprintf_chk (in /usr/lib64/l= ibc-2.15.so) + =3D=3D4749=3D=3D by 0x4CAEEF7: virVasprintf (stdio2.h:199) + =3D=3D4749=3D=3D by 0x4C8A55E: virLogVMessage (virlog.c:814) + =3D=3D4749=3D=3D by 0x4C8AA96: virLogMessage (virlog.c:751) + =3D=3D4749=3D=3D by 0x4DA0056: virNetTLSContextCheckCertKeyUsage (= virnettlscontext.c:225) + =3D=3D4749=3D=3D by 0x4DA06DB: virNetTLSContextCheckCert (virnettl= scontext.c:439) + =3D=3D4749=3D=3D by 0x4DA1620: virNetTLSContextNew (virnettlsconte= xt.c:562) + =3D=3D4749=3D=3D by 0x4DA26FC: virNetTLSContextNewServer (virnettl= scontext.c:927) + =3D=3D4749=3D=3D by 0x409C39: testTLSContextInit (virnettlscontext= test.c:467) + =3D=3D4749=3D=3D 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: + + :: + + =3D=3D4643=3D=3D 7 bytes in 1 blocks are possibly lost in loss record= 4 of 20 + =3D=3D4643=3D=3D at 0x4A0881C: malloc (vg_replace_malloc.c:270) + =3D=3D4643=3D=3D by 0x34D90853F1: strdup (in /usr/lib64/libc-2.15.= so) + =3D=3D4643=3D=3D by 0x34EEC2C08A: ??? (in /usr/lib64/libnl.so.1.1) + =3D=3D4643=3D=3D by 0x34EEC15B81: ??? (in /usr/lib64/libnl.so.1.1) + =3D=3D4643=3D=3D by 0x34D8C0EE15: call_init.part.0 (in /usr/lib64/= ld-2.15.so) + =3D=3D4643=3D=3D by 0x34D8C0EECF: _dl_init (in /usr/lib64/ld-2.15.= so) + =3D=3D4643=3D=3D 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 `__. 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 `__ 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 `__. + +Language Usage +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D + +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 +=3D=3D=3D=3D=3D=3D=3D + +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 `__, a vim + plugin for libclang-powered semantic syntax highlighting; +- `YouCompleteMe `__, a + vim plugin for libclang-powered semantic code completion. + +Naming conventions +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D + +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 +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D + +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=3D8 + set shiftwidth=3D4 + set expandtab + set cinoptions=3D(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=3D1408, which will +load the .lvimrc only when you edit libvirt code. + +Code formatting (especially for new code) +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D + +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 =3D foo (wizz); // Bad + bar =3D 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 =3D 0 ;i < limit ; ++i) { // Bad + for (i =3D 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 =3D waitpid(pid, &st, 0) =3D=3D -1) && + errno =3D=3D EINTR); // ok + while ((rc =3D waitpid(pid, &st, 0) =3D=3D -1) && + errno =3D=3D 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 lin= e, + 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 =3D y; // BAD: braceless "else" with braced "then", + // and short block last + + if (expr) + x =3D 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 =3D 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 =3D 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[][] =3D { + { // 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 =3D b; + b =3D a; + a =3D 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 =3D NULL; + size nfoos =3D 0; + bool hasFoos =3D false; + + GOOD: + if (!foos) + if (!hasFoos) + if (nfoos =3D=3D 0) + if (foos =3D=3D NULL) + if (hasFoos =3D=3D 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 =3D baz ? + virDoSomethingReallyComplex(driver, vm, something, baz->fo= o) : + NULL; + + char *foo =3D 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 =3D=3D 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 = | ++=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D+=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D+ +| ``ATTRIBUTE_NONNULL`` | passing NULL for this parameter is not a= llowed | ++-------------------------------+-----------------------------------------= -------------------+ +| ``ATTRIBUTE_PACKED`` | force a structure to be packed = | ++-------------------------------+-----------------------------------------= -------------------+ +| ``G_GNUC_FALLTHROUGH`` | allow code reuse by multiple switch case= s | ++-------------------------------+-----------------------------------------= -------------------+ +| ``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 matc= hes parameters | ++-------------------------------+-----------------------------------------= -------------------+ +| ``G_GNUC_UNUSED`` | parameter is unused in this implementati= on 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 `__. 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 | N= otes | ++=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D+=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D+=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D+ +| ``VIR_AUTOPTR`` | ``g_autoptr`` | = | ++-------------------------------+--------------------------------------+--= -----------------------------------------+ +| ``VIR_AUTOCLEAN`` | ``g_auto`` | = | ++-------------------------------+--------------------------------------+--= -----------------------------------------+ +| ``VIR_AUTOFREE`` | ``g_autofree`` | T= he GLib version does not use parentheses | ++-------------------------------+--------------------------------------+--= -----------------------------------------+ +| ``VIR_AUTOUNREF`` | ``g_autoptr`` | T= he 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 =3D 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 =3D VIR_FDOPEN(fd, "r")) =3D=3D 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 =3D g_strdup(src); + dst =3D 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 `__ +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 =3D VIR_BUFFER_INITIALIZER; + + ... + + virBufferAddLit(&buf, "\n"); + + ... + + if (some_error) + return NULL; /* g_auto will free the memory used so far */ + + ... + + virBufferAddLit(&buf, "\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 Must come first in every file. + + #include Any system includes you need. + #include + #include + + #if WITH_NUMACTL Some system includes aren't supported + # include everywhere so need these #if guards. + #endif + + #include "internal.h" Include this first, after system include= s. + + #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'"), hostnam= e) + + 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 `__ + +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 +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D + +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 --=20 2.25.1