[MPlayer-cvslog] r34257 - in trunk: cfg-common.h cfg-mplayer.h m_option.c m_option.h

[Reimar Döffinger]

On Wed, Oct 26, 2011 at 07:31:15PM +0200, Ingo Brückl wrote:
> reimar wrote on Tue, 25 Oct 2011 22:18:35 +0200 (CEST):
> 
> > Author: reimar
> > Date: Tue Oct 25 22:18:35 2011
> > New Revision: 34257
> 
> > Log:
> > Sanitize include behaviour.
> 
> > The normal func_param argument type will iterate over all previous
> > values each time a new value is assigned.
> > This leads e.g. to a complete mess and non-working recursion limiting
> > when creating a config file that includes itself.
> > Seem to also fix bug #1994.
> 
> > Modified:
> >    trunk/cfg-common.h
> >    trunk/cfg-mplayer.h
> >    trunk/m_option.c
> >    trunk/m_option.h
> 
> > Modified: trunk/cfg-mplayer.h
> > ==============================================================================
> > +++ trunk/cfg-mplayer.h Tue Oct 25 22:18:35 2011        (r34257)
> > @@ -301,7 +301,7 @@ const m_option_t mplayer_opts[]={
> >      {"noenqueue", &enqueue, CONF_TYPE_FLAG, 0, 1, 0, NULL},
> >      {"guiwid", "-guiwid has been removed, use -gui-wid instead.\n",
> > CONF_TYPE_PRINT, 0, 0, 0, NULL},
> >      {"gui-wid", &guiWinID, CONF_TYPE_INT, 0, 0, 0, NULL},
> > -    {"gui-include", cfg_gui_include, CONF_TYPE_FUNC_PARAM, CONF_NOSAVE, 0, 0, NULL},
> > +    {"gui-include", cfg_gui_include, CONF_TYPE_FUNC_PARAM_IMMEDIATE, CONF_NOSAVE, 0, 0, NULL},
> 
> This segfaults the GUI.
> 
> If I understand CONF_TYPE_FUNC_PARAM_IMMEDIATE correctly, this isn't the
> desired behaviour either.
> 
> Option gui-include is meant to override the gui.conf settings. In order to do
> this, MPlayer must have called cfg_read() where gui.conf is read and
> m_config_t *gui_conf is set. CONF_TYPE_FUNC_PARAM_IMMEDIATE seems to act
> prior to this, thus gui_conf still is NULL.
> 
> As the GUI's config files don't allow includes anyway, it seems save to stay
> with CONF_TYPE_FUNC_PARAM.

Feel free to revert, crashes are definitely no good.
However you will end up with some really strange behaviour if someone
ever uses multiple gui_include (recursion is just the case where it is
most obvious).
Hm, if it's not supposed to work properly with multiple includes etc.
anyway, why going this complicated way?
You could just make it a string list or whatever and just read them
all in the cfg_read function.
And rather not call it include, when it doesn't really behave like that.