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 '..._find_...'?
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 '..._string'?
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)
config_error_nonbool();
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html