From: Ben Pfaff Date: Tue, 2 Mar 2010 06:20:02 +0000 (-0800) Subject: procedure: Fix LIST procedure (and others) in GUI. X-Git-Tag: sav-api~367 X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?p=pspp;a=commitdiff_plain;h=885ed747012fd54269ca2d9f8b55a838435ebb3e procedure: Fix LIST procedure (and others) in GUI. John Darrington reported that the following syntax makes PSPPIRE crash: data list list /alpha * beta * gamma *. begin data. 1 2 3 4 5 6 end data. list. The problem is that the GUI output driver keeps a reference to the output_items that are submitted to it. An output_item that in turn has a reference to the casereader for the procedure casereader (that is, the casereader returned by proc_open()), which currently is LIST and few if any other procedures, then causes an assertion failure in proc_commit(). This is because until now a precondition of proc_commit() has been that the procedure casereader be closed, which is what the assertion enforces. This commit fixes the problem by getting rid of the precondition on proc_commit(). Now the procedure casereader's lifetime may be extended arbitrarily. This should help take PSPP output in the direction that I want it to go, where more and more output is generated directly or semi-directly from casereaders, as the LIST procedure now does. --- diff --git a/src/data/procedure.c b/src/data/procedure.c index 5e013a14af..c79e784e8b 100644 --- a/src/data/procedure.c +++ b/src/data/procedure.c @@ -1,5 +1,5 @@ /* PSPP - a program for statistical analysis. - Copyright (C) 1997-9, 2000, 2006, 2007, 2009 Free Software Foundation, Inc. + Copyright (C) 1997-9, 2000, 2006, 2007, 2009, 2010 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 @@ -16,30 +16,32 @@ #include +#include "data/procedure.h" + #include #include #include #include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include "minmax.h" -#include "xalloc.h" +#include "data/case.h" +#include "data/case-map.h" +#include "data/caseinit.h" +#include "data/casereader.h" +#include "data/casereader-provider.h" +#include "data/casereader-shim.h" +#include "data/casewriter.h" +#include "data/dictionary.h" +#include "data/file-handle-def.h" +#include "data/transformations.h" +#include "data/variable.h" +#include "libpspp/deque.h" +#include "libpspp/misc.h" +#include "libpspp/str.h" +#include "libpspp/taint.h" +#include "libpspp/i18n.h" + +#include "gl/minmax.h" +#include "gl/xalloc.h" struct dataset { /* Cases are read from source, @@ -92,8 +94,9 @@ struct dataset { but proc_commit not yet called. */ } proc_state; - casenumber cases_written; /* Cases output so far. */ - bool ok; /* Error status. */ + casenumber cases_written; /* Cases output so far. */ + bool ok; /* Error status. */ + struct casereader_shim *shim; /* Shim on proc_open() casereader. */ void (*callback) (void *); /* Callback for when the dataset changes */ void *cb_data; @@ -165,6 +168,8 @@ static const struct casereader_class proc_casereader_class; struct casereader * proc_open (struct dataset *ds) { + struct casereader *reader; + assert (ds->source != NULL); assert (ds->proc_state == PROC_COMMITTED); @@ -217,9 +222,19 @@ proc_open (struct dataset *ds) /* FIXME: use taint in dataset in place of `ok'? */ /* FIXME: for trivial cases we can just return a clone of ds->source? */ - return casereader_create_sequential (NULL, dict_get_proto (ds->dict), - CASENUMBER_MAX, - &proc_casereader_class, ds); + + /* Create casereader and insert a shim on top. The shim allows us to + arbitrarily extend the casereader's lifetime, by slurping the cases into + the shim's buffer in proc_commit(). That is especially useful when output + table_items are generated directly from the procedure casereader (e.g. by + the LIST procedure) when we are using an output driver that keeps a + reference to the output items passed to it (e.g. the GUI output driver in + PSPPIRE). */ + reader = casereader_create_sequential (NULL, dict_get_proto (ds->dict), + CASENUMBER_MAX, + &proc_casereader_class, ds); + ds->shim = casereader_shim_insert (reader); + return reader; } /* Returns true if a procedure is in progress, that is, if @@ -298,6 +313,11 @@ proc_casereader_destroy (struct casereader *reader, void *ds_) struct dataset *ds = ds_; struct ccase *c; + /* We are always the subreader for a casereader_buffer, so if we're being + destroyed then it's because the casereader_buffer has read all the cases + that it ever will. */ + ds->shim = NULL; + /* Make sure transformations happen for every input case, in case they have side effects, and ensure that the replacement active file gets all the cases it should. */ @@ -318,6 +338,9 @@ proc_casereader_destroy (struct casereader *reader, void *ds_) bool proc_commit (struct dataset *ds) { + if (ds->shim != NULL) + casereader_shim_slurp (ds->shim); + assert (ds->proc_state == PROC_CLOSED); ds->proc_state = PROC_COMMITTED;