[MPlayer-dev-eng] CONF_TYPE_STRING issue

[Reimar Döffinger]

On Thu, May 05, 2011 at 12:40:46PM +0200, Ingo Brückl wrote:
> Reimar Döffinger wrote on Wed, 4 May 2011 23:15:35 +0200:
> 
> > Below seems to work for me, [...]
> > Index: m_option.c
> > ===================================================================
> > +++ m_option.c  (working copy)
> > @@ -413,7 +413,7 @@
> >  }
> >
> >  static void copy_str(const m_option_t* opt,void* dst, const void* src) {
> > -  if(dst && src) {
> > +  if(dst && src && VAL(dst) != VAL(src)) {
> >  #ifndef NO_FREE
> >      free(VAL(dst)); //FIXME!!!
> >  #endif
> > @@ -421,6 +421,12 @@
> >    }
> >  }
> >
> > +static void set_str(const m_option_t* opt,void* dst, const void* src) {
> > +  if(dst && src) {
> > +    VAL(dst) = VAL(src);
> > +  }
> > +}
> > +
> >  static void free_str(void* src) {
> >    if(src && VAL(src)){
> >  #ifndef NO_FREE
> > @@ -438,8 +444,8 @@
> >    parse_str,
> >    print_str,
> >    copy_str,
> > +  set_str,
> >    copy_str,
> > -  copy_str,
> >    free_str
> >  };
> >
> 
> Wouldn't this give direct access to the slot data, meaning that changing the
> variable (for what reason ever) disables later restoring it from the initial
> config parsed value?

By what I can tell from the code it's not supposed to restore to the
config parsed value.
However I realized it didn't quite do what I intended, does attached
patch look better to you?
It should avoid the issue you pointed out: setting the variable to
a non-malloced value. It is not necessary to support it, but if we can...
However your patch should used partially as well, just instead of
calling m_option_free opt->p should be set to NULL.
I noticed that due to a bug in mplayer.c: It first frees the config
and then still uses mp_msg. Without the NULLing it will try to use
free'd memory.

> Will push/pop still work?

I think so.

> BTW, there are other M_OPT_TYPE_DYNAMICs as well, not only strings.

I know, one thing at a time was the idea.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: opt.diff
Type: text/x-diff
Size: 2290 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20110505/3ebde15c/attachment.bin>