Re: [PATCH 4/7] add line number and file name info to `config_set`

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 23, 2014, 22:24 UTC
Message-ID: <xmqqy4vjbtnk.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1406140978-9472-5-git-send-email-tanayabh@gmail.com>

Tanay Abhra <tanayabh@gmail.com> writes:

Show 6 quoted lines

> @@ -1287,6 +1300,8 @@ static struct config_set_element *configset_find_element(struct config_set *cs,
>  static int configset_add_value(struct config_set *cs, const char *key, const char *value)
>  {
>  	struct config_set_element *e;
> +	struct key_value_info *kv_info = xmalloc(sizeof(*kv_info));
> +	struct string_list_item *si;

I have this suspicion that we may want to eventually build a custom "config_value_list" API that is very similar to what string_list does, with the only difference from string_list being that the util item of config_value_item (i.e. a parallel to string_list_item) would not be a "void *" that points at anything, but is "struct key_value_info" embedded within, so that we do not have to waste a pointer and fragmented allocation.

I suspect such a config_value_list API must be built on top of a kind of generics which C does not allow, which would mean we would be doing some preprocessor macro tricks (similar to the way how commit-slab.h allows different kinds of payload) that lets us build a templated "string-list" API, discarding the existing "string-list.[ch]" and replacing them with something like these two liners:

    declare_generic_string_list(string_list, void *); /* in string-list.h */
    define_generic_string_list(string_list, void *); /* in string-list.c */

And at that point,

    declare_generic_string_list(config_value_list, struct key_value);
    define_generic_string_list(config_value_list, struct key_value);

would give us an API declaration and implementation that parallel that of string-list, but with "struct key_value" in the util field of each item.

But that kind of clean-up can come much later after this series settles, and what this patch has is fine for now.