From 35c4cb8cfb59bf6e1eb770114850e1184cfafc9b Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Tue, 16 Jan 2007 15:30:28 +0000 Subject: [PATCH] Clean up and improve case code. Patch #5690. --- src/data/ChangeLog | 56 ++++++++ src/data/case.c | 178 ++++++++++++------------ src/data/case.h | 204 ++++------------------------ src/data/casefile.c | 6 +- src/data/fastfile.c | 17 ++- src/language/expressions/evaluate.c | 4 +- src/language/stats/ChangeLog | 17 +++ src/language/stats/chisquare.c | 7 +- src/language/stats/oneway.q | 18 ++- src/language/stats/rank.q | 9 +- src/ui/ChangeLog | 5 + src/ui/flexifile.c | 11 +- 12 files changed, 242 insertions(+), 290 deletions(-) diff --git a/src/data/ChangeLog b/src/data/ChangeLog index fab0145e..ac936458 100644 --- a/src/data/ChangeLog +++ b/src/data/ChangeLog @@ -1,3 +1,59 @@ +Mon Jan 15 16:18:10 2007 Ben Pfaff + + * case.c (case_is_null): Change return type to bool. + +Mon Jan 15 10:57:28 2007 Ben Pfaff + + 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 + + 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 + + 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 + + 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 * automake.mk: Add casedeque.h to sources. diff --git a/src/data/case.c b/src/data/case.c index 8c92bbe3..c0855715 100644 --- a/src/data/case.c +++ b/src/data/case.c @@ -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 @@ -17,68 +17,64 @@ 02110-1301, USA. */ #include -#include "case.h" + +#include + +#include #include #include -#include "value.h" + +#include +#include #include #include -#include "variable.h" -#ifdef DEBUGGING -#undef NDEBUG -#else -#ifndef NDEBUG -#define NDEBUG -#endif -#endif -#include +#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; } diff --git a/src/data/case.h b/src/data/case.h index ff2ecfb7..3d918119 100644 --- a/src/data/case.h +++ b/src/data/case.h @@ -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 @@ -19,10 +19,12 @@ #ifndef HEADER_CASE #define HEADER_CASE +#include #include #include #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 -#include - -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 */ diff --git a/src/data/casefile.c b/src/data/casefile.c index fde8228c..d81ec29e 100644 --- a/src/data/casefile.c +++ b/src/data/casefile.c @@ -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); diff --git a/src/data/fastfile.c b/src/data/fastfile.c index 3b5cfdba..7fa6eb85 100644 --- a/src/data/fastfile.c +++ b/src/data/fastfile.c @@ -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 @@ -17,9 +17,11 @@ 02110-1301, USA. */ #include + #include "casefile.h" #include "casefile-private.h" #include "fastfile.h" + #include #include #include @@ -27,16 +29,19 @@ #include #include #include + +#include +#include +#include +#include #include -#include "case.h" #include #include +#include +#include + #include "full-read.h" #include "full-write.h" -#include -#include "make-file.h" -#include "settings.h" -#include "variable.h" #include "gettext.h" #define _(msgid) gettext (msgid) diff --git a/src/language/expressions/evaluate.c b/src/language/expressions/evaluate.c index 2498b094..9318f94b 100644 --- a/src/language/expressions/evaluate.c +++ b/src/language/expressions/evaluate.c @@ -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); diff --git a/src/language/stats/ChangeLog b/src/language/stats/ChangeLog index cf4df793..712c8428 100644 --- a/src/language/stats/ChangeLog +++ b/src/language/stats/ChangeLog @@ -1,3 +1,20 @@ +Mon Jan 15 11:03:20 2007 Ben Pfaff + + 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 Fix bug #18739. diff --git a/src/language/stats/chisquare.c b/src/language/stats/chisquare.c index 05fe41e9..a406edc7 100644 --- a/src/language/stats/chisquare.c +++ b/src/language/stats/chisquare.c @@ -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); diff --git a/src/language/stats/oneway.q b/src/language/stats/oneway.q index 69ab5a48..c63dc3a7 100644 --- a/src/language/stats/oneway.q +++ b/src/language/stats/oneway.q @@ -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 ); diff --git a/src/language/stats/rank.q b/src/language/stats/rank.q index f5be9e9a..8efc816e 100644 --- a/src/language/stats/rank.q +++ b/src/language/stats/rank.q @@ -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++; } diff --git a/src/ui/ChangeLog b/src/ui/ChangeLog index e727ccc9..1eba8ed6 100644 --- a/src/ui/ChangeLog +++ b/src/ui/ChangeLog @@ -1,3 +1,8 @@ +Mon Jan 15 11:06:31 2007 Ben Pfaff + + * flexifile.c [DEBUGGING] (dump_case_data): Use case accessor + functions. + Wed Dec 20 21:14:29 WST 2006 John Darrington * flexifile.c (flexifilereader_cnum) : new function diff --git a/src/ui/flexifile.c b/src/ui/flexifile.c index dacb93dc..339764e0 100644 --- a/src/ui/flexifile.c +++ b/src/ui/flexifile.c @@ -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 + +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 -- 2.30.2