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

[Ingo Brückl]

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?

> Hm, if it's not supposed to work properly with multiple includes etc.
> anyway, why going this complicated way?

Well, it seems it does.

> 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 (unfortunately currently with different names) and a
few GUI specific ones (unfortunately currently not properly using a common
gui namespace). I don't think that a different option name would make sense,
the gui prefix already should indicate that it is gui-ish.

Ingo