[Pspp-cvs] pspp src/data/procedure.c src/data/ChangeLog ./...

[Ben Pfaff]

CVSROOT:        /cvsroot/pspp
Module name:    pspp
Branch:         
Changes by:     Ben Pfaff <address@hidden>      06/05/06 05:56:59

Modified files:
        src/data       : procedure.c ChangeLog 
        .              : TODO 

Log message:
        Continue reforming procedure execution.  Change internal_procedure()
        so that it calls open_active_file() and close_active_file(), which
        isolates most of the actual procedure functionality.

CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/pspp/pspp/src/data/procedure.c.diff?tr1=1.2&tr2=1.3&r1=text&r2=text
http://cvs.savannah.gnu.org/viewcvs/pspp/pspp/src/data/ChangeLog.diff?tr1=1.31&tr2=1.32&r1=text&r2=text
http://cvs.savannah.gnu.org/viewcvs/pspp/pspp/TODO.diff?tr1=1.38&tr2=1.39&r1=text&r2=text

Patches:
Index: pspp/TODO
diff -u pspp/TODO:1.38 pspp/TODO:1.39
--- pspp/TODO:1.38      Fri May  5 04:46:53 2006
+++ pspp/TODO   Sat May  6 05:56:59 2006
@@ -1,13 +1,9 @@
-Time-stamp: <2006-05-04 21:46:50 blp>
+Time-stamp: <2006-05-05 22:56:48 blp>
 
 Procedure processing:
 
 * Should not need temporary casefile in the common case.
 
-* All of the procedure_*() variants can (and should) be implemented in terms of
-  a variant that provides "proc_func" plus an "end_func" called after all
-  processing is complete.
-
 * The "split" variants should not dump the splits to the output file
   automatically.  There is no need for the procedure code to talk to the output
   manager.
Index: pspp/src/data/ChangeLog
diff -u pspp/src/data/ChangeLog:1.31 pspp/src/data/ChangeLog:1.32
--- pspp/src/data/ChangeLog:1.31        Sat May  6 04:39:20 2006
+++ pspp/src/data/ChangeLog     Sat May  6 05:56:59 2006
@@ -1,6 +1,34 @@
+Fri May  5 22:48:50 2006  Ben Pfaff  <address@hidden>
+
+       Continue reforming procedure execution.  Change
+       internal_procedure() so that it calls open_active_file() and
+       close_active_file(), which isolates most of the actual procedure
+       functionality.
+
+       * procedure.c: (struct write_case_data) Rename `proc_func' member
+       to `case_func' and update all references.
+       (procedure) Rewrite as one-line wrapper around
+       internal_procedure().
+       (struct multipass_aux_data) New.
+       (multipass_callback) Renamed multipass_case_func().  Use struct
+       multipass_aux_data as auxiliary data.
+       (multipass_end_func) New function.
+       (multipass_procedure) Rewrite as wrapper for internal_procedure()
+       that uses multipass_case_func, multipass_end_func.
+       (internal_procedure) Add `end_func' argument.  Move optimization
+       of trivial case in here.  Move call to open_active_file() and
+       close_active_file() in here.  Now assert that vfm_source is
+       non-null.
+       (procedure_with_splits_callback) Rename
+       split_procedure_case_func().
+       (split_procedure_end_func) New function.
+       (multipass_split_callback) Rename multipass_split_case_func.
+       (multipass_split_end_func) New function.
+       (discard_variables) No need to test for nonnull vfm_source.
+
 Fri May  5 21:34:02 2006  Ben Pfaff  <address@hidden>
 
-       Get rid of unused member.
+       Continue reforming procedure execution.  Get rid of unused member.
 
        * procedure.c: (struct write_case_data) Remove `cases_analyzed'
        member.
Index: pspp/src/data/procedure.c
diff -u pspp/src/data/procedure.c:1.2 pspp/src/data/procedure.c:1.3
--- pspp/src/data/procedure.c:1.2       Sat May  6 04:39:20 2006
+++ pspp/src/data/procedure.c   Sat May  6 05:56:59 2006
@@ -60,7 +60,7 @@
 struct write_case_data
   {
     /* Function to call for each case. */
-    bool (*proc_func) (struct ccase *, void *); /* Function. */
+    bool (*case_func) (struct ccase *, void *); /* Function. */
     void *aux;                                 /* Auxiliary data. */ 
 
     struct ccase trns_case;     /* Case used for transformations. */
@@ -104,7 +104,8 @@
 static void add_filter_trns (void);
 static void add_process_if_trns (void);
 
-static bool internal_procedure (bool (*proc_func) (struct ccase *, void *),
+static bool internal_procedure (bool (*case_func) (struct ccase *, void *),
+                                bool (*end_func) (void *),
                                 void *aux);
 static void update_last_vfm_invocation (void);
 static void create_trns_case (struct ccase *, struct dictionary *);
@@ -124,6 +125,8 @@
     update_last_vfm_invocation ();
   return last_vfm_invocation;
 }
+
+/* Regular procedure. */
 
 /* Reads the data from the input program and writes it to a new
    active file.  For each case we read from the input program, we
@@ -143,38 +146,34 @@
 bool
 procedure (bool (*proc_func) (struct ccase *, void *), void *aux)
 {
-  if (proc_func == NULL
-      && case_source_is_class (vfm_source, &storage_source_class)
-      && vfm_sink == NULL
-      && temporary_trns_chain == NULL
-      && trns_chain_is_empty (permanent_trns_chain))
-    {
-      expr_free (process_if_expr);
-      process_if_expr = NULL;
-      dict_set_case_limit (default_dict, 0);
-      dict_clear_vectors (default_dict);
+  return internal_procedure (proc_func, NULL, aux);
+}
+
+/* Multipass procedure. */
 
-      update_last_vfm_invocation ();
-      return true;
-    }
-  else 
-    {
-      bool ok;
-      
-      open_active_file ();
-      ok = internal_procedure (proc_func, aux);
-      ok = close_active_file () && ok;
+struct multipass_aux_data 
+  {
+    struct casefile *casefile;
+    
+    bool (*proc_func) (const struct casefile *, void *aux);
+    void *aux;
+  };
 
-      return ok;
-    }
+/* Case processing function for multipass_procedure(). */
+static bool
+multipass_case_func (struct ccase *c, void *aux_data_) 
+{
+  struct multipass_aux_data *aux_data = aux_data_;
+  return casefile_append (aux_data->casefile, c);
 }
 
-/* Callback function for multipass_procedure(). */
+/* End-of-file function for multipass_procedure(). */
 static bool
-multipass_callback (struct ccase *c, void *cf_) 
+multipass_end_func (void *aux_data_) 
 {
-  struct casefile *cf = cf_;
-  return casefile_append (cf, c);
+  struct multipass_aux_data *aux_data = aux_data_;
+  return (aux_data->proc_func == NULL
+          || aux_data->proc_func (aux_data->casefile, aux_data->aux));
 }
 
 /* Procedure that allows multiple passes over the input data.
@@ -184,67 +183,76 @@
 multipass_procedure (bool (*proc_func) (const struct casefile *, void *aux),
                      void *aux) 
 {
-  if (case_source_is_class (vfm_source, &storage_source_class)
-      && vfm_sink == NULL
-      && temporary_trns_chain == NULL
-      && trns_chain_is_empty (permanent_trns_chain))
-    {
-      proc_func (storage_source_get_casefile (vfm_source), aux);
-
-      expr_free (process_if_expr);
-      process_if_expr = NULL;
-      dict_set_case_limit (default_dict, 0);
-      dict_clear_vectors (default_dict);
-
-      update_last_vfm_invocation ();
-      return true;
-    }
-  else 
-    {
-      struct casefile *cf;
-      bool ok;
-
-      assert (proc_func != NULL);
+  struct multipass_aux_data aux_data;
+  bool ok;
 
-      cf = casefile_create (dict_get_next_value_idx (default_dict));
+  aux_data.casefile = casefile_create (dict_get_next_value_idx (default_dict));
+  aux_data.proc_func = proc_func;
+  aux_data.aux = aux;
 
-      open_active_file ();
-      ok = internal_procedure (multipass_callback, cf);
-      ok = proc_func (cf, aux) && ok;
-      ok = close_active_file () && ok;
+  ok = internal_procedure (multipass_case_func, multipass_end_func, &aux_data);
+  ok = !casefile_error (aux_data.casefile) && ok;
 
-      casefile_destroy (cf);
+  casefile_destroy (aux_data.casefile);
 
-      return ok;
-    }
+  return ok;
 }
+
+/* Procedure implementation. */
 
-/* Executes a procedure, as procedure(), except that the caller
-   is responsible for calling open_active_file() and
-   close_active_file().
-   Returns true if successful, false if an I/O error occurred. */
+/* Executes a procedure.
+   Passes each case to CASE_FUNC.
+   Calls END_FUNC after the last case.
+   Returns true if successful, false if an I/O error occurred (or
+   if CASE_FUNC or END_FUNC ever returned false). */
 static bool
-internal_procedure (bool (*proc_func) (struct ccase *, void *), void *aux) 
+internal_procedure (bool (*case_func) (struct ccase *, void *),
+                    bool (*end_func) (void *),
+                    void *aux) 
 {
   struct write_case_data wc_data;
-  bool ok;
+  bool ok = true;
+
+  assert (vfm_source != NULL);
+
+  update_last_vfm_invocation ();
 
-  wc_data.proc_func = proc_func;
+  /* Optimize the trivial case where we're not going to do
+     anything with the data, by not reading the data at all. */
+  if (case_func == NULL && end_func == NULL
+      && case_source_is_class (vfm_source, &storage_source_class)
+      && vfm_sink == NULL
+      && (temporary_trns_chain == NULL
+          || trns_chain_is_empty (temporary_trns_chain))
+      && trns_chain_is_empty (permanent_trns_chain))
+    {
+      n_lag = 0;
+      expr_free (process_if_expr);
+      process_if_expr = NULL;
+      dict_set_case_limit (default_dict, 0);
+      dict_clear_vectors (default_dict);
+      return true;
+    }
+  
+  open_active_file ();
+  
+  wc_data.case_func = case_func;
   wc_data.aux = aux;
   create_trns_case (&wc_data.trns_case, default_dict);
   case_create (&wc_data.sink_case, dict_get_next_value_idx (default_dict));
   wc_data.cases_written = 0;
 
-  update_last_vfm_invocation ();
-
-  ok = (vfm_source == NULL
-        || vfm_source->class->read (vfm_source,
-                                    &wc_data.trns_case,
-                                    write_case, &wc_data));
+  ok = vfm_source->class->read (vfm_source,
+                                &wc_data.trns_case,
+                                write_case, &wc_data) && ok;
+  if (end_func != NULL)
+    ok = end_func (aux) && ok;
 
   case_destroy (&wc_data.sink_case);
   case_destroy (&wc_data.trns_case);
 
+  ok = close_active_file () && ok;
+
   return ok;
 }
 
@@ -364,8 +372,8 @@
     }
 
   /* Pass case to procedure. */
-  if (wc_data->proc_func != NULL)
-    if (!wc_data->proc_func (&wc_data->trns_case, wc_data->aux))
+  if (wc_data->case_func != NULL)
+    if (!wc_data->case_func (&wc_data->trns_case, wc_data->aux))
       retval = TRNS_ERROR;
 
  done:
@@ -428,7 +436,8 @@
   if (compactor != NULL) 
     {
       dict_compactor_destroy (compactor);
-      dict_compact_values (default_dict); 
+      dict_compact_values (default_dict);
+      compactor = NULL;
     }
     
   /* Free data source. */
@@ -467,6 +476,8 @@
     return NULL;
 }
 
+/* Procedure that separates the data into SPLIT FILE groups. */
+
 /* Represents auxiliary data for handling SPLIT FILE. */
 struct split_aux_data 
   {
@@ -481,7 +492,8 @@
   };
 
 static int equal_splits (const struct ccase *, const struct ccase *);
-static bool procedure_with_splits_callback (struct ccase *, void *);
+static bool split_procedure_case_func (struct ccase *c, void *split_aux_);
+static bool split_procedure_end_func (void *split_aux_);
 static void dump_splits (struct ccase *);
 
 /* Like procedure(), but it automatically breaks the case stream
@@ -515,21 +527,17 @@
   split_aux.end_func = end_func;
   split_aux.func_aux = func_aux;
 
-  open_active_file ();
-  ok = internal_procedure (procedure_with_splits_callback, &split_aux);
-  if (split_aux.case_count > 0 && end_func != NULL)
-    end_func (func_aux);
-  if (!close_active_file ())
-    ok = false;
+  ok = internal_procedure (split_procedure_case_func,
+                           split_procedure_end_func, &split_aux);
 
   case_destroy (&split_aux.prev_case);
 
   return ok;
 }
 
-/* procedure() callback used by procedure_with_splits(). */
+/* Case callback used by procedure_with_splits(). */
 static bool
-procedure_with_splits_callback (struct ccase *c, void *split_aux_) 
+split_procedure_case_func (struct ccase *c, void *split_aux_) 
 {
   struct split_aux_data *split_aux = split_aux_;
 
@@ -549,10 +557,19 @@
     }
 
   split_aux->case_count++;
-  if (split_aux->proc_func != NULL)
-    return split_aux->proc_func (c, split_aux->func_aux);
-  else
-    return true;
+  return (split_aux->proc_func == NULL
+          || split_aux->proc_func (c, split_aux->func_aux));
+}
+
+/* End-of-file callback used by procedure_with_splits(). */
+static bool
+split_procedure_end_func (void *split_aux_) 
+{
+  struct split_aux_data *split_aux = split_aux_;
+
+  if (split_aux->case_count > 0 && split_aux->end_func != NULL)
+    split_aux->end_func (split_aux->func_aux);
+  return true;
 }
 
 /* Compares the SPLIT FILE variables in cases A and B and returns
@@ -608,6 +625,9 @@
   tab_submit (t);
 }
 
+/* Multipass procedure that separates the data into SPLIT FILE
+   groups. */
+
 /* Represents auxiliary data for handling SPLIT FILE in a
    multipass procedure. */
 struct multipass_split_aux_data 
@@ -620,7 +640,8 @@
     void *func_aux;                            /* Auxiliary data. */ 
   };
 
-static bool multipass_split_callback (struct ccase *c, void *aux_);
+static bool multipass_split_case_func (struct ccase *c, void *aux_);
+static bool multipass_split_end_func (void *aux_);
 static bool multipass_split_output (struct multipass_split_aux_data *);
 
 /* Returns true if successful, false if an I/O error occurred. */
@@ -632,29 +653,21 @@
   struct multipass_split_aux_data aux;
   bool ok;
 
-  assert (split_func != NULL);
-
-  open_active_file ();
-
   case_nullify (&aux.prev_case);
   aux.casefile = NULL;
   aux.split_func = split_func;
   aux.func_aux = func_aux;
 
-  ok = internal_procedure (multipass_split_callback, &aux);
-  if (aux.casefile != NULL)
-    ok = multipass_split_output (&aux) && ok;
+  ok = internal_procedure (multipass_split_case_func,
+                           multipass_split_end_func, &aux);
   case_destroy (&aux.prev_case);
 
-  if (!close_active_file ())
-    ok = false;
-
   return ok;
 }
 
-/* procedure() callback used by multipass_procedure_with_splits(). */
+/* Case callback used by multipass_procedure_with_splits(). */
 static bool
-multipass_split_callback (struct ccase *c, void *aux_)
+multipass_split_case_func (struct ccase *c, void *aux_)
 {
   struct multipass_split_aux_data *aux = aux_;
   bool ok = true;
@@ -678,6 +691,14 @@
   return casefile_append (aux->casefile, c) && ok;
 }
 
+/* End-of-file callback used by multipass_procedure_with_splits(). */
+static bool
+multipass_split_end_func (void *aux_)
+{
+  struct multipass_split_aux_data *aux = aux_;
+  return (aux->casefile == NULL || multipass_split_output (aux));
+}
+
 static bool
 multipass_split_output (struct multipass_split_aux_data *aux)
 {
@@ -690,7 +711,7 @@
 
   return ok;
 }
-
+
 /* Discards all the current state in preparation for a data-input
    command like DATA LIST or GET. */
 void
@@ -701,11 +722,8 @@
 
   n_lag = 0;
   
-  if (vfm_source != NULL)
-    {
-      free_case_source (vfm_source);
-      vfm_source = NULL;
-    }
+  free_case_source (vfm_source);
+  vfm_source = NULL;
 
   proc_cancel_all_transformations ();