Don't count leaks as errors with --leak-check=summary, because the results

can be confusing.  Document the behaviour.

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@10798

diff --git a/memcheck/docs/mc-manual.xml b/memcheck/docs/mc-manual.xml
index df04c663a475f04f395503e2db1563ca6949cf78..3ee00f21dc69193b8582aeae2eb68ba3e1c47c39 100644 (file)
--- a/memcheck/docs/mc-manual.xml
+++ b/memcheck/docs/mc-manual.xml
@@ -568,6 +568,32 @@ two examples show.</para>
    by 0x........: main (leak-cases.c:80)
 ]]></programlisting>
 
+<para>Because there are different kinds of leaks with different severities, an
+interesting question is this: which leaks should be counted as true "errors"
+and which should not?  The answer to this question affects the numbers printed
+in the <computeroutput>ERROR SUMMARY</computeroutput> line, and also the effect
+of the <option>--error-exitcode</option> option.  Memcheck uses the following
+criteria:</para>
+
+<itemizedlist>
+  <listitem>
+    <para>First, a leak is only counted as a true "error" if
+    <option>--leak-check=full</option> is specified.  In other words, an
+    unprinted leak is not considered a true "error".  If this were not the
+    case, it would be possible to get a high error count but not have any
+    errors printed, which would be confusing.</para>
+  </listitem>
+
+  <listitem>
+    <para>After that, definitely lost and possibly lost blocks are counted as
+    true "errors".  Indirectly lost and still reachable blocks are not counted
+    as true "errors", even if <option>--show-reachable=yes</option> is
+    specified and they are printed;  this is because such blocks don't need
+    direct fixing by the programmer.
+    </para>
+  </listitem>
+</itemizedlist>
+
 </sect2>
 
 </sect1>

diff --git a/memcheck/mc_leakcheck.c b/memcheck/mc_leakcheck.c
index ce9a6e992c0b64d3f5fb2741b430f344b556d6ff..3893f27b366842f6ab115dbede7404b8271200ca 100644 (file)
--- a/memcheck/mc_leakcheck.c
+++ b/memcheck/mc_leakcheck.c
@@ -842,13 +842,19 @@ static void print_results(ThreadId tid, Bool is_full_check)
       // includes indirectly lost blocks!
       //
       lr = lr_array[i];
-      // You could argue that indirect leaks should be counted as errors, but
-      // it seems better to make the counting criteria similar to the printing
-      // criteria.  So we don't count them.
-      count_as_error = Unreached == lr->key.state || 
-                       Possible  == lr->key.state;
       print_record = is_full_check &&
-                     ( MC_(clo_show_reachable) || count_as_error );
+                     ( MC_(clo_show_reachable) ||
+                       Unreached == lr->key.state || 
+                       Possible  == lr->key.state );
+      // We don't count a leaks as errors with --leak-check=summary.
+      // Otherwise you can get high error counts with few or no error
+      // messages, which can be confusing.  Also, you could argue that
+      // indirect leaks should be counted as errors, but it seems better to
+      // make the counting criteria similar to the printing criteria.  So we
+      // don't count them.
+      count_as_error = is_full_check && 
+                       ( Unreached == lr->key.state || 
+                         Possible  == lr->key.state );
       is_suppressed = 
          MC_(record_leak_error) ( tid, i+1, n_lossrecords, lr, print_record,
                                   count_as_error );