DELETE VARIABLES: Fix crash with FILTER.

FILTER has the surprising property that it introduces a temporary
transformation, even if proc_make_temporary_transformations_permanent()
was previously called.  This causes a crash if the procedure implementation
really relies on there being no temporary transformations.

This commit fixes the problem in the procedures I was able to identify as
having it, adds a comment to proc_make_temporary_transformations_permanent()
to make it a little less likely to happen again, and adds a test
specifically to check for regression on DELETE VARIABLES.

Reported by John Darrington.
Bug #38843.

diff --git a/src/data/dataset.c b/src/data/dataset.c
index 7448bd38efb7a2ec66807faf62e553a3a8712050..7a5a6a4a6a08a22749baf5340b9e71d709d66c51 100644 (file)
--- a/src/data/dataset.c
+++ b/src/data/dataset.c
@@ -746,9 +746,13 @@ proc_start_temporary_transformations (struct dataset *ds)
     }
 }
 
-/* Converts all the temporary transformations, if any, to
-   permanent transformations.  Further transformations will be
-   permanent.
+/* Converts all the temporary transformations, if any, to permanent
+   transformations.  Further transformations will be permanent.
+
+   The FILTER command is implemented as a temporary transformation, so a
+   procedure that uses this function should usually use proc_open_filtering()
+   with FILTER false, instead of plain proc_open().
+
    Returns true if anything changed, false otherwise. */
 bool
 proc_make_temporary_transformations_permanent (struct dataset *ds)

diff --git a/src/language/data-io/combine-files.c b/src/language/data-io/combine-files.c
index d803ccdc869d40d2fe49cb295e0d50ad23c01533..9d353c81a112488aa9db988f43f65e0c0ddad8c0 100644 (file)
--- a/src/language/data-io/combine-files.c
+++ b/src/language/data-io/combine-files.c
@@ -445,7 +445,7 @@ combine_files (enum comb_command_type command,
           if (active_file == NULL)
             {
               proc_discard_output (ds);
-              file->reader = active_file = proc_open (ds);
+              file->reader = active_file = proc_open_filtering (ds, false);
             }
           else
             file->reader = casereader_clone (active_file);

diff --git a/src/language/dictionary/delete-variables.c b/src/language/dictionary/delete-variables.c
index c9040adc934a898a985c0c1101f6f54322a913c0..ec5d45371551ed5824101337b0e02bd7ef9757da 100644 (file)
--- a/src/language/dictionary/delete-variables.c
+++ b/src/language/dictionary/delete-variables.c
@@ -1,5 +1,5 @@
 /* PSPP - a program for statistical analysis.
-   Copyright (C) 2006, 2007, 2010, 2011 Free Software Foundation, Inc.
+   Copyright (C) 2006, 2007, 2010, 2011, 2013 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
@@ -50,7 +50,7 @@ cmd_delete_variables (struct lexer *lexer, struct dataset *ds)
       goto error;
     }
 
-  ok = casereader_destroy (proc_open (ds));
+  ok = casereader_destroy (proc_open_filtering (ds, false));
   ok = proc_commit (ds) && ok;
   if (!ok)
     goto error;

diff --git a/src/language/stats/descriptives.c b/src/language/stats/descriptives.c
index de05d2b89b6ea1910de096102597af04fdfae996..413826cc9cdaa64c7e9b10d503f80ceae15dc57c 100644 (file)
--- a/src/language/stats/descriptives.c
+++ b/src/language/stats/descriptives.c
@@ -441,7 +441,8 @@ cmd_descriptives (struct lexer *lexer, struct dataset *ds)
       dsc->vars[i].moments = moments_create (dsc->max_moment);
 
   /* Data pass. */
-  grouper = casegrouper_create_splits (proc_open (ds), dict);
+  grouper = casegrouper_create_splits (proc_open_filtering (ds, z_cnt == 0),
+                                       dict);
   while (casegrouper_get_next_group (grouper, &group))
     calc_descriptives (dsc, group, ds);
   ok = casegrouper_destroy (grouper);

diff --git a/src/language/stats/flip.c b/src/language/stats/flip.c
index 6378ffeb90ef6d4ee3a01709a5d58d631153ad02..e1ae9330dd89ac143b399509abcbfc505edb0353 100644 (file)
--- a/src/language/stats/flip.c
+++ b/src/language/stats/flip.c
@@ -166,7 +166,7 @@ cmd_flip (struct lexer *lexer, struct dataset *ds)
   flip->encoding = dict_get_encoding (new_dict);
   dict_clear (new_dict);
 
-  input = proc_open (ds);
+  input = proc_open_filtering (ds, false);
   while ((c = casereader_read (input)) != NULL)
     {
       flip->n_cases++;

diff --git a/src/language/stats/regression.c b/src/language/stats/regression.c
index b63b28e876c8727291c7d0f6ded96848b15dd7d0..1ec00c9f33806f9fc9e6db6605423fa4411928ef 100644 (file)
--- a/src/language/stats/regression.c
+++ b/src/language/stats/regression.c
@@ -270,6 +270,7 @@ cmd_regression (struct lexer *lexer, struct dataset *ds)
   int k;
   struct regression regression;
   const struct dictionary *dict = dataset_dict (ds);
+  bool save;
 
   memset (&regression, 0, sizeof (struct regression));
 
@@ -388,7 +389,8 @@ cmd_regression (struct lexer *lexer, struct dataset *ds)
 
   regression.models = xcalloc (regression.n_dep_vars, sizeof *regression.models);
 
-  if (regression.pred || regression.resid)
+  save = regression.pred || regression.resid;
+  if (save)
     {
       if (proc_make_temporary_transformations_permanent (ds))
         msg (SW, _("REGRESSION with SAVE ignores TEMPORARY.  "
@@ -400,14 +402,15 @@ cmd_regression (struct lexer *lexer, struct dataset *ds)
     struct casereader *group;
     bool ok;
     
-    grouper = casegrouper_create_splits (proc_open (ds), dict);
+    grouper = casegrouper_create_splits (proc_open_filtering (ds, !save),
+                                         dict);
     while (casegrouper_get_next_group (grouper, &group))
       run_regression (&regression, group);
     ok = casegrouper_destroy (grouper);
     ok = proc_commit (ds) && ok;
   }
 
-  if (regression.pred || regression.resid )
+  if (save)
     {
       subcommand_save (&regression);
     }

diff --git a/tests/automake.mk b/tests/automake.mk
index 3ec278c6ff0c55497277ed8894441d8c94d938ef..47951c53923a30d8b9f637d3b38ecfa6a3593e01 100644 (file)
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -266,6 +266,7 @@ TESTSUITE_AT = \
        tests/language/data-io/save-translate.at \
        tests/language/data-io/update.at \
        tests/language/dictionary/attributes.at \
+       tests/language/dictionary/delete-variables.at \
        tests/language/dictionary/formats.at \
        tests/language/dictionary/missing-values.at \
        tests/language/dictionary/mrsets.at \

diff --git a/tests/language/dictionary/delete-variables.at b/tests/language/dictionary/delete-variables.at
new file mode 100644 (file)
index 0000000..48bc40a
--- /dev/null
+++ b/tests/language/dictionary/delete-variables.at
@@ -0,0 +1,29 @@
+AT_BANNER([DELETE VARIABLES])
+
+dnl Checks for regressions against a crash reported in bug #38843.
+AT_SETUP([DELETE VARIABLES with FILTER])
+AT_DATA([delete-variables.sps], [dnl
+DATA LIST LIST /a b.
+BEGIN DATA.
+1 3
+4 6
+7 9
+END DATA.
+
+FILTER BY b.
+DELETE VARIABLES a.
+LIST.
+])
+AT_CHECK([pspp -O format=csv delete-variables.sps], [0], [dnl
+Table: Reading free-form data from INLINE.
+Variable,Format
+a,F8.0
+b,F8.0
+
+Table: Data List
+b
+3.00
+6.00
+9.00
+])
+AT_CLEANUP