Re: Plugging the leaks

[Ben Pfaff]

John Darrington <address@hidden> writes:

> tests/bugs/case-map.sh leaks memory.
>
> The attached patch fixes the leak, but causes
> tests/command/n_of_cases.sh to fail.
>
> I haven't managed to work out what its problem is. Maybe somebody can
> tell me --- if not I'll have another look at it next week.

The problem is an impedance mismatch between
casereader_create_translator and the translator function that it
calls.  The translator function is supposed to destroy its input
case, according to the casereader_create_translator function
comment, but the translator function actually used in get.c
(get_translate_case) doesn't do that.  Nor is it properly able to
do so, because the input case is "const".

Actually, the real problem is with casewriter_create_translator,
but the problem is slightly worse there because that function has
no comment at all.

Here's a patch that fixes the problem for both functions.  (I
didn't run a leak check or valgrind on the whole tree, only on
case-map.sh.  It does pass "make check".)

---
 src/data/casereader-translator.c |    5 ++---
 src/data/casereader.h            |    2 +-
 src/data/casewriter-translator.c |   17 ++++++++++++++---
 src/data/casewriter.h            |    2 +-
 src/language/data-io/get.c       |    6 +++---
 5 files changed, 21 insertions(+), 11 deletions(-)

--- merge.orig/src/data/casereader-translator.c
+++ merge/src/data/casereader-translator.c
@@ -33,8 +33,7 @@ struct casereader_translator
   {
     struct casereader *subreader; /* Source of input cases. */
 
-    void (*translate) (const struct ccase *input, struct ccase *output,
-                       void *aux);
+    void (*translate) (struct ccase *input, struct ccase *output, void *aux);
     bool (*destroy) (void *aux);
     void *aux;
   };
@@ -56,7 +55,7 @@ static struct casereader_class casereade
 struct casereader *
 casereader_create_translator (struct casereader *subreader,
                               size_t output_value_cnt,
-                              void (*translate) (const struct ccase *input,
+                              void (*translate) (struct ccase *input,
                                                  struct ccase *output,
                                                  void *aux),
                               bool (*destroy) (void *aux),
--- merge.orig/src/data/casereader.h
+++ merge/src/data/casereader.h
@@ -105,7 +105,7 @@ casereader_create_counter (struct casere
 
 struct casereader *
 casereader_create_translator (struct casereader *, size_t output_value_cnt,
-                              void (*translate) (const struct ccase *input,
+                              void (*translate) (struct ccase *input,
                                                  struct ccase *output,
                                                  void *aux),
                               bool (*destroy) (void *aux),
--- merge.orig/src/data/casewriter-translator.c
+++ merge/src/data/casewriter-translator.c
@@ -29,18 +29,29 @@ struct casewriter_translator
   {
     struct casewriter *subwriter;
 
-    void (*translate) (const struct ccase *input, struct ccase *output,
-                       void *aux);
+    void (*translate) (struct ccase *input, struct ccase *output, void *aux);
     bool (*destroy) (void *aux);
     void *aux;
   };
 
 static struct casewriter_class casewriter_translator_class;
 
+/* Creates and returns a new casewriter whose cases are passed
+   through TRANSLATE, which must create case OUTPUT, with
+   OUTPUT_VALUE_CNT values, and populate it based on INPUT and
+   auxiliary data AUX.  The translated cases are then written to
+   SUBWRITER.  TRANSLATE must also destroy INPUT.
+
+   When the translating casewriter is destroyed, DESTROY will be
+   called to allow any state maintained by TRANSLATE to be freed.
+
+   After this function is called, SUBWRITER must not ever again
+   be referenced directly.  It will be destroyed automatically
+   when the translating casewriter is destroyed. */
 struct casewriter *
 casewriter_create_translator (struct casewriter *subwriter,
                               size_t translated_value_cnt,
-                              void (*translate) (const struct ccase *input,
+                              void (*translate) (struct ccase *input,
                                                  struct ccase *output,
                                                  void *aux),
                               bool (*destroy) (void *aux),
--- merge.orig/src/data/casewriter.h
+++ merge/src/data/casewriter.h
@@ -43,7 +43,7 @@ struct casewriter *autopaging_writer_cre
 
 struct casewriter *
 casewriter_create_translator (struct casewriter *, size_t translated_value_cnt,
-                              void (*translate) (const struct ccase *input,
+                              void (*translate) (struct ccase *input,
                                                  struct ccase *output,
                                                  void *aux),
                               bool (*destroy) (void *aux),
--- merge.orig/src/language/data-io/get.c
+++ merge/src/language/data-io/get.c
@@ -60,8 +60,7 @@ enum reader_command
     IMPORT_CMD
   };
 
-static void get_translate_case (const struct ccase *, struct ccase *,
-                                void *map_);
+static void get_translate_case (struct ccase *, struct ccase *, void *map_);
 static bool get_destroy_case_map (void *map_);
 
 /* Parses a GET or IMPORT command. */
@@ -143,11 +142,12 @@ parse_read_command (struct lexer *lexer,
 }
 
 static void
-get_translate_case (const struct ccase *input, struct ccase *output,
+get_translate_case (struct ccase *input, struct ccase *output,
                     void *map_)
 {
   struct case_map *map = map_;
   case_map_execute (map, input, output);
+  case_destroy (input);
 }
 
 static bool

-- 
"A computer is a state machine.
 Threads are for people who cant [sic] program state machines."
--Alan Cox