Am 23.06.2014 12:11, schrieb Tanay Abhra:


> +
> +static struct config_cache_entry *config_cache_find_entry(const char *key)
> +{
> +     struct hashmap *config_cache;
> +     struct config_cache_entry k;
> +     struct config_cache_entry *found_entry;
> +     char *normalized_key;
> +     int ret;
> +     config_cache = get_config_cache();
> +     ret = git_config_parse_key(key, &normalized_key, NULL);

[I didn't follow all previous discussion, so feel free to ignore me if this has 
been discussed already]

Is it really necessary to normalize keys on each lookup? The current git config 
code simply does a 'strcmp("lower-case-key", var)' so normalization shouldn't 
be necessary for the majority of callers. If a caller uses a non-constant key 
(e.g. as passed to 'git config --get <name>'), it would be the caller's 
responsibility to normalize.


> +int git_config_get_string(const char *key, const char **value)

I would have expected '..._get_string' to return a string. If the return value 
indicates whether something was found, it should probably be called 

In the end, you want typed config functions that do type conversion and handle 
parse errors, at least for the common types bool/int/string... Thus the generic 
function that returns unparsed data should probably be called '..._value', not 

A typical pull-style config API will also let you specify default values, e.g.:

  const char *git_config_get_string(const char *key, const char *default_value)
    const char *value;
    if (!git_config_find_value(key, &value))
      return default_value;
    if (!value)
    return value;

  int git_config_get_bool(const char *key, int default_value)
    const char *value;
    if (!git_config_find_value(key, &value))
      return default_value;
    return git_config_bool(key, value);

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to
More majordomo info at

Reply via email to