Re: [PATCH v1] config: Add hashtable for config parsing & retrival

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Jun 9, 2014, 14:24 UTC
Message-ID: <vpqmwdmtb2o.fsf@anie.imag.fr>
In-Reply-To: <1402318146-5062-2-git-send-email-tanayabh@gmail.com>

Tanay Abhra <tanayabh@gmail.com> writes:

> +the highest priority(i.e. for the same variable value in the repo config

                       ^
missing space.

Show 5 quoted lines

> +struct config_cache_entry {
> +	struct hashmap_entry ent;
> +	char *key;
> +	struct string_list *value_list;
> +};

I guess this crossed Eric's remark about the fact that this is a pointer.

> +static int hashmap_is_init;

I'd call it hashmap_initialized, but that's a matter of taste.

> +static void config_cache_free(void)

I didn't look closely enough to make sure there were no memory leak remaining, but did you check with valgrind --leak-check that it is the case in practice?

> +	/* contents of config file has changed, so invalidate the
> +	 * config cache used by non-callback based query functions.
> +	 */

Style: Git usually writes multi-line comments

/*
 * like
 * this
 */

(not always applied, but documented in Documentation/CodingGuidelines)

(no time for a more detailed review, sorry)

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/