Re: [PATCH v2 2/2] config: Add hashtable for config parsing & retrieval
- From
Jeff King <peff@peff.net>
- Date
- Jun 17, 2014, 05:34 UTC
- Message-ID
- <20140617053436.GB29957@sigill.intra.peff.net>
- In-Reply-To
- <1402907232-24629-3-git-send-email-tanayabh@gmail.com>
On Mon, Jun 16, 2014 at 01:27:12AM -0700, Tanay Abhra wrote:
Show 5 quoted lines
> +static int config_cache_callback(const char *key, const char *value, void *unused)
> +{
> + config_cache_set_value(key, value);
> + return 0;
> +}This gets the normalized key, which is good; you should be able to just hash/lookup values without further munging as long as they, too, are normalized (you may want to note that in the documentaiton for config_cache_find_entry and friends).
The value may be a string value, or NULL if it is a true boolean. That value gets passed to...
Show 19 quoted lines
> +static int config_cache_set_value(const char *key, const char *value)
> +{
> + struct hashmap *config_cache;
> + struct config_cache_entry *e;
> +
> + config_cache = get_config_cache();
> + e = config_cache_find_entry(key);
> + if (!e) {
> + e = xmalloc(sizeof(*e));
> + hashmap_entry_init(e, strhash(key));
> + e->key = xstrdup(key);
> + string_list_init_dup(&e->value_list);
> + string_list_append(&e->value_list, value);
> + hashmap_add(config_cache, e);
> + } else {
> + string_list_append(&e->value_list, value);
> + }
> + return 0;
> +}...this function, which then passes it to string_list_append. Which is going to call xstrdup() on it, which will then segfault.
You need some mechanism to store entries that are NULL. It may be enough to silently convert them into the string "true" inside the cached storage. But there may be callers who treat NULL specially (e.g., a tri-state true/false/auto that treats a bare boolean as "auto"); you'd need to check.
The other alternative is to use something besides string_list that can handle a NULL. You may also need to give some thought to how such NULLs would be handled by git_config_get_string() (since a NULL there also means "not found").
-Peff