Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Merge branch 'jk/config-parsing-cleanup'

Configuration parsing for tar.* configuration variables were
broken. Introduce a new config-keyname parser API to make the
callers much less error prone.

* jk/config-parsing-cleanup:
  reflog: use parse_config_key in config callback
  help: use parse_config_key for man config
  submodule: simplify memory handling in config parsing
  submodule: use parse_config_key when parsing config
  userdiff: drop parse_driver function
  convert some config callbacks to parse_config_key
  archive-tar: use parse_config_key when parsing config
  config: add helper function for parsing key names
  • Loading branch information...
commit 099ba556d05571001293c8eda10a4fc659f83f48 2 parents 149a421 + b3873c3
@gitster authored
View
10 archive-tar.c
@@ -327,20 +327,12 @@ static struct archiver *find_tar_filter(const char *name, int len)
static int tar_filter_config(const char *var, const char *value, void *data)
{
struct archiver *ar;
- const char *dot;
const char *name;
const char *type;
int namelen;
- if (prefixcmp(var, "tar."))
+ if (parse_config_key(var, "tar", &name, &namelen, &type) < 0 || !name)
return 0;
- dot = strrchr(var, '.');
- if (dot == var + 9)
- return 0;
-
- name = var + 4;
- namelen = dot - name;
- type = dot + 1;
ar = find_tar_filter(name, namelen);
if (!ar) {
View
14 builtin/help.c
@@ -236,21 +236,21 @@ static int add_man_viewer_cmd(const char *name,
static int add_man_viewer_info(const char *var, const char *value)
{
- const char *name = var + 4;
- const char *subkey = strrchr(name, '.');
+ const char *name, *subkey;
+ int namelen;
- if (!subkey)
+ if (parse_config_key(var, "man", &name, &namelen, &subkey) < 0 || !name)
return 0;
- if (!strcmp(subkey, ".path")) {
+ if (!strcmp(subkey, "path")) {
if (!value)
return config_error_nonbool(var);
- return add_man_viewer_path(name, subkey - name, value);
+ return add_man_viewer_path(name, namelen, value);
}
- if (!strcmp(subkey, ".cmd")) {
+ if (!strcmp(subkey, "cmd")) {
if (!value)
return config_error_nonbool(var);
- return add_man_viewer_cmd(name, subkey - name, value);
+ return add_man_viewer_cmd(name, namelen, value);
}
return 0;
View
13 builtin/reflog.c
@@ -510,26 +510,27 @@ static int parse_expire_cfg_value(const char *var, const char *value, unsigned l
static int reflog_expire_config(const char *var, const char *value, void *cb)
{
- const char *lastdot = strrchr(var, '.');
+ const char *pattern, *key;
+ int pattern_len;
unsigned long expire;
int slot;
struct reflog_expire_cfg *ent;
- if (!lastdot || prefixcmp(var, "gc."))
+ if (parse_config_key(var, "gc", &pattern, &pattern_len, &key) < 0)
return git_default_config(var, value, cb);
- if (!strcmp(lastdot, ".reflogexpire")) {
+ if (!strcmp(key, "reflogexpire")) {
slot = EXPIRE_TOTAL;
if (parse_expire_cfg_value(var, value, &expire))
return -1;
- } else if (!strcmp(lastdot, ".reflogexpireunreachable")) {
+ } else if (!strcmp(key, "reflogexpireunreachable")) {
slot = EXPIRE_UNREACH;
if (parse_expire_cfg_value(var, value, &expire))
return -1;
} else
return git_default_config(var, value, cb);
- if (lastdot == var + 2) {
+ if (!pattern) {
switch (slot) {
case EXPIRE_TOTAL:
default_reflog_expire = expire;
@@ -541,7 +542,7 @@ static int reflog_expire_config(const char *var, const char *value, void *cb)
return 0;
}
- ent = find_cfg_ent(var + 3, lastdot - (var+3));
+ ent = find_cfg_ent(pattern, pattern_len);
if (!ent)
return -1;
switch (slot) {
View
15 cache.h
@@ -1170,6 +1170,21 @@ struct config_include_data {
#define CONFIG_INCLUDE_INIT { 0 }
extern int git_config_include(const char *name, const char *value, void *data);
+/*
+ * Match and parse a config key of the form:
+ *
+ * section.(subsection.)?key
+ *
+ * (i.e., what gets handed to a config_fn_t). The caller provides the section;
+ * we return -1 if it does not match, 0 otherwise. The subsection and key
+ * out-parameters are filled by the function (and subsection is NULL if it is
+ * missing).
+ */
+extern int parse_config_key(const char *var,
+ const char *section,
+ const char **subsection, int *subsection_len,
+ const char **key);
+
extern int committer_ident_sufficiently_given(void);
extern int author_ident_sufficiently_given(void);
View
33 config.c
@@ -1681,3 +1681,36 @@ int config_error_nonbool(const char *var)
{
return error("Missing value for '%s'", var);
}
+
+int parse_config_key(const char *var,
+ const char *section,
+ const char **subsection, int *subsection_len,
+ const char **key)
+{
+ int section_len = strlen(section);
+ const char *dot;
+
+ /* Does it start with "section." ? */
+ if (prefixcmp(var, section) || var[section_len] != '.')
+ return -1;
+
+ /*
+ * Find the key; we don't know yet if we have a subsection, but we must
+ * parse backwards from the end, since the subsection may have dots in
+ * it, too.
+ */
+ dot = strrchr(var, '.');
+ *key = dot + 1;
+
+ /* Did we have a subsection at all? */
+ if (dot == var + section_len) {
+ *subsection = NULL;
+ *subsection_len = 0;
+ }
+ else {
+ *subsection = var + section_len + 1;
+ *subsection_len = dot - *subsection;
+ }
+
+ return 0;
+}
View
14 convert.c
@@ -457,7 +457,7 @@ static struct convert_driver {
static int read_convert_config(const char *var, const char *value, void *cb)
{
- const char *ep, *name;
+ const char *key, *name;
int namelen;
struct convert_driver *drv;
@@ -465,10 +465,8 @@ static int read_convert_config(const char *var, const char *value, void *cb)
* External conversion drivers are configured using
* "filter.<name>.variable".
*/
- if (prefixcmp(var, "filter.") || (ep = strrchr(var, '.')) == var + 6)
+ if (parse_config_key(var, "filter", &name, &namelen, &key) < 0 || !name)
return 0;
- name = var + 7;
- namelen = ep - name;
for (drv = user_convert; drv; drv = drv->next)
if (!strncmp(drv->name, name, namelen) && !drv->name[namelen])
break;
@@ -479,8 +477,6 @@ static int read_convert_config(const char *var, const char *value, void *cb)
user_convert_tail = &(drv->next);
}
- ep++;
-
/*
* filter.<name>.smudge and filter.<name>.clean specifies
* the command line:
@@ -490,13 +486,13 @@ static int read_convert_config(const char *var, const char *value, void *cb)
* The command-line will not be interpolated in any way.
*/
- if (!strcmp("smudge", ep))
+ if (!strcmp("smudge", key))
return git_config_string(&drv->smudge, var, value);
- if (!strcmp("clean", ep))
+ if (!strcmp("clean", key))
return git_config_string(&drv->clean, var, value);
- if (!strcmp("required", ep)) {
+ if (!strcmp("required", key)) {
drv->required = git_config_bool(var, value);
return 0;
}
View
14 ll-merge.c
@@ -222,7 +222,7 @@ static const char *default_ll_merge;
static int read_merge_config(const char *var, const char *value, void *cb)
{
struct ll_merge_driver *fn;
- const char *ep, *name;
+ const char *key, *name;
int namelen;
if (!strcmp(var, "merge.default")) {
@@ -236,15 +236,13 @@ static int read_merge_config(const char *var, const char *value, void *cb)
* especially, we do not want to look at variables such as
* "merge.summary", "merge.tool", and "merge.verbosity".
*/
- if (prefixcmp(var, "merge.") || (ep = strrchr(var, '.')) == var + 5)
+ if (parse_config_key(var, "merge", &name, &namelen, &key) < 0 || !name)
return 0;
/*
* Find existing one as we might be processing merge.<name>.var2
* after seeing merge.<name>.var1.
*/
- name = var + 6;
- namelen = ep - name;
for (fn = ll_user_merge; fn; fn = fn->next)
if (!strncmp(fn->name, name, namelen) && !fn->name[namelen])
break;
@@ -256,16 +254,14 @@ static int read_merge_config(const char *var, const char *value, void *cb)
ll_user_merge_tail = &(fn->next);
}
- ep++;
-
- if (!strcmp("name", ep)) {
+ if (!strcmp("name", key)) {
if (!value)
return error("%s: lacks value", var);
fn->description = xstrdup(value);
return 0;
}
- if (!strcmp("driver", ep)) {
+ if (!strcmp("driver", key)) {
if (!value)
return error("%s: lacks value", var);
/*
@@ -289,7 +285,7 @@ static int read_merge_config(const char *var, const char *value, void *cb)
return 0;
}
- if (!strcmp("recursive", ep)) {
+ if (!strcmp("recursive", key)) {
if (!value)
return error("%s: lacks value", var);
fn->recursive = xstrdup(value);
View
43 submodule.c
@@ -126,45 +126,44 @@ void gitmodules_config(void)
int parse_submodule_config_option(const char *var, const char *value)
{
- int len;
struct string_list_item *config;
- struct strbuf submodname = STRBUF_INIT;
+ const char *name, *key;
+ int namelen;
- var += 10; /* Skip "submodule." */
+ if (parse_config_key(var, "submodule", &name, &namelen, &key) < 0 || !name)
+ return 0;
- len = strlen(var);
- if ((len > 5) && !strcmp(var + len - 5, ".path")) {
- strbuf_add(&submodname, var, len - 5);
+ if (!strcmp(key, "path")) {
config = unsorted_string_list_lookup(&config_name_for_path, value);
if (config)
free(config->util);
else
config = string_list_append(&config_name_for_path, xstrdup(value));
- config->util = strbuf_detach(&submodname, NULL);
- strbuf_release(&submodname);
- } else if ((len > 23) && !strcmp(var + len - 23, ".fetchrecursesubmodules")) {
- strbuf_add(&submodname, var, len - 23);
- config = unsorted_string_list_lookup(&config_fetch_recurse_submodules_for_name, submodname.buf);
+ config->util = xmemdupz(name, namelen);
+ } else if (!strcmp(key, "fetchrecursesubmodules")) {
+ char *name_cstr = xmemdupz(name, namelen);
+ config = unsorted_string_list_lookup(&config_fetch_recurse_submodules_for_name, name_cstr);
if (!config)
- config = string_list_append(&config_fetch_recurse_submodules_for_name,
- strbuf_detach(&submodname, NULL));
+ config = string_list_append(&config_fetch_recurse_submodules_for_name, name_cstr);
+ else
+ free(name_cstr);
config->util = (void *)(intptr_t)parse_fetch_recurse_submodules_arg(var, value);
- strbuf_release(&submodname);
- } else if ((len > 7) && !strcmp(var + len - 7, ".ignore")) {
+ } else if (!strcmp(key, "ignore")) {
+ char *name_cstr;
+
if (strcmp(value, "untracked") && strcmp(value, "dirty") &&
strcmp(value, "all") && strcmp(value, "none")) {
warning("Invalid parameter \"%s\" for config option \"submodule.%s.ignore\"", value, var);
return 0;
}
- strbuf_add(&submodname, var, len - 7);
- config = unsorted_string_list_lookup(&config_ignore_for_name, submodname.buf);
- if (config)
+ name_cstr = xmemdupz(name, namelen);
+ config = unsorted_string_list_lookup(&config_ignore_for_name, name_cstr);
+ if (config) {
free(config->util);
- else
- config = string_list_append(&config_ignore_for_name,
- strbuf_detach(&submodname, NULL));
- strbuf_release(&submodname);
+ free(name_cstr);
+ } else
+ config = string_list_append(&config_ignore_for_name, name_cstr);
config->util = xstrdup(value);
return 0;
}
View
3  t/t5000-tar-tree.sh
@@ -212,7 +212,8 @@ test_expect_success 'git-archive --prefix=olde-' '
test_expect_success 'setup tar filters' '
git config tar.tar.foo.command "tr ab ba" &&
git config tar.bar.command "tr ab ba" &&
- git config tar.bar.remote true
+ git config tar.bar.remote true &&
+ git config tar.invalid baz
'
test_expect_success 'archive --list mentions user filter' '
View
57 userdiff.c
@@ -184,35 +184,6 @@ static struct userdiff_driver *userdiff_find_by_namelen(const char *k, int len)
return NULL;
}
-static struct userdiff_driver *parse_driver(const char *var,
- const char *value, const char *type)
-{
- struct userdiff_driver *drv;
- const char *dot;
- const char *name;
- int namelen;
-
- if (prefixcmp(var, "diff."))
- return NULL;
- dot = strrchr(var, '.');
- if (dot == var + 4)
- return NULL;
- if (strcmp(type, dot+1))
- return NULL;
-
- name = var + 5;
- namelen = dot - name;
- drv = userdiff_find_by_namelen(name, namelen);
- if (!drv) {
- ALLOC_GROW(drivers, ndrivers+1, drivers_alloc);
- drv = &drivers[ndrivers++];
- memset(drv, 0, sizeof(*drv));
- drv->name = xmemdupz(name, namelen);
- drv->binary = -1;
- }
- return drv;
-}
-
static int parse_funcname(struct userdiff_funcname *f, const char *k,
const char *v, int cflags)
{
@@ -240,20 +211,34 @@ static int parse_bool(int *b, const char *k, const char *v)
int userdiff_config(const char *k, const char *v)
{
struct userdiff_driver *drv;
+ const char *name, *type;
+ int namelen;
+
+ if (parse_config_key(k, "diff", &name, &namelen, &type) || !name)
+ return 0;
+
+ drv = userdiff_find_by_namelen(name, namelen);
+ if (!drv) {
+ ALLOC_GROW(drivers, ndrivers+1, drivers_alloc);
+ drv = &drivers[ndrivers++];
+ memset(drv, 0, sizeof(*drv));
+ drv->name = xmemdupz(name, namelen);
+ drv->binary = -1;
+ }
- if ((drv = parse_driver(k, v, "funcname")))
+ if (!strcmp(type, "funcname"))
return parse_funcname(&drv->funcname, k, v, 0);
- if ((drv = parse_driver(k, v, "xfuncname")))
+ if (!strcmp(type, "xfuncname"))
return parse_funcname(&drv->funcname, k, v, REG_EXTENDED);
- if ((drv = parse_driver(k, v, "binary")))
+ if (!strcmp(type, "binary"))
return parse_tristate(&drv->binary, k, v);
- if ((drv = parse_driver(k, v, "command")))
+ if (!strcmp(type, "command"))
return git_config_string(&drv->external, k, v);
- if ((drv = parse_driver(k, v, "textconv")))
+ if (!strcmp(type, "textconv"))
return git_config_string(&drv->textconv, k, v);
- if ((drv = parse_driver(k, v, "cachetextconv")))
+ if (!strcmp(type, "cachetextconv"))
return parse_bool(&drv->textconv_want_cache, k, v);
- if ((drv = parse_driver(k, v, "wordregex")))
+ if (!strcmp(type, "wordregex"))
return git_config_string(&drv->word_regex, k, v);
return 0;
Please sign in to comment.
Something went wrong with that request. Please try again.