[RFC PATCH] cli: factor out config handling code to get/set lists.
Austin Clements
amdragon at MIT.EDU
Sat Dec 10 10:24:36 PST 2011
Deduplicating this code seems like a great idea, but I don't think
macros are the way to do it; especially not one that expands to an
important top-level construct like a function definition.
What about something like
const char **
notmuch_config_get_user_other_email (notmuch_config_t *config,
size_t *length)
{
return _config_get_list (config, "user", "other_email",
&config->user_other_email, length);
}
where config->user_other_email is a, say, struct _notmuch_config_list
that combines together the cached string list and length?
_config_get_list would look a lot like what you have now, but outlist
would be a struct _notmuch_config_list* instead of a const char*** and
it would always set *length (in addition to setting it in the cached
value if necessary).
Set could be similarly simple
void
notmuch_config_set_user_other_email (notmuch_config_t *config,
const char *other_email[],
size_t length)
{
_config_set_list (config, "user", "other_email", &config->user_other_email,
other_email, length);
}
Quoth David Bremner on Dec 10 at 1:50 pm:
> From: David Bremner <bremner at debian.org>
>
> The code is already duplicated once, and I want to add a third
> configuration item that is also a list.
> ---
>
> Mainly I am curious if people think using macros to declare these
> "getters" and "setters" makes the code less maintainable.
>
> notmuch-config.c | 112 +++++++++++++++++++----------------------------------
> 1 files changed, 40 insertions(+), 72 deletions(-)
>
> diff --git a/notmuch-config.c b/notmuch-config.c
> index 1a7ed58..33778a7 100644
> --- a/notmuch-config.c
> +++ b/notmuch-config.c
> @@ -520,93 +520,61 @@ notmuch_config_set_user_primary_email (notmuch_config_t *config,
> config->user_primary_email = NULL;
> }
>
> -const char **
> -notmuch_config_get_user_other_email (notmuch_config_t *config,
> - size_t *length)
> +static void
> +_config_get_list (notmuch_config_t *config,
> + const char *section, const char *key,
> + const char ***outlist, size_t *length)
> {
> - char **emails;
> - size_t emails_length;
> + char **inlist;
> unsigned int i;
>
> - if (config->user_other_email == NULL) {
> - emails = g_key_file_get_string_list (config->key_file,
> - "user", "other_email",
> - &emails_length, NULL);
> - if (emails) {
> - config->user_other_email = talloc_size (config,
> - sizeof (char *) *
> - (emails_length + 1));
> - for (i = 0; i < emails_length; i++)
> - config->user_other_email[i] = talloc_strdup (config->user_other_email,
> - emails[i]);
> - config->user_other_email[i] = NULL;
> -
> - g_strfreev (emails);
> -
> - config->user_other_email_length = emails_length;
> + if (*outlist == NULL) {
> + inlist = g_key_file_get_string_list (config->key_file,
> + section, key,
> + length, NULL);
> + if (inlist) {
> + *outlist = talloc_size (config, sizeof (char *) *
> + (*length + 1));
> + for (i = 0; i < *length; i++)
> + (*outlist)[i] = talloc_strdup (*outlist, inlist[i]);
> + (*outlist)[i] = NULL;
> +
> + g_strfreev (inlist);
> +
> }
> }
>
> - *length = config->user_other_email_length;
> - return config->user_other_email;
> }
>
> -void
> -notmuch_config_set_user_other_email (notmuch_config_t *config,
> - const char *other_email[],
> - size_t length)
> -{
> - g_key_file_set_string_list (config->key_file,
> - "user", "other_email",
> - other_email, length);
> -
> - talloc_free (config->user_other_email);
> - config->user_other_email = NULL;
> +#define _GET_LIST(list, group, name) \
> +const char ** \
> +notmuch_config_get_##list (notmuch_config_t *config, size_t *length) \
> +{ \
> + _config_get_list (config, group, name, &(config->list), \
> + &(config->list##_length)); \
> + *length = config->list##_length; \
> + return config->list; \
> }
>
> -const char **
> -notmuch_config_get_new_tags (notmuch_config_t *config,
> - size_t *length)
> -{
> - char **tags;
> - size_t tags_length;
> - unsigned int i;
> +_GET_LIST (user_other_email, "user", "other_email");
> +_GET_LIST (new_tags, "new", "tags");
>
> - if (config->new_tags == NULL) {
> - tags = g_key_file_get_string_list (config->key_file,
> - "new", "tags",
> - &tags_length, NULL);
> - if (tags) {
> - config->new_tags = talloc_size (config,
> - sizeof (char *) *
> - (tags_length + 1));
> - for (i = 0; i < tags_length; i++)
> - config->new_tags[i] = talloc_strdup (config->new_tags,
> - tags[i]);
> - config->new_tags[i] = NULL;
> -
> - g_strfreev (tags);
> -
> - config->new_tags_length = tags_length;
> - }
> - }
> +#undef _GET_LIST
>
> - *length = config->new_tags_length;
> - return config->new_tags;
> +#define _SET_LIST(list, group, name) \
> +void \
> +notmuch_config_set_##list (notmuch_config_t *config, const char *list[], \
> + size_t length) \
> +{ \
> + g_key_file_set_string_list (config->key_file, group, name, list, length); \
> + talloc_free (config->list); \
> + config->list = NULL; \
> }
>
> -void
> -notmuch_config_set_new_tags (notmuch_config_t *config,
> - const char *new_tags[],
> - size_t length)
> -{
> - g_key_file_set_string_list (config->key_file,
> - "new", "tags",
> - new_tags, length);
> +_SET_LIST(user_other_email, "user", "other_email");
> +_SET_LIST(new_tags, "new", "tags");
>
> - talloc_free (config->new_tags);
> - config->new_tags = NULL;
> -}
> +#undef _SET_LIST
>
> /* Given a configuration item of the form <group>.<key> return the
> * component group and key. If any error occurs, print a message on
More information about the notmuch
mailing list