Re: svn commit: r1590751 - /subversion/trunk/subversion/svn/svn.c

> Author: philip

> URL: http://svn.apache.org/r1590751
> Log:
> If reading the users config fails, say because $HOME is unreadable,
> provide an empty config rather than a NULL config.  This fixes a
> SEGV and allows command line options that override the default
> config to work.

Hi Philip.

I'm wondering why you create the two SVN_CONFIG_CATEGORY_... entries in
the hash? If there's a need for a standard empty config to be created that's more than just an empty hash, we should have a constructor function for it.

The main thing 'svn' does with the config hash is pass it to svn_client_create_context2(), and that already claims to accept NULL. The only other things we do with it are to call svn_hash_gets(), which could be trivially conditionalized, and call svn_cmdline__apply_config_options(). That last call is the only place where we need to actually create a hash. Any reason not to do it there?

What about the other command-line programs, don't they want the same? svnmucc had the same code in it, and svnrdump and svnsync both call svn_config_get_config() without any error handling at all, and presumably all of these should handle these conditions like 'svn' does. svnadmin has a config_dir option but doesn't use it. (We should maybe fix its help to say it's unused, or remove the option, or something.) The other programs don't use a config dir.

I note that you proposed this for back-port to 1.8.

- Julian

>
> * subversion/svn/svn.c
>   (sub_main): Provide a fallback empty config.

> Modified: subversion/trunk/subversion/svn/svn.c
> ==============================================================================
> --- subversion/trunk/subversion/svn/svn.c (original)
> +++ subversion/trunk/subversion/svn/svn.c Mon Apr 28 19:11:38 2014
> @@ -2590,9 +2590,15 @@ sub_main(int *exit_code, int argc, const
>       if (APR_STATUS_IS_EACCES(err->apr_err)
>           || SVN__APR_STATUS_IS_ENOTDIR(err->apr_err))
>         {
> +          svn_config_t *empty_cfg;
> +
>           svn_handle_warning2(stderr, err, "svn: ");
>           svn_error_clear(err);
> -          cfg_hash = NULL;
> +          cfg_hash = apr_hash_make(pool);
> +          SVN_ERR(svn_config_create2(&empty_cfg, FALSE, FALSE, pool));
> +          svn_hash_sets(cfg_hash, SVN_CONFIG_CATEGORY_CONFIG, empty_cfg);
> +          SVN_ERR(svn_config_create2(&empty_cfg, FALSE, FALSE, pool));
> +          svn_hash_sets(cfg_hash, SVN_CONFIG_CATEGORY_SERVERS, empty_cfg);
>         }
>       else
>         return err;
>
Received on 2014-04-29 11:00:22 CEST