Re: [PATCH v2 2/2] config: Add hashtable for config parsing & retrieval

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jun 16, 2014 at 01:27:12AM -0700, Tanay Abhra wrote:

> +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...

> +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
--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]