Tanay Abhra <tanayabh@xxxxxxxxx> writes:
> - git_config(notes_display_config, &load_config_refs);
> + if (load_config_refs) {
> + values = git_config_get_value_multi("notes.displayref");
> + if (values) {
> + for (i = 0; i < values->nr; i++) {
> + if (!values->items[i].string)
> + config_error_nonbool("notes.displayref");
> + else
> + string_list_add_refs_by_glob(&display_notes_refs,
> + values->items[i].string);
> + }
> + }
> + }
It seems to me that you're doing a lot here that should have been done
once in the config API:
* if (values) {
for (i = 0; i < values->nr
=> We could avoid the "if" statement if git_config_get_value_multi was
always returning a string_list, possibly empty (values->nr == 0
instead of values == NULL).
Not as obvious as it seems, because you normally return a pointer to
the string_list that is already in the hashmap, so you can't just
malloc() an empty one if you don't want to leak it.
Another option would be to provide an iterator that would call a
function on each value of the list, and do nothing when there's no
list at all (back to the callback-style API, but you would iterate
only through the values for the right key).
* if (!values->items[i].string)
config_error_nonbool(
=> This check could be done once and for all in a function, say
git_config_get_value_multi_nonbool, a trivial wrapper around
git_config_get_value_multi like
const struct string_list *git_configset_get_value_multi_nonbool(struct config_set *cs, const char *key)
{
struct string_list l = git_configset_get_value_multi(cs, key);
// possibly if(l) depending on the point above.
for (i = 0; i < values->nr; i++) {
if (!values->items[i].string)
git_config_die(key);
}
return l;
}
const struct string_list *git_config_get_value_multi_nonbool(const char *key)
{
git_config_check_init();
return git_configset_get_value_multi_nonbool(&the_config_set, key);
}
(totally untested)
BTW, is it intentional that you call config_error_nonbool() without
die-ing?
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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