Re: do_summary_box() bugs?

[Ben Pfaff]

John Darrington <address@hidden> writes:

> On Mon, Feb 15, 2010 at 03:27:49PM -0800, Ben Pfaff wrote:
>      The "Clang" static analyzer found what looks to me like a bug in
>      do_summary_box(), in npar-summary.c.
>      
>      It reports that "desc" can be a null pointer when it is
>      dereferenced in the "for" loop at the end.  Indeed, it looks to
>      me as if it can be, as long as "desc" can be null, and the code
>      itself thinks that it can be, since it tests whether it is
>      nonnull.
>      
>      On my own, I think I see another problem: the code appears to add
>      headings for quartiles, but I do not see any code that would
>      insert the quartiles themselves.
>      
>      Let me know what you think.  I'm happy to file a bug report if
>      that's what you want.
>
> Please do - or fix it yourself. Thanks.

OK.  I am preparing to push the appended commit, once it passes
checks.

I was also planning to add code to display the quartiles, but
then I noticed that no values for quartiles are actually passed
in.  Do you know where they are supposed to come from?

commit 32b1cdd68664c94211ff4427f68f8247b6c7941a
Author: Ben Pfaff <address@hidden>
Date:   Sat Feb 20 11:11:18 2010 -0800

    NPAR TESTS: Avoid segfault in do_summary_box() if descriptives disabled.
    
    Found by Clang (http://clang-analyzer.llvm.org).

diff --git a/src/language/stats/npar-summary.c 
b/src/language/stats/npar-summary.c
index 05fa1d0..6252913 100644
--- a/src/language/stats/npar-summary.c
+++ b/src/language/stats/npar-summary.c
@@ -1,5 +1,5 @@
 /* PSPP - a program for statistical analysis.
-   Copyright (C) 2006, 2009 Free Software Foundation, Inc.
+   Copyright (C) 2006, 2009, 2010 Free Software Foundation, Inc.
 
    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -160,11 +160,15 @@ do_summary_box (const struct descriptives *desc,
 
       tab_text (table, 0, 2 + v, 0, var_to_string (var));
 
-      tab_double (table, 1, 2 + v, 0, desc[v].n, fmt);
-      tab_double (table, 2, 2 + v, 0, desc[v].mean, fmt);
-      tab_double (table, 3, 2 + v, 0, desc[v].std_dev, fmt);
-      tab_double (table, 4, 2 + v, 0, desc[v].min, fmt);
-      tab_double (table, 5, 2 + v, 0, desc[v].max, fmt);
+      col = 1;
+      if (desc != NULL)
+        {
+          tab_double (table, col++, 2 + v, 0, desc[v].n, fmt);
+          tab_double (table, col++, 2 + v, 0, desc[v].mean, fmt);
+          tab_double (table, col++, 2 + v, 0, desc[v].std_dev, fmt);
+          tab_double (table, col++, 2 + v, 0, desc[v].min, fmt);
+          tab_double (table, col++, 2 + v, 0, desc[v].max, fmt);
+        }
     }
 
 

-- 
Ben Pfaff 
http://benpfaff.org