From: John Darrington Date: Sun, 27 Mar 2016 11:51:53 +0000 (+0200) Subject: Fixed bug in EXAMINE where the parametric calculation would crash if the weight X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=eba86bfdc3b849058a19fffa82da05249ec9a90a;p=pspp Fixed bug in EXAMINE where the parametric calculation would crash if the weight variable contained a missing value. Found by zzuf. --- diff --git a/src/data/dictionary.c b/src/data/dictionary.c index dc74851792..e50397a156 100644 --- a/src/data/dictionary.c +++ b/src/data/dictionary.c @@ -1007,15 +1007,8 @@ dict_get_case_weight (const struct dictionary *d, const struct ccase *c, else { double w = case_num (c, d->weight); - if (w < 0.0 || var_is_num_missing (d->weight, w, MV_ANY)) - w = 0.0; - if ( w == 0.0 && warn_on_invalid != NULL && *warn_on_invalid ) { - *warn_on_invalid = false; - msg (SW, _("At least one case in the data file had a weight value " - "that was user-missing, system-missing, zero, or " - "negative. These case(s) were ignored.")); - } - return w; + + return var_force_valid_weight (d->weight, w, warn_on_invalid); } } diff --git a/src/data/variable.c b/src/data/variable.c index 971154368c..6a00d014a1 100644 --- a/src/data/variable.c +++ b/src/data/variable.c @@ -1312,3 +1312,29 @@ var_clear_vardict (struct variable *v) { v->vardict = NULL; } + + +/* + Returns zero, if W is a missing value for WV or if it is less than zero. + Typically used to force a numerical value into a valid weight. + + As a side effect, this function will emit a warning if the value + WARN_ON_INVALID points to a bool which is TRUE. That bool will be then + set to FALSE. + */ +double +var_force_valid_weight (const struct variable *wv, double w, bool *warn_on_invalid) +{ + if (w < 0.0 || (wv && var_is_num_missing (wv, w, MV_ANY))) + w = 0.0; + + if (w == 0.0 && warn_on_invalid != NULL && *warn_on_invalid) + { + *warn_on_invalid = false; + msg (SW, _("At least one case in the data file had a weight value " + "that was user-missing, system-missing, zero, or " + "negative. These case(s) were ignored.")); + } + + return w; +} diff --git a/src/data/variable.h b/src/data/variable.h index e960e09234..1fefe1ee8d 100644 --- a/src/data/variable.h +++ b/src/data/variable.h @@ -206,4 +206,7 @@ const char *var_get_encoding (const struct variable *); /* Function types. */ typedef bool var_predicate_func (const struct variable *); +double var_force_valid_weight (const struct variable *wv, double w, + bool *warn_on_invalid); + #endif /* data/variable.h */ diff --git a/src/language/stats/examine.c b/src/language/stats/examine.c index 68b6411626..cfe380d5b5 100644 --- a/src/language/stats/examine.c +++ b/src/language/stats/examine.c @@ -1533,7 +1533,7 @@ update_n (const void *aux1, void *aux2 UNUSED, void *user_data, int v; const struct examine *examine = aux1; struct exploratory_stats *es = user_data; - + bool this_case_is_missing = false; /* LISTWISE missing must be dealt with here */ if (!examine->missing_pw) @@ -1632,13 +1632,15 @@ calculate_n (const void *aux1, void *aux2 UNUSED, void *user_data) value_init_pool (examine->pool, &es[v].maxima[i].identity, examine->id_width) ; value_init_pool (examine->pool, &es[v].minima[i].identity, examine->id_width) ; } - + + bool warn = true; for (reader = casereader_clone (es[v].sorted_reader); (c = casereader_read (reader)) != NULL; case_unref (c)) { const double val = case_data_idx (c, EX_VAL)->f; - const double wt = case_data_idx (c, EX_WT)->f; - + double wt = case_data_idx (c, EX_WT)->f; + wt = var_force_valid_weight (examine->wv, wt, &warn); + moments_pass_two (es[v].mom, val, wt); if (es[v].histogram) diff --git a/src/math/categoricals.c b/src/math/categoricals.c index 140542fe02..8bd6683e2c 100644 --- a/src/math/categoricals.c +++ b/src/math/categoricals.c @@ -413,6 +413,7 @@ categoricals_update (struct categoricals *cat, const struct ccase *c) return; weight = cat->wv ? case_data (c, cat->wv)->f : 1.0; + weight = var_force_valid_weight (cat->wv, weight, NULL); assert (NULL == cat->reverse_variable_map_short); assert (NULL == cat->reverse_variable_map_long); @@ -471,10 +472,8 @@ categoricals_update (struct categoricals *cat, const struct ccase *c) if (cat->payload) { - double weight = cat->wv ? case_data (c, cat->wv)->f : 1.0; cat->payload->update (cat->aux1, cat->aux2, node->user_data, c, weight); } - } } diff --git a/tests/language/stats/examine.at b/tests/language/stats/examine.at index a55facf7c2..95889c50b4 100644 --- a/tests/language/stats/examine.at +++ b/tests/language/stats/examine.at @@ -1073,3 +1073,45 @@ AT_CLEANUP +dnl Test for yet another crash. This time for extremes vs. missing weight values. +AT_SETUP([EXAMINE -- Extremes vs. Missing Weights]) + +AT_DATA([examine-missing-weights.sps], [dnl +data list notable list /h * g *. +begin data. +3 1 +4 . +5 1 +2 1 +end data. + +WEIGHT BY g. + +EXAMINE h + /STATISTICS extreme(3) + . +]) + +AT_CHECK([pspp -O format=csv examine-missing-weights.sps], [0], [dnl +"examine-missing-weights.sps:13: warning: EXAMINE: At least one case in the data file had a weight value that was user-missing, system-missing, zero, or negative. These case(s) were ignored." + +Table: Case Processing Summary +,Cases,,,,, +,Valid,,Missing,,Total, +,N,Percent,N,Percent,N,Percent +h,3.00,100%,.00,0%,3.00,100% + +Table: Extreme Values +,,,Case Number,Value +h,Highest,1,3,5.00 +,,2,2,4.00 +,,3,1,3.00 +,Lowest,1,4,2.00 +,,2,1,3.00 +,,3,2,4.00 +]) + +AT_CLEANUP + + +