RELIABILITY: Fix memory leaks and dereferences of uninitialized pointers.

The 'total' struct and 'm' array in struct reliability were allocated once
per case group, but freed exactly once.  If there were no case groups,
this caused a null dereference.  If there was more than one case group,
this caused a memory leak.  This commit fixes both problems.

This commit also fixes a related crash on bad syntax due to dereferencing
a null pointer in reliability_destroy().

Bug #38660.
Reported by William Rogers.

diff --git a/src/language/stats/reliability.c b/src/language/stats/reliability.c
index 695beabb5706d863dd716e5d24e1612afb9c777e..a4c4b862fdb344f98a3cf2a49cce3f2ee39265ba 100644 (file)
--- a/src/language/stats/reliability.c
+++ b/src/language/stats/reliability.c
@@ -118,9 +118,10 @@ reliability_destroy (struct reliability *rel)
       {
        int x;
        free (rel->sc[j].items);
-       moments1_destroy (rel->sc[j].total);
-       for (x = 0; x < rel->sc[j].n_items; ++x)
-         free (rel->sc[j].m[x]);
+        moments1_destroy (rel->sc[j].total);
+        if (rel->sc[j].m)
+          for (x = 0; x < rel->sc[j].n_items; ++x)
+            free (rel->sc[j].m[x]);
        free (rel->sc[j].m);
       }
 
@@ -374,6 +375,20 @@ run_reliability (struct dataset *ds, const struct reliability *reliability)
   struct casereader *group;
 
   struct casegrouper *grouper = casegrouper_create_splits (proc_open (ds), dict);
+  int si;
+
+  for (si = 0 ; si < reliability->n_sc; ++si)
+    {
+      struct cronbach *s = &reliability->sc[si];
+      int i;
+
+      s->m = xzalloc (sizeof *s->m * s->n_items);
+      s->total = moments1_create (MOMENT_VARIANCE);
+
+      for (i = 0 ; i < s->n_items ; ++i )
+       s->m[i] = moments1_create (MOMENT_VARIANCE);
+    }
+
 
   while (casegrouper_get_next_group (grouper, &group))
     {
@@ -437,11 +452,10 @@ do_reliability (struct casereader *input, struct dataset *ds,
     {
       struct cronbach *s = &rel->sc[si];
 
-      s->m = xzalloc (sizeof (s->m) * s->n_items);
-      s->total = moments1_create (MOMENT_VARIANCE);
+      moments1_clear (s->total);
 
       for (i = 0 ; i < s->n_items ; ++i )
-       s->m[i] = moments1_create (MOMENT_VARIANCE);
+        moments1_clear (s->m[i]);
     }
 
   input = casereader_create_filter_missing (input,

diff --git a/tests/language/stats/reliability.at b/tests/language/stats/reliability.at
index f320d3e6f38246772e474e4e43c3173b541f2d8d..dcae7d517ad5e44e367e70f01a425e293327d846 100644 (file)
--- a/tests/language/stats/reliability.at
+++ b/tests/language/stats/reliability.at
@@ -268,3 +268,32 @@ reliability x y.
 AT_CHECK([pspp -O format=csv bad-syntax.sps], [1], [ignore])
 
 AT_CLEANUP
+
+dnl Checks for a crash when bad syntax followed scale specification.
+AT_SETUP([RELIABILITY bad syntax 2])
+AT_DATA([bad-syntax.sps], [dnl
+new file.
+data list notable list /f01 f02 f03 f04 f05 f06 f07 f08 f09 f10 *.
+begin data.
+end data.
+
+* This syntax is incorrect
+reliability variables=f01 to f10/asdfj.
+])
+AT_CHECK([pspp -O format=csv bad-syntax.sps], [1], [ignore])
+AT_CLEANUP
+
+
+dnl Checks for a crash when the active file was empty.  Bug #38660.
+AT_SETUP([RELIABILITY crash with no data])
+AT_DATA([reliability.sps], [dnl
+new file.
+data list notable list /f01 f02 f03 f04 f05 f06 f07 f08 f09 f10 *.
+begin data.
+end data.
+
+reliability variables=f01 to f10.
+])
+AT_CHECK([pspp -O format=csv reliability.sps], [0], [])
+AT_CLEANUP
+