(Can't sleep...)
Arfrever Frehtes Taifersar Arahesis wrote on Mon, 29 Dec 2008 at 12:24 -0800:
> Author: arfrever
> Date: Mon Dec 29 12:24:29 2008
> New Revision: 34967
>
> Log:
> Implement the '--config-option=FILE:SECTION:OPTION=[VALUE]' option for svn.
>
...
>
> Modified: trunk/subversion/svn/main.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/svn/main.c?pathrev4967&r14966&r24967
> =============================================================================> --- trunk/subversion/svn/main.c Mon Dec 29 11:12:18 2008 (r34966)
> +++ trunk/subversion/svn/main.c Mon Dec 29 12:24:29 2008 (r34967)
> @@ -1020,6 +1023,13 @@ svn_cl__check_cancel(void *baton)
> return SVN_NO_ERROR;
> }
>
> +typedef struct config_option_t
> +{
> + char *file;
> + char *section;
> + char *option;
> + char *value;
s/char/const char/ ?
> +} config_option_t;
>
>
> /*** Main. ***/
> @@ -1424,6 +1434,115 @@ main(int argc, const char *argv[])
> err = svn_utf_cstring_to_utf8(&path_utf8, opt_arg, pool);
> opt_state.config_dir = svn_path_canonicalize(path_utf8, pool);
> break;
> + case opt_config_options:
> + {
> + config_option_t *config_option;
> + int i, e;
> + svn_boolean_t v = FALSE;
Please use longer variable names than 'e' and 'v'.
The code is very hard to read as it is. (However, in this case I think
you can drop them entirely; see below)
> + apr_size_t len = strlen(opt_arg);
> + /* 6 is the length of "x:x:x=". */
> + if (len < 6)
> + {
> + err = svn_error_create
> + (SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> + _("Invalid syntax of argument of --config-option"));
> + return svn_cmdline_handle_exit_error(err, pool, "svn: ");
> + }
> + config_option = apr_pcalloc(pool, sizeof(config_option_t));
> + config_option->option = apr_pcalloc(pool, len);
> + config_option->value = apr_pcalloc(pool, len);
> + e = 0;
> + for (i = 0; i < len; i++)
> + {
> + if (opt_arg[i] == ':')
> + {
> + break;
> + }
> + }
Use strchr()? (see below)
> + if (i == e)
> + {
> + err = svn_error_create
> + (SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> + _("Invalid syntax of argument of --config-option"));
> + return svn_cmdline_handle_exit_error(err, pool, "svn: ");
> + }
> + config_option->file = apr_pcalloc(pool, i - e + 1);
> + memcpy(config_option->file, opt_arg + e, i -e);
Use apr_pstrndup() ? (see below)
> + if (i < len -1)
> + {
> + i++;
> + }
> + else
> + {
> + err = svn_error_create
> + (SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> + _("Invalid syntax of argument of --config-option"));
> + return svn_cmdline_handle_exit_error(err, pool, "svn: ");
> + }
> + e = i;
> + for (; i < len; i++)
> + {
> + if (opt_arg[i] == ':')
> + {
> + break;
> + }
> + }
> + if (i == e)
> + {
> + err = svn_error_create
> + (SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> + _("Invalid syntax of argument of --config-option"));
> + return svn_cmdline_handle_exit_error(err, pool, "svn: ");
> + }
> + config_option->section = apr_pcalloc(pool, i - e + 1);
> + memcpy(config_option->section, opt_arg + e, i - e);
> + if (i < len -1)
> + {
> + i++;
> + }
> + else
> + {
> + err = svn_error_create
> + (SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> + _("Invalid syntax of argument of --config-option"));
> + return svn_cmdline_handle_exit_error(err, pool, "svn: ");
> + }
> + e = i;
> + for (; i < len; i++)
> + {
> + if (opt_arg[i] == '=')
> + {
> + v = TRUE;
> + break;
> + }
> + else if (opt_arg[i] == ':')
> + {
> + break;
> + }
> + }
> + if ((i == e) || (v == FALSE))
> + {
> + err = svn_error_create
> + (SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> + _("Invalid syntax of argument of --config-option"));
> + return svn_cmdline_handle_exit_error(err, pool, "svn: ");
> + }
It looks very complicated. Could you simplify it a little? For example,
function(const char *opt_arg)
{
const char *first_colon, *second_colon, *equal_sign;
if (first_colon = strchr(opt_arg, ':'))
if (second_colon = strchr(first_colon, ':'))
if (equal_sign = strchr(second_colon, '='))
{
/* allocate and return a config_option_t (with apr_pstrndup) */
}
return error;
}
?
> /* Update the options in the config */
> + for (i = 0; i < opt_state.config_options->nelts; i++)
> + {
> + config_option_t *config_option = APR_ARRAY_IDX(opt_state.config_options,
> + i, config_option_t *);
> + if (strcmp(config_option->file, "config") == 0)
> + {
> + svn_config_set(cfg_config, config_option->section,
> + config_option->option, config_option->value);
> + }
> + else if(strcmp(config_option->file, "servers") == 0)
> + {
> + svn_config_set(cfg_servers, config_option->section,
> + config_option->option, config_option->value);
> + }
else
{
/* print a warning */
}
?
> + }
> +
Still can't sleep,
Daniel
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=995351
Received on 2008-12-30 00:46:29 CET