Re: [PATCH] Dataset and PsppireDict to have their own copy of struct dictionary

[John Darrington]

On Sun, Sep 23, 2018 at 10:55:14PM -0700, Ben Pfaff wrote:
     On Sun, Sep 23, 2018 at 03:02:35PM +0200, John Darrington wrote:
     > diff --git a/src/data/dataset.c b/src/data/dataset.c
     > index 7a5a6a4a..15774eec 100644
     > --- a/src/data/dataset.c
     > +++ b/src/data/dataset.c
     > @@ -293,7 +293,8 @@ dataset_set_dict (struct dataset *ds, struct 
dictionary *dict)
     >    dataset_clear (ds);
     >  
     >    dict_destroy (ds->dict);
     > -  ds->dict = dict;
     > +  ds->dict = dict_clone (dict);
     > +
     >    dict_set_change_callback (ds->dict, dict_callback, ds);
     >  }
     
     The above is not obviously to me a safe change.
     
     The caller might assume that the dictionary passed in is actually going
     to be used as the dictionary; did you check whether the callers rely on
     that?

You are right.  That is another reason why ref-counting is a better
option.  
     
     Also, currently the dictionary takes ownership of the dictionary
     argument, I believe, and it appears to me that this will leak that
     dictionary.

Yep.  I took the view that a memory leak was the lesser evil than
referencing freed memory.

J'