All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Tanay Abhra <tanayabh@gmail.com>
Cc: git@vger.kernel.org, Ramkumar Ramachandra <artagnon@gmail.com>,
	Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Subject: Re: [PATCH 4/7] add line number and file name info to `config_set`
Date: Wed, 23 Jul 2014 15:24:31 -0700	[thread overview]
Message-ID: <xmqqy4vjbtnk.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1406140978-9472-5-git-send-email-tanayabh@gmail.com> (Tanay Abhra's message of "Wed, 23 Jul 2014 11:42:55 -0700")

Tanay Abhra <tanayabh@gmail.com> writes:

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

  reply	other threads:[~2014-07-23 22:24 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-23 18:42 [PATCH 0/7] Rewrite `git_config()` using config-set API Tanay Abhra
2014-07-23 18:42 ` [PATCH 1/7] config.c: fix accuracy of line number in errors Tanay Abhra
2014-07-23 21:49   ` Junio C Hamano
2014-07-24 13:33     ` Tanay Abhra
2014-07-24 18:31       ` Junio C Hamano
2014-07-24 18:40         ` Tanay Abhra
2014-07-24 18:49           ` Matthieu Moy
2014-07-23 18:42 ` [PATCH 2/7] rewrite git_config() to use the config-set API Tanay Abhra
2014-07-23 19:55   ` Matthieu Moy
2014-07-23 19:58   ` Matthieu Moy
2014-07-23 21:58   ` Junio C Hamano
2014-07-24  6:43     ` Matthieu Moy
2014-07-23 18:42 ` [PATCH 3/7] add a test for semantic errors in config files Tanay Abhra
2014-07-23 19:55   ` Matthieu Moy
2014-07-23 22:11     ` Junio C Hamano
2014-07-24 13:56       ` Tanay Abhra
2014-07-24 14:05         ` Matthieu Moy
2014-07-24 16:41           ` Junio C Hamano
2014-07-23 18:42 ` [PATCH 4/7] add line number and file name info to `config_set` Tanay Abhra
2014-07-23 22:24   ` Junio C Hamano [this message]
2014-07-23 18:42 ` [PATCH 5/7] enforce `xfuncname` precedence over `funcname` Tanay Abhra
2014-07-23 20:05   ` Matthieu Moy
2014-07-23 21:04   ` Eric Sunshine
2014-07-23 22:34   ` Junio C Hamano
2014-07-24  6:39     ` Matthieu Moy
2014-07-24 17:09       ` Junio C Hamano
2014-07-24 18:33         ` Tanay Abhra
2014-07-24 18:47           ` Matthieu Moy
2014-07-24 19:04             ` John Keeping
2014-07-24 19:20             ` Junio C Hamano
2014-07-24 19:29               ` Tanay Abhra
2014-07-24 19:54                 ` Junio C Hamano
2014-07-24 21:22                   ` Ramsay Jones
2014-07-25  3:16                     ` Tanay Abhra
2014-07-25  6:54                       ` Matthieu Moy
2014-07-25 17:09                         ` Junio C Hamano
2014-07-25 17:45                           ` Matthieu Moy
2014-07-25 18:55                             ` Junio C Hamano
2014-07-23 18:42 ` [PATCH 6/7] config: add `git_die_config()` to the config-set API Tanay Abhra
2014-07-23 18:42 ` [PATCH 7/7] Add tests for `git_config_get_string()` Tanay Abhra
2014-07-23 20:08   ` Matthieu Moy
2014-07-23 19:38 ` [PATCH 0/7] Rewrite `git_config()` using config-set API Matthieu Moy
2014-07-23 21:44 ` Junio C Hamano
2014-07-24 15:04   ` Tanay Abhra
2014-07-24 15:39     ` Matthieu Moy
2014-07-24 15:59       ` Tanay Abhra
2014-07-24 16:17         ` Matthieu Moy
2014-07-24 20:03           ` Junio C Hamano
2014-07-24 15:06   ` [PATCH v12 1/2] add `config_set` API for caching config-like files Tanay Abhra
2014-07-24 18:41     ` Junio C Hamano
2014-07-24 15:09   ` [PATCH v12 2/2] test-config: add tests for the config_set API Tanay Abhra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqy4vjbtnk.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --cc=artagnon@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=tanayabh@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.