[libvirt] [PATCH 11/13] hacking: Add more information about Valgrind

[John Ferlan]

---
 docs/hacking.html.in | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 93 insertions(+), 3 deletions(-)

diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index fe4324a..7ef826c 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -109,8 +109,9 @@
   make syntax-check
   make -C tests valgrind
 </pre>
-        <p>
-          The latter test checks for memory leaks.
+        <p><a href="http://valgrind.org/">Valgrind</a> is a test that checks
+        for memory management issues, such as leaks or use of uninitialized
+        variables.
         </p>
 
         <p>
@@ -134,7 +135,96 @@
         <p>There is also a <code>./run</code> script at the top level,
           to make it easier to run programs that have not yet been
           installed, as well as to wrap invocations of various tests
-          under gdb or valgrind.</p>
+          under gdb or Valgrind.
+        </p>
+
+      </li>
+      <li><p>The Valgrind test should produce similar output to
+          <code>make check</code>. If the output has traces within libvirt
+          API's, then investigation is required in order to determine the
+          cause of the issue. Output such as the following indicates some
+          sort of leak:
+        </p>
+<pre>
+==5414== 4 bytes in 1 blocks are definitely lost in loss record 3 of 89
+==5414==    at 0x4A0881C: malloc (vg_replace_malloc.c:270)
+==5414==    by 0x34DE0AAB85: xmlStrndup (in /usr/lib64/libxml2.so.2.7.8)
+==5414==    by 0x4CC97A6: virDomainVideoDefParseXML (domain_conf.c:7410)
+==5414==    by 0x4CD581D: virDomainDefParseXML (domain_conf.c:10188)
+==5414==    by 0x4CD8C73: virDomainDefParseNode (domain_conf.c:10640)
+==5414==    by 0x4CD8DDB: virDomainDefParse (domain_conf.c:10590)
+==5414==    by 0x41CB1D: testCompareXMLToArgvHelper (qemuxml2argvtest.c:100)
+==5414==    by 0x41E20F: virtTestRun (testutils.c:161)
+==5414==    by 0x41C7CB: mymain (qemuxml2argvtest.c:866)
+==5414==    by 0x41E84A: virtTestMain (testutils.c:723)
+==5414==    by 0x34D9021734: (below main) (in /usr/lib64/libc-2.15.so)
+</pre>
+        <p>In this example, the <code>virDomainDefParseXML()</code> had
+           an error path where the <code>virDomainVideoDefPtr video</code>
+           pointer was not properly disposed. By simply adding a
+           <code>virDomainVideoDefFree(video);</code> in the error path,
+           the issue was resolved.
+        </p>
+
+        <p>Another common mistake is calling a printing function, such as
+           <code>VIR_DEBUG()</code> without initializing a variable to be
+           printed.  The following example involved a call which could return
+           an error, but not set variables passed by reference to the call.
+           The solution was to initialize the variables prior to the call.
+        </p>
+<pre>
+==4749== Use of uninitialised value of size 8
+==4749==    at 0x34D904650B: _itoa_word (in /usr/lib64/libc-2.15.so)
+==4749==    by 0x34D9049118: vfprintf (in /usr/lib64/libc-2.15.so)
+==4749==    by 0x34D9108F60: __vasprintf_chk (in /usr/lib64/libc-2.15.so)
+==4749==    by 0x4CAEEF7: virVasprintf (stdio2.h:199)
+==4749==    by 0x4C8A55E: virLogVMessage (virlog.c:814)
+==4749==    by 0x4C8AA96: virLogMessage (virlog.c:751)
+==4749==    by 0x4DA0056: virNetTLSContextCheckCertKeyUsage (virnettlscontext.c:225)
+==4749==    by 0x4DA06DB: virNetTLSContextCheckCert (virnettlscontext.c:439)
+==4749==    by 0x4DA1620: virNetTLSContextNew (virnettlscontext.c:562)
+==4749==    by 0x4DA26FC: virNetTLSContextNewServer (virnettlscontext.c:927)
+==4749==    by 0x409C39: testTLSContextInit (virnettlscontexttest.c:467)
+==4749==    by 0x40AB8F: virtTestRun (testutils.c:161)
+</pre>
+        <p>Valgrind will also find some false positives or code paths
+           which cannot be resolved by making changes to the libvirt code.
+           For these paths, it is possible to add a filter to avoid the
+           errors. For example:
+        </p>
+<pre>
+==4643== 7 bytes in 1 blocks are possibly lost in loss record 4 of 20
+==4643==    at 0x4A0881C: malloc (vg_replace_malloc.c:270)
+==4643==    by 0x34D90853F1: strdup (in /usr/lib64/libc-2.15.so)
+==4643==    by 0x34EEC2C08A: ??? (in /usr/lib64/libnl.so.1.1)
+==4643==    by 0x34EEC15B81: ??? (in /usr/lib64/libnl.so.1.1)
+==4643==    by 0x34D8C0EE15: call_init.part.0 (in /usr/lib64/ld-2.15.so)
+==4643==    by 0x34D8C0EECF: _dl_init (in /usr/lib64/ld-2.15.so)
+==4643==    by 0x34D8C01569: ??? (in /usr/lib64/ld-2.15.so)
+
+</pre>
+        <p>In this instance, it is acceptible to modify the
+           <code>tests/.valgrind.supp</code> file in order to add a
+           suppression filter. The filter should be unique enough to
+           not suppress real leaks, but it should be generic enough to
+           cover multiple code paths. The format of the entry can be
+           found in the documentation found at the
+           <a href="http://valgrind.org/">Valgrind home page.</a>
+           The following trace was added to <code>tests/.valgrind.supp</code>
+           in order to suppress the warning:
+        </p>
+<pre>
+{
+    dlInitMemoryLeak1
+    Memcheck:Leak
+    fun:?alloc
+    ...
+    fun:call_init.part.0
+    fun:_dl_init
+    ...
+    obj:*/lib*/ld-2.*so*
+}
+</pre>
       </li>
       <li>Update tests and/or documentation, particularly if you are adding
         a new feature or changing the output of a program.</li>
-- 
1.7.11.7