Re: Match files bug

[Ben Pfaff]

John Darrington <address@hidden> writes:

> The problem is, that one of the linked list elements may have a dict
> member that is equal to mtf->dict.  In which case it gets destroyed
> twice -- once in the for loop and once in the explicit call to
> dict_destroy that comes after the loop.

Yes, it was definitely a double-free, although not right there.
Instead, it was that cmd_match_files() was freeing default_dict
and then mtf_free(), through mtf_close_file(), was doing the same
thing.  But it was only doing that because mtf_processing() was
returning a spurious error; otherwise mtf_free() wouldn't have
any elements left to free anyhow.  The latter bug revealed the
former, in other words.

> For that matter, is there anything preventing two or more members of
> the linked list from sharing the same dict member ?

Yes.  Each member represents has a different dictionary, even if
they represent the same file.  I'm not worried about that.

Here's the fix.  The first change is cosmetic, the rest are real
fixes.  Please apply.

diff -X pat -urpN feb13/src/language/data-io/get.c 
feb13-0/src/language/data-io/get.c
--- feb13/src/language/data-io/get.c    2006-02-11 20:54:28.000000000 -0800
+++ feb13-0/src/language/data-io/get.c  2006-02-15 22:16:47.000000000 -0800
@@ -1131,8 +1131,10 @@ cmd_match_files (void)
 
   if (!mtf_read_nonactive_records (&mtf))
     goto error;
-  ok = !used_active_file || procedure (mtf_processing, &mtf);
-  if (ok)
+
+  if (used_active_file) 
+    ok = procedure (mtf_processing, &mtf) && mtf_processing_finish (&mtf);
+  else
     ok = mtf_processing_finish (&mtf);
 
   free_case_source (vfm_source);
@@ -1145,7 +1147,7 @@ cmd_match_files (void)
   free_case_sink (mtf.sink);
   
   if (!mtf_free (&mtf))
-    goto error;
+    ok = false;
   return ok ? CMD_SUCCESS : CMD_CASCADING_FAILURE;
   
  error:
@@ -1207,7 +1209,7 @@ mtf_close_file (struct mtf_file *file)
   bool ok = file->reader == NULL || !any_reader_error (file->reader);
   free (file->by);
   any_reader_close (file->reader);
-  if (file->dict != default_dict)
+  if (file->handle != NULL)
     dict_destroy (file->dict);
   case_destroy (&file->input);
   free (file->in_name);
@@ -1390,7 +1392,7 @@ mtf_processing (struct ccase *c, void *m
               else /* cmp > 0 */
                 {
                   if (iter->handle == NULL)
-                    return false;
+                    return true;
                   if (any_reader_read (iter->reader, &iter->input))
                     continue;
                   if (!mtf_delete_file_in_place (mtf, &iter))

-- 
"I was born lazy.  I am no lazier now than I was forty years ago, 
 but that is because I reached the limit forty years ago.  You can't 
 go beyond possibility."
--Mark Twain