[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 10:07:45PM +0200, Ingo Brückl wrote:
> Reimar Döffinger wrote on Wed, 26 Oct 2011 20:22:53 +0200:
> 
> > 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.
> 
> > 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).
> 
> I was just doing some tests, but could not figure out any problems. Every
> gui-include seems to override properly what the previous gui-include may
> have set. Maybe you could give me a hint what to look or check for?

Profiles and thus probably also per-protocol or extension options
probably will not be working properly.
No idea if they are supposed to anyway though.

> > And rather not call it include, when it doesn't really behave like that.
> 
> I does behave like include, but limited to the gui options which are a subset
> of the MPlayer options

No, it doesn't really. As said, for example a second "include" will
cause all previous includes to be re-evaluated.
Especially with section this has all kinds of funny and unexpected
effects.
You probably should be able to also see it if you do something like
gui-include=cache.conf # configures cache
nocache=yes
gui-include=empty.conf

should result in cache being enabled at the end.
Which probably can't really be considered wrong with the gui-include
one though...
With include this should now be working correctly.