Clean up and improve case code.
authorBen Pfaff <blp@gnu.org>
Tue, 16 Jan 2007 15:30:28 +0000 (15:30 +0000)
committerBen Pfaff <blp@gnu.org>
Tue, 16 Jan 2007 15:30:28 +0000 (15:30 +0000)
Patch #5690.

12 files changed:
src/data/ChangeLog
src/data/case.c
src/data/case.h
src/data/casefile.c
src/data/fastfile.c
src/language/expressions/evaluate.c
src/language/stats/ChangeLog
src/language/stats/chisquare.c
src/language/stats/oneway.q
src/language/stats/rank.q
src/ui/ChangeLog
src/ui/flexifile.c

index fab0145ef228e9d3b9a7599b20def32916878219..ac93645850b57da50880d0189961555e046994a7 100644 (file)
@@ -1,3 +1,59 @@
+Mon Jan 15 16:18:10 2007  Ben Pfaff  <blp@gnu.org>
+
+       * case.c (case_is_null): Change return type to bool.
+
+Mon Jan 15 10:57:28 2007  Ben Pfaff  <blp@gnu.org>
+
+       Add debugging code.
+       
+       * case.c (case_clone) [DEBUGGING]: When debugging, don't use
+       reference counting to share data.  This makes it easy for
+       valgrind, etc. to find accesses to cases that have been destroyed
+       but have been kept around by another user's ref-count.  This often
+       happens when the data set is small enough to find in memory; if a
+       bigger data set that would overflow to disk were used, then data
+       corruption would occur.
+
+Mon Jan 15 10:55:18 2007  Ben Pfaff  <blp@gnu.org>
+
+       Simplify code.
+
+       * case.c (case_unshare): Make it check internally whether the
+       ref_cnt is greater than 1, so that the callers don't have to.
+       Update callers not to check.
+
+Mon Jan 15 10:53:01 2007  Ben Pfaff  <blp@gnu.org>
+
+       Before, I was thinking that I might want to get rid of reference
+       counting at some point.  Now, I'm pretty sure that it's here to
+       stay.  Thus, because we have to store the value_cnt anyway for
+       reference-counted cases, we might as well expose it to users.
+
+       * case.c (case_get_value_cnt): New function.
+       (case_resize): Drop OLD_CNT argument.  Update all callers.  Only
+       resize case if its size actually changed.
+
+       * casefile.c (casefile_append_xfer): Use case_get_value_cnt
+       instead of peeking inside struct case directly.
+       (casefile_append): Ditto.
+
+Mon Jan 15 10:50:22 2007  Ben Pfaff  <blp@gnu.org>
+
+       Get rid of the inlines for the case functions, which made the
+       header file hard to read.  (Also, in testing with "-O2 -DNDEBUG",
+       the inlines didn't speed up "make check" at all, which is not a
+       perfect benchmark but seems indicative.)
+       
+       * case.c: Remove #ifdef DEBUGGING...#endif around many function
+       definitions.  Remove some assertions on nonnull pointers that were
+       redundant with a pointer dereference soon after in the function.
+       Also:
+       (struct case_data): Move definition here from case.h.
+       (case_data): Ditto.
+       (case_num): Ditto.
+       (case_str): Ditto.
+       (case_data_wr): Ditto.
+       
 Sun Jan 14 21:41:12 2007  Ben Pfaff  <blp@gnu.org>
 
        * automake.mk: Add casedeque.h to sources.
index 8c92bbe31d8d92958c5e2e19594fbb76687655cb..c08557156357779327fd1559b4dde4dcd88467a6 100644 (file)
@@ -1,5 +1,5 @@
 /* PSPP - computes sample statistics.
-   Copyright (C) 2004 Free Software Foundation, Inc.
+   Copyright (C) 2004, 2007 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
    02110-1301, USA. */
 
 #include <config.h>
-#include "case.h"
+
+#include <data/case.h>
+
+#include <assert.h>
 #include <limits.h>
 #include <stdlib.h>
-#include "value.h"
+
+#include <data/value.h>
+#include <data/variable.h>
 #include <libpspp/alloc.h>
 #include <libpspp/str.h>
-#include "variable.h"
 
-#ifdef DEBUGGING
-#undef NDEBUG
-#else
-#ifndef NDEBUG
-#define NDEBUG
-#endif
-#endif
-#include <assert.h>
+#include "minmax.h"
 
-/* Changes C not to share data with any other case.
-   C must be a case with a reference count greater than 1.
-   There should be no reason for external code to call this
-   function explicitly.  It will be called automatically when
-   needed. */
-void
+/* Reference-counted case implementation. */
+struct case_data
+  {
+    size_t value_cnt;                   /* Number of values. */
+    unsigned ref_cnt;                   /* Reference count. */
+    union value values[1];              /* Values. */
+  };
+
+/* Ensures that C does not share data with any other case. */
+static void
 case_unshare (struct ccase *c) 
 {
-  struct case_data *cd;
-  
-  assert (c->case_data->ref_cnt > 1);
-
-  cd = c->case_data;
-  cd->ref_cnt--;
-  case_create (c, c->case_data->value_cnt);
-  memcpy (c->case_data->values, cd->values,
-          sizeof *cd->values * cd->value_cnt); 
+  if (c->case_data->ref_cnt > 1)
+    {
+      struct case_data *cd = c->case_data;
+      cd->ref_cnt--;
+      case_create (c, cd->value_cnt);
+      memcpy (c->case_data->values, cd->values,
+              sizeof *cd->values * cd->value_cnt); 
+    }
 }
 
 /* Returns the number of bytes needed by a case with VALUE_CNT
    values. */
-static inline size_t
+static size_t
 case_size (size_t value_cnt) 
 {
   return (offsetof (struct case_data, values)
           + value_cnt * sizeof (union value));
 }
 
-#ifdef DEBUGGING
 /* Initializes C as a null case. */
 void
 case_nullify (struct ccase *c) 
 {
   c->case_data = NULL;
 }
-#endif /* DEBUGGING */
 
-#ifdef DEBUGGING
 /* Returns true iff C is a null case. */
-int
+bool
 case_is_null (const struct ccase *c) 
 {
   return c->case_data == NULL;
 }
-#endif /* DEBUGGING */
 
 /* Initializes C as a new case that can store VALUE_CNT values.
    The values have indeterminate contents until explicitly
@@ -90,7 +86,6 @@ case_create (struct ccase *c, size_t value_cnt)
     xalloc_die ();
 }
 
-#ifdef DEBUGGING
 /* Initializes CLONE as a copy of ORIG. */
 void
 case_clone (struct ccase *clone, const struct ccase *orig)
@@ -100,10 +95,11 @@ case_clone (struct ccase *clone, const struct ccase *orig)
   if (clone != orig) 
     *clone = *orig;
   orig->case_data->ref_cnt++;
+#ifdef DEBUGGING
+  case_unshare (clone);
+#endif
 }
-#endif /* DEBUGGING */
 
-#ifdef DEBUGGING
 /* Replaces DST by SRC and nullifies SRC.
    DST and SRC must be initialized cases at entry. */
 void
@@ -117,17 +113,13 @@ case_move (struct ccase *dst, struct ccase *src)
       case_nullify (src); 
     }
 }
-#endif /* DEBUGGING */
 
-#ifdef DEBUGGING
 /* Destroys case C. */
 void
 case_destroy (struct ccase *c) 
 {
   struct case_data *cd;
   
-  assert (c != NULL);
-
   cd = c->case_data;
   if (cd != NULL && --cd->ref_cnt == 0) 
     {
@@ -136,18 +128,28 @@ case_destroy (struct ccase *c)
       free (cd); 
     }
 }
-#endif /* DEBUGGING */
 
-/* Resizes case C from OLD_CNT to NEW_CNT values. */
+/* Returns the number of union values in C. */
+size_t
+case_get_value_cnt (const struct ccase *c) 
+{
+  return c->case_data->value_cnt;
+}
+
+/* Resizes case C to NEW_CNT union values. */
 void
-case_resize (struct ccase *c, size_t old_cnt, size_t new_cnt) 
+case_resize (struct ccase *c, size_t new_cnt) 
 {
-  struct ccase new;
+  size_t old_cnt = case_get_value_cnt (c);
+  if (old_cnt != new_cnt)
+    {
+      struct ccase new;
 
-  case_create (&new, new_cnt);
-  case_copy (&new, 0, c, 0, old_cnt < new_cnt ? old_cnt : new_cnt);
-  case_swap (&new, c);
-  case_destroy (&new);
+      case_create (&new, new_cnt);
+      case_copy (&new, 0, c, 0, MIN (old_cnt, new_cnt));
+      case_swap (&new, c);
+      case_destroy (&new);
+    }
 }
 
 /* Swaps cases A and B. */
@@ -186,7 +188,6 @@ case_try_clone (struct ccase *clone, const struct ccase *orig)
   return true;
 }
 
-#ifdef DEBUGGING
 /* Copies VALUE_CNT values from SRC (starting at SRC_IDX) to DST
    (starting at DST_IDX). */
 void
@@ -202,16 +203,13 @@ case_copy (struct ccase *dst, size_t dst_idx,
 
   if (dst->case_data != src->case_data || dst_idx != src_idx) 
     {
-      if (dst->case_data->ref_cnt > 1)
-        case_unshare (dst);
+      case_unshare (dst);
       memmove (dst->case_data->values + dst_idx,
                src->case_data->values + src_idx,
                sizeof *dst->case_data->values * value_cnt); 
     }
 }
-#endif /* DEBUGGING */
 
-#ifdef DEBUGGING
 /* Copies case C to OUTPUT.
    OUTPUT_SIZE is the number of `union values' in OUTPUT,
    which must match the number of `union values' in C. */
@@ -226,9 +224,7 @@ case_to_values (const struct ccase *c, union value *output,
   memcpy (output, c->case_data->values,
           c->case_data->value_cnt * sizeof *output);
 }
-#endif /* DEBUGGING */
 
-#ifdef DEBUGGING
 /* Copies INPUT into case C.
    INPUT_SIZE is the number of `union values' in INPUT,
    which must match the number of `union values' in C. */
@@ -236,51 +232,78 @@ void
 case_from_values (struct ccase *c, const union value *input,
                   size_t input_size UNUSED) 
 {
-  assert (c != NULL);
-  assert (c->case_data != NULL);
   assert (c->case_data->ref_cnt > 0);
   assert (input_size == c->case_data->value_cnt);
   assert (input != NULL || input_size == 0);
 
-  if (c->case_data->ref_cnt > 1)
-    case_unshare (c);
+  case_unshare (c);
   memcpy (c->case_data->values, input,
           c->case_data->value_cnt * sizeof *input);
 }
-#endif /* DEBUGGING */
 
-#ifdef DEBUGGING
+/* Returns a pointer to the `union value' used for the
+   element of C for variable V.
+   Case C must be drawn from V's dictionary.
+   The caller must not modify the returned data. */
+const union value *
+case_data (const struct ccase *c, const struct variable *v)
+{
+  return case_data_idx (c, var_get_case_index (v));
+}
+
+/* Returns the numeric value of the `union value' in C for
+   variable V.
+   Case C must be drawn from V's dictionary. */
+double
+case_num (const struct ccase *c, const struct variable *v) 
+{
+  return case_num_idx (c, var_get_case_index (v));
+}
+
+/* Returns the string value of the `union value' in C for
+   variable V.
+   Case C must be drawn from V's dictionary.
+   (Note that the value is not null-terminated.)
+   The caller must not modify the return value. */
+const char *
+case_str (const struct ccase *c, const struct variable *v) 
+{
+  return case_str_idx (c, var_get_case_index (v));
+}
+
+/* Returns a pointer to the `union value' used for the
+   element of C for variable V.
+   Case C must be drawn from V's dictionary.   
+   The caller is allowed to modify the returned data. */
+union value *
+case_data_rw (struct ccase *c, const struct variable *v) 
+{
+  return case_data_rw_idx (c, var_get_case_index (v));
+}
+
 /* Returns a pointer to the `union value' used for the
    element of C numbered IDX.
    The caller must not modify the returned data. */
 const union value *
 case_data_idx (const struct ccase *c, size_t idx) 
 {
-  assert (c != NULL);
-  assert (c->case_data != NULL);
   assert (c->case_data->ref_cnt > 0);
   assert (idx < c->case_data->value_cnt);
 
   return &c->case_data->values[idx];
 }
-#endif /* DEBUGGING */
 
-#ifdef DEBUGGING
 /* Returns the numeric value of the `union value' in C numbered
    IDX. */
 double
 case_num_idx (const struct ccase *c, size_t idx) 
 {
-  assert (c != NULL);
-  assert (c->case_data != NULL);
   assert (c->case_data->ref_cnt > 0);
   assert (idx < c->case_data->value_cnt);
 
   return c->case_data->values[idx].f;
 }
-#endif /* DEBUGGING */
 
-#ifdef DEBUGGING
 /* Returns the string value of the `union value' in C numbered
    IDX.
    (Note that the value is not null-terminated.)
@@ -288,32 +311,24 @@ case_num_idx (const struct ccase *c, size_t idx)
 const char *
 case_str_idx (const struct ccase *c, size_t idx) 
 {
-  assert (c != NULL);
-  assert (c->case_data != NULL);
   assert (c->case_data->ref_cnt > 0);
   assert (idx < c->case_data->value_cnt);
 
   return c->case_data->values[idx].s;
 }
-#endif /* DEBUGGING */
 
-#ifdef DEBUGGING
 /* Returns a pointer to the `union value' used for the
    element of C numbered IDX.
    The caller is allowed to modify the returned data. */
 union value *
 case_data_rw_idx (struct ccase *c, size_t idx) 
 {
-  assert (c != NULL);
-  assert (c->case_data != NULL);
   assert (c->case_data->ref_cnt > 0);
   assert (idx < c->case_data->value_cnt);
 
-  if (c->case_data->ref_cnt > 1)
-    case_unshare (c);
+  case_unshare (c);
   return &c->case_data->values[idx];
 }
-#endif /* DEBUGGING */
 
 /* Compares the values of the VAR_CNT variables in VP
    in cases A and B and returns a strcmp()-type result. */
@@ -368,8 +383,6 @@ case_compare_2dict (const struct ccase *ca, const struct ccase *cb,
 const union value *
 case_data_all (const struct ccase *c) 
 {
-  assert (c != NULL);
-  assert (c->case_data != NULL);
   assert (c->case_data->ref_cnt > 0);
 
   return c->case_data->values;
@@ -383,11 +396,8 @@ case_data_all (const struct ccase *c)
 union value *
 case_data_all_rw (struct ccase *c) 
 {
-  assert (c != NULL);
-  assert (c->case_data != NULL);
   assert (c->case_data->ref_cnt > 0);
 
-  if (c->case_data->ref_cnt > 1)
-    case_unshare (c);
+  case_unshare (c);
   return c->case_data->values;
 }
index ff2ecfb7e07e0f726c3680a1460bc4d34b2ddda8..3d9181192c0aa49b3698958a6f91b978ce80eb96 100644 (file)
@@ -1,5 +1,5 @@
 /* PSPP - computes sample statistics.
-   Copyright (C) 2004 Free Software Foundation, Inc.
+   Copyright (C) 2004, 2007 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
 #ifndef HEADER_CASE
 #define HEADER_CASE
 
+#include <limits.h>
 #include <stddef.h>
 #include <stdbool.h>
 #include "value.h"
-#include "variable.h"
+
+struct variable;
 
 /* Opaque structure that represents a case.  Use accessor
    functions instead of accessing any members directly.  Use
@@ -32,57 +34,40 @@ struct ccase
     struct case_data *case_data;        /* Actual data. */
   };
 
-/* Invisible to user code. */
-struct case_data
-  {
-    size_t value_cnt;                   /* Number of values. */
-    unsigned ref_cnt;                   /* Reference count. */
-    union value values[1];              /* Values. */
-  };
-
-#ifdef DEBUGGING
-#define CASE_INLINE
-#else
-#define CASE_INLINE static
-#endif
-
-CASE_INLINE void case_nullify (struct ccase *);
-CASE_INLINE int case_is_null (const struct ccase *);
+void case_nullify (struct ccase *);
+bool case_is_null (const struct ccase *);
 
 void case_create (struct ccase *, size_t value_cnt);
-CASE_INLINE void case_clone (struct ccase *, const struct ccase *);
-CASE_INLINE void case_move (struct ccase *, struct ccase *);
-CASE_INLINE void case_destroy (struct ccase *);
+void case_clone (struct ccase *, const struct ccase *);
+void case_move (struct ccase *, struct ccase *);
+void case_destroy (struct ccase *);
+
+size_t case_get_value_cnt (const struct ccase *);
 
-void case_resize (struct ccase *, size_t old_cnt, size_t new_cnt);
+void case_resize (struct ccase *, size_t new_value_cnt);
 void case_swap (struct ccase *, struct ccase *);
 
 bool case_try_create (struct ccase *, size_t value_cnt);
 bool case_try_clone (struct ccase *, const struct ccase *);
 
-CASE_INLINE void case_copy (struct ccase *dst, size_t dst_idx,
+void case_copy (struct ccase *dst, size_t dst_idx,
                             const struct ccase *src, size_t src_idx,
                             size_t cnt);
 
-CASE_INLINE void case_to_values (const struct ccase *, union value *, size_t);
-CASE_INLINE void case_from_values (struct ccase *,
+void case_to_values (const struct ccase *, union value *, size_t);
+void case_from_values (struct ccase *,
                                    const union value *, size_t);
 
-static inline const union value *case_data (const struct ccase *,
-                                            const struct variable *);
-static inline double case_num (const struct ccase *, const struct variable *);
-static inline const char *case_str (const struct ccase *,
-                                    const struct variable *);
-static inline union value *case_data_rw (struct ccase *,
-                                         const struct variable *);
+const union value *case_data (const struct ccase *, const struct variable *);
+double case_num (const struct ccase *, const struct variable *);
+const char *case_str (const struct ccase *, const struct variable *);
+union value *case_data_rw (struct ccase *, const struct variable *);
 
-CASE_INLINE const union value *case_data_idx (const struct ccase *,
-                                              size_t idx);
-CASE_INLINE double case_num_idx (const struct ccase *, size_t idx);
-CASE_INLINE const char *case_str_idx (const struct ccase *, size_t idx);
-CASE_INLINE union value *case_data_rw_idx (struct ccase *, size_t idx);
+const union value *case_data_idx (const struct ccase *, size_t idx);
+double case_num_idx (const struct ccase *, size_t idx);
+const char *case_str_idx (const struct ccase *, size_t idx);
+union value *case_data_rw_idx (struct ccase *, size_t idx);
 
-struct variable;
 int case_compare (const struct ccase *, const struct ccase *,
                   struct variable *const *, size_t var_cnt);
 int case_compare_2dict (const struct ccase *, const struct ccase *,
@@ -92,147 +77,4 @@ int case_compare_2dict (const struct ccase *, const struct ccase *,
 const union value *case_data_all (const struct ccase *);
 union value *case_data_all_rw (struct ccase *);
 
-void case_unshare (struct ccase *);
-
-#ifndef DEBUGGING
-#include <stdlib.h>
-#include <libpspp/str.h>
-
-static inline void
-case_nullify (struct ccase *c) 
-{
-  c->case_data = NULL;
-}
-
-static inline int
-case_is_null (const struct ccase *c) 
-{
-  return c->case_data == NULL;
-}
-
-static inline void
-case_clone (struct ccase *clone, const struct ccase *orig)
-{
-  *clone = *orig;
-  orig->case_data->ref_cnt++;
-}
-
-static inline void
-case_move (struct ccase *dst, struct ccase *src) 
-{
-  if (dst != src) 
-    {
-      *dst = *src;
-      src->case_data = NULL; 
-    }
-}
-
-static inline void
-case_destroy (struct ccase *c) 
-{
-  struct case_data *cd = c->case_data;
-  if (cd != NULL && --cd->ref_cnt == 0)
-    free (cd);
-}
-
-static inline void
-case_copy (struct ccase *dst, size_t dst_idx,
-           const struct ccase *src, size_t src_idx,
-           size_t value_cnt) 
-{
-  if (dst->case_data != src->case_data || dst_idx != src_idx) 
-    {
-      if (dst->case_data->ref_cnt > 1)
-        case_unshare (dst);
-      memmove (dst->case_data->values + dst_idx,
-               src->case_data->values + src_idx,
-               sizeof *dst->case_data->values * value_cnt); 
-    }
-}
-
-static inline void
-case_to_values (const struct ccase *c, union value *output,
-                size_t output_size ) 
-{
-  memcpy (output, c->case_data->values,
-          output_size * sizeof *output);
-}
-
-static inline void
-case_from_values (struct ccase *c, const union value *input,
-                  size_t input_size UNUSED) 
-{
-  if (c->case_data->ref_cnt > 1)
-    case_unshare (c);
-  memcpy (c->case_data->values, input,
-          c->case_data->value_cnt * sizeof *input);
-}
-
-static inline const union value *
-case_data_idx (const struct ccase *c, size_t idx) 
-{
-  return &c->case_data->values[idx];
-}
-
-static inline double
-case_num_idx (const struct ccase *c, size_t idx) 
-{
-  return c->case_data->values[idx].f;
-}
-
-static inline const char *
-case_str_idx (const struct ccase *c, size_t idx)
-{
-  return c->case_data->values[idx].s;
-}
-
-static inline union value *
-case_data_rw_idx (struct ccase *c, size_t idx)
-{
-  if (c->case_data->ref_cnt > 1)
-    case_unshare (c);
-  return &c->case_data->values[idx];
-}
-#endif /* !DEBUGGING */
-
-/* Returns a pointer to the `union value' used for the
-   element of C for variable V.
-   Case C must be drawn from V's dictionary.
-   The caller must not modify the returned data. */
-static inline const union value *
-case_data (const struct ccase *c, const struct variable *v)
-{
-  return case_data_idx (c, var_get_case_index (v));
-}
-
-/* Returns the numeric value of the `union value' in C for
-   variable V.
-   Case C must be drawn from V's dictionary. */
-static inline double
-case_num (const struct ccase *c, const struct variable *v) 
-{
-  return case_num_idx (c, var_get_case_index (v));
-}
-
-/* Returns the string value of the `union value' in C for
-   variable V.
-   Case C must be drawn from V's dictionary.
-   (Note that the value is not null-terminated.)
-   The caller must not modify the return value. */
-static inline const char *
-case_str (const struct ccase *c, const struct variable *v) 
-{
-  return case_str_idx (c, var_get_case_index (v));
-}
-
-/* Returns a pointer to the `union value' used for the
-   element of C for variable V.
-   Case C must be drawn from V's dictionary.   
-   The caller is allowed to modify the returned data. */
-static inline union value *
-case_data_rw (struct ccase *c, const struct variable *v) 
-{
-  return case_data_rw_idx (c, var_get_case_index (v));
-}
-
 #endif /* case.h */
index fde8228c56ef30f3c39f29f292675bc1099ec6b1..d81ec29efa0a2c9d331ea08114e6b4be8986aaa4 100644 (file)
@@ -1,5 +1,5 @@
 /* PSPP - computes sample statistics.
-   Copyright (C) 2004, 2006 Free Software Foundation, Inc.
+   Copyright (C) 2004, 2006, 2007 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
@@ -274,7 +274,7 @@ casefile_get_destructive_reader (struct casefile *cf)
 bool 
 casefile_append (struct casefile *cf, const struct ccase *c)
 {
-  assert (c->case_data->value_cnt >= casefile_get_value_cnt (cf));
+  assert (case_get_value_cnt (c) >= casefile_get_value_cnt (cf));
 
   return cf->class->append(cf, c);
 }
@@ -285,7 +285,7 @@ casefile_append (struct casefile *cf, const struct ccase *c)
 bool 
 casefile_append_xfer (struct casefile *cf, struct ccase *c)
 {
-  assert (c->case_data->value_cnt >= casefile_get_value_cnt (cf));
+  assert (case_get_value_cnt (c) >= casefile_get_value_cnt (cf));
 
   cf->class->append (cf, c);
   case_destroy (c);
index 3b5cfdba5f4161a1b4a60b569e255fd59a8498f5..7fa6eb85916e1befcbdb84c81c1c13abbcc20c15 100644 (file)
@@ -1,5 +1,5 @@
 /* PSPP - computes sample statistics.
-   Copyright (C) 2004, 2006 Free Software Foundation, Inc.
+   Copyright (C) 2004, 2006, 2007 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
    02110-1301, USA. */
 
 #include <config.h>
+
 #include "casefile.h"
 #include "casefile-private.h"
 #include "fastfile.h"
+
 #include <assert.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+
+#include <data/case.h>
+#include <data/make-file.h>
+#include <data/settings.h>
+#include <data/variable.h>
 #include <libpspp/alloc.h>
-#include "case.h"
 #include <libpspp/compiler.h>
 #include <libpspp/message.h>
+#include <libpspp/misc.h>
+#include <libpspp/str.h>
+
 #include "full-read.h"
 #include "full-write.h"
-#include <libpspp/misc.h>
-#include "make-file.h"
-#include "settings.h"
-#include "variable.h"
 
 #include "gettext.h"
 #define _(msgid) gettext (msgid)
index 2498b0945528c26bff216e5b2a6c50c91981e715..9318f94b56c5b2f12500f5625fe755d6c34b71f3 100644 (file)
@@ -1,5 +1,5 @@
 /* PSPP - computes sample statistics.
-   Copyright (C) 1997-9, 2000, 2006 Free Software Foundation, Inc.
+   Copyright (C) 1997-9, 2000, 2006, 2007 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
@@ -176,7 +176,7 @@ cmd_debug_evaluate (struct lexer *lexer, struct dataset *dsother UNUSED)
               case_create (c, dict_get_next_value_idx (d));
             }
           else
-            case_resize (c, old_value_cnt, dict_get_next_value_idx (d));
+            case_resize (c, dict_get_next_value_idx (d));
 
           if (lex_is_number (lexer))
             case_data_rw (c, v)->f = lex_tokval (lexer);
index cf4df7932d538e29489528a8e2cbd98da5c6b7d2..712c8428332688cbdcd47ed64f90b8c708f8ed02 100644 (file)
@@ -1,3 +1,20 @@
+Mon Jan 15 11:03:20 2007  Ben Pfaff  <blp@gnu.org>
+
+       Fix bugs found by valgrind when --enable-debug is used with the
+       new case code.  These bugs are hidden when the data set is small
+       enough to find in memory; if a bigger data set that would overflow
+       to disk were used, then data corruption would occur.
+
+       * chisquare.c (create_freq_hash): Pass free_freq_mutable_hash to
+       hsh_create as free function.  Make copy of data put into hash.
+
+       * oneway.q (free_value): New function.
+       (run_oneway): Use free_value as arg to hsh_create.  Make copy of
+       data put into hash.
+
+       * rank.q (rank_cases): Don't access data in case after we've given
+       away the case.
+
 Tue Jan  9 19:16:11 2007  Ben Pfaff  <blp@gnu.org>
 
        Fix bug #18739.
index 05fe41e9719fbb33c69bd02925e967bc01b47990..a406edc7433d8ff9ef9f128d68385e32ef590f12 100644 (file)
@@ -1,5 +1,5 @@
 /* PSPP - computes sample statistics.
-   Copyright (C) 2006 Free Software Foundation, Inc.
+   Copyright (C) 2006, 2007 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
@@ -147,7 +147,7 @@ create_freq_hash (const struct dictionary *dict,
 
   struct hsh_table *freq_hash = 
     hsh_create (4, compare_freq, hash_freq, 
-               free_freq_hash,
+               free_freq_mutable_hash,
                (void *) var);
 
   while (casereader_read(r, &c))
@@ -172,7 +172,8 @@ create_freq_hash (const struct dictionary *dict,
        }
       else
        {
-         *existing_fr = fr;
+          *existing_fr = fr;
+          fr->value = value_dup (fr->value, var_get_width (var));
        }
 
       case_destroy (&c);
index 69ab5a48e15d2bf664af388681d03f589fc9594c..c63dc3a797103d2bf037fa00cfb74b76b1991b30 100644 (file)
@@ -1,6 +1,6 @@
 /* PSPP - One way ANOVA. -*-c-*-
 
-Copyright (C) 1997-9, 2000 Free Software Foundation, Inc.
+Copyright (C) 1997-9, 2000, 2007 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
@@ -879,6 +879,12 @@ precalc ( struct cmd_oneway *cmd UNUSED )
     }
 }
 
+static void
+free_value (void *value_, const void *aux UNUSED) 
+{
+  union value *value = value_;
+  free (value);
+}
 
 static bool
 run_oneway(const struct ccase *first, const struct casefile *cf, 
@@ -895,7 +901,7 @@ run_oneway(const struct ccase *first, const struct casefile *cf,
   global_group_hash = hsh_create(4, 
                                 (hsh_compare_func *) compare_values,
                                 (hsh_hash_func *) hash_value,
-                                0,
+                                free_value,
                                 (void *) var_get_width (indep_var) );
 
   precalc(cmd);
@@ -912,13 +918,17 @@ run_oneway(const struct ccase *first, const struct casefile *cf,
 
       const double weight = 
        dict_get_case_weight (dataset_dict (ds), &c, &bad_weight_warn);
-      
-      const union value *indep_val;
 
+      const union value *indep_val;
+      void **p;
+      
       if ( casefilter_variable_missing (filter, &c, indep_var))
        continue;
 
       indep_val = case_data (&c, indep_var);
+      p = hsh_probe (global_group_hash, indep_val);
+      if (*p == NULL)
+        *p = value_dup (indep_val, var_get_width (indep_var));
          
       hsh_insert ( global_group_hash, (void *) indep_val );
 
index f5be9e9a24405bb4351ea8676f52fa7bee033c22..8efc816efe700af5cf37453c296517996ade3a63 100644 (file)
@@ -510,6 +510,7 @@ rank_cases (struct casereader *cr,
     {
       struct casereader *lookahead;
       const union value *this_value;
+      bool this_value_is_missing;
       struct ccase this_case, lookahead_case;
       double c;
       int i;
@@ -519,6 +520,8 @@ rank_cases (struct casereader *cr,
         break;
 
       this_value = case_data_idx (&this_case, fv);
+      this_value_is_missing = mv_is_value_missing (mv, this_value,
+                                                   exclude_values);
       c = dict_get_case_weight (dict, &this_case, &warn);
 
       lookahead = casereader_clone (cr);
@@ -545,7 +548,7 @@ rank_cases (struct casereader *cr,
       casereader_destroy (lookahead);
 
       cc_1 = cc;
-      if ( !mv_is_value_missing (mv, this_value, exclude_values) )
+      if ( !this_value_is_missing )
        cc += c;
 
       do
@@ -554,7 +557,7 @@ rank_cases (struct casereader *cr,
             {
               const struct variable *dst_var = rs[i].destvars[dest_var_index];
 
-             if  ( mv_is_value_missing (mv, this_value, exclude_values) )
+             if  (this_value_is_missing)
                case_data_rw (&this_case, dst_var)->f = SYSMIS;
              else
                case_data_rw (&this_case, dst_var)->f =
@@ -564,7 +567,7 @@ rank_cases (struct casereader *cr,
         }
       while (n-- > 0 && casereader_read_xfer (cr, &this_case));
 
-      if ( !mv_is_value_missing (mv, this_value, exclude_values) )
+      if ( !this_value_is_missing )
        iter++;
     }
 
index e727ccc92ee4e84f8dde87d7d00f5058fb218248..1eba8ed6dae6f315d3c662072e5f39da5398beec 100644 (file)
@@ -1,3 +1,8 @@
+Mon Jan 15 11:06:31 2007  Ben Pfaff  <blp@gnu.org>
+
+       * flexifile.c [DEBUGGING] (dump_case_data): Use case accessor
+       functions.
+       
 Wed Dec 20 21:14:29 WST 2006 John Darrington <john@darrington.wattle.id.au>
 
        * flexifile.c (flexifilereader_cnum) : new function
index dacb93dc442567b34ec4bf1fbf7f4b98b6ae1913..339764e09520bc1f76f5cb91f256d05ce6279241 100644 (file)
@@ -1,6 +1,6 @@
 /* PSPP - computes sample statistics.
 
-   Copyright (C) 2006 Free Software Foundation, Inc.
+   Copyright (C) 2006, 2007 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
@@ -315,12 +315,15 @@ impl_get_case(const struct flexifile *ff, unsigned long casenum,
 }
 
 #if DEBUGGING
-static void
+#include <stdio.h>
+
+static void 
 dumpcasedata(struct ccase *c)
 {
+  size_t value_cnt = case_get_value_cnt (c);
   int i;
-  for ( i = 0 ; i < c->case_data->value_cnt * MAX_SHORT_STRING; ++i )
-    putchar(c->case_data->values->s[i]);
+  for ( i = 0 ; i < value_cnt * MAX_SHORT_STRING; ++i )
+    putchar (case_str (c, 0)[i]);
   putchar('\n');
 }
 #endif