[MPlayer-dev-eng] [PATCH] Add support for directory-wide configuration overrides V3.0

[Reimar Döffinger]

On Mon, Aug 31, 2009 at 04:07:24PM +0300, Christian P. Schmidt wrote:
> Reimar Döffinger wrote:
> > On Mon, Aug 31, 2009 at 11:52:02AM +0300, Christian P. Schmidt wrote:
> >> Reimar Döffinger wrote:
> >>> On Mon, Aug 31, 2009 at 10:32:19AM +0300, Christian P. Schmidt wrote:
> >>>> Diego Biurrun wrote:
> >>>>> On Sun, Aug 30, 2009 at 03:04:12PM +0300, Christian P. Schmidt wrote:
> >>>>>> --- mplayer.c	(revision 29565)
> >>>>>> +++ mplayer.c	(working copy)
> >>>>>> @@ -916,10 +917,26 @@
> >>>>>>  
> >>>>>> +    if (use_filedir_conf)
> >>>>>> +    {
> >>>>> if (use_filedir_conf) {
> >>>> That breaks the style currently in use in the function. Is it ok to mix
> >>>> styles?
> >>> Sorry you already got the cosmetic comments, the more significant is
> >>> that there is already code to find the start of the file name a few
> >>> lines below, you should move that up and reuse instead of duplicating
> >>> it (so I'll only have to fix one place once I find the time to test on
> >>> Windows).
> >> Right, redid the whole function.
> > 
> > I was thinking of something that makes the whole thing (patch and
> > resulting code) simpler, not more confusing.
> > Something similar to this untested patch:
> 
> Using extra variables is like cheating ;)
> 
> The second patch silences a few warnings by making some variables const.

You should have done svn up first, the try_load_config I used was
already there, the const changes I have already made too (though one
slightly different).
Your check_and_load_config is slightly wrong, because the code is not supposed
to look for a conf file in .mplayer just because e.g. the one in the local
directory has a typo (i.e. the return value if
m_config_parse_config_file is _supposed_ to be ignored).

> -    if ((confpath = get_path (name)) != NULL)
> -    {
> -	if (!stat (confpath, &st))
> -	{
> -	    mp_msg(MSGT_CPLAYER,MSGL_INFO,MSGTR_LoadingConfig, confpath);
> -	    m_config_parse_config_file (conf, confpath);
> -	}
> +        /* Then look up a file specific config file in the file's directory */
> +        if (check_and_load_config(conf, cfg) > 0)
> +            return;
> +    }
>  
> -	free (confpath);
> +    /* Look up a file specific config file in mplayer's config directory */
> +    if ((confpath = get_path(name))) {
> +        check_and_load_config(conf, confpath);
> +        free(confpath);

Mixing cosmetics/whitespace changes and other changes makes the diff an
unreviewable mess.