Re: [PATCH v5 05/11] string-list: add string_list_remove function

[Stefan Beller <sbeller@xxxxxxxxxx>]

On Tue, Apr 18, 2017 at 4:17 PM, Brandon Williams <bmwill@xxxxxxxxxx> wrote:

>
> +void string_list_remove(struct string_list *list, const char *string,
> +                       int free_util)
> +{
> +       int exact_match;
> +       int i = get_entry_index(list, string, &exact_match);
> +
> +       if (exact_match) {
> +               if (list->strdup_strings)
> +                       free(list->items[i].string);
> +               if (free_util)
> +                       free(list->items[i].util);
> +
> +               list->nr--;
> +               memmove(list->items + i, list->items + i + 1,
> +                       (list->nr - i) * sizeof(struct string_list_item));
> +       }

Looks correct. I shortly wondered if we'd have any value in returing
`exact_match`, as that may save the caller some code, as I imagine the
caller to be:

  if (!string_list_has_string(&list, string))
    die("BUG: ...");
  string_list_remove(&list, string, 0);

which could be simplified if we had the exact_match returned, i.e.
the string_list_remove returns the implicit string_list_has_string.

>  /*
> + * Removes the given string from the sorted list.

What happens when the string is not found?

> + */
> +void string_list_remove(struct string_list *list, const char *string, int free_util);

How much do we care about (eventual) consistency? ;)
i.e. mark it extern ?

Thanks,
Stefan