Jonathan Nieder <jrnieder@xxxxxxxxx> writes:
> D. Eliminate for_each_string_list_item and let callers just do
>
> unsigned int i;
> for (i = 0; i < list->nr; i++) {
> struct string_list_item *item = list->items[i];
> ...
> }
>
> Having to declare item is unnecessarily verbose, decreasing the
> appeal of this option. I think I like it anyway, but I wasn't able
> to convince coccinelle to do it.
When using the macro, item still needs to be declared outside by the
user, so it's not all that unpleasant, though.
> E. Use subtraction instead of addition:
> #define for_each_string_list_item(item, list) \
> for (item = ...; \
> (item == list->items ? 0 : item - list->items) < nr; \
> item++)
>
> I expected the compiler to figure out that this is a long way of writing
> (item - list->items), but at least with gcc 4.8.4 -O2, no such
> luck. This generates uglier assembly than the NULL check.
Yuck. You cannot easily unsee such an ugliness X-<.
The patch and explanation above --- looked quite nicely written.
Will queue.
Thanks.
> diff --git a/string-list.h b/string-list.h
> index 29bfb7ae45..79ae567cbc 100644
> --- a/string-list.h
> +++ b/string-list.h
> @@ -32,8 +32,10 @@ void string_list_clear_func(struct string_list *list, string_list_clear_func_t c
> typedef int (*string_list_each_func_t)(struct string_list_item *, void *);
> int for_each_string_list(struct string_list *list,
> string_list_each_func_t, void *cb_data);
> -#define for_each_string_list_item(item,list) \
> - for (item = (list)->items; item < (list)->items + (list)->nr; ++item)
> +#define for_each_string_list_item(item,list) \
> + for (item = (list)->items; \
> + item && item < (list)->items + (list)->nr; \
> + ++item)
>
> /*
> * Apply want to each item in list, retaining only the ones for which
