On 04/18, Stefan Beller wrote:
> 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.
I don't really see the value in this, as the only caller doesn't need it
right now.
>
> > /*
> > + * Removes the given string from the sorted list.
>
> What happens when the string is not found?
nothing.
>
> > + */
> > +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 ?
If I need to do another reroll I can mark it extern.
--
Brandon Williams