On 09/19/2017 02:08 AM, Stefan Beller wrote:
>> I am hoping that this last one is not allowed and we can use the
>> "same condition is checked every time we loop" version that hides
>> the uglyness inside the macro.
>
> By which you are referring to Jonathans solution posted.
> Maybe we can combine the two solutions (checking for thelist
> to not be NULL once, by Jonathan) and using an outer structure
> (SZEDERs solution) by replacing the condition by a for loop,
> roughly (untested):
>
> #define for_each_string_list_item(item,list) \
> - for (item = (list)->items; item < (list)->items + (list)->nr; ++item)
> + for (; list; list = NULL)
> + for (item = (list)->items; item < (list)->items + (list)->nr; ++item)
>
> as that would not mingle with any dangling else clause.
> It is also just one statement, such that
>
> if (bla)
> for_each_string_list_item {
> baz(item);
> }
> else
> foo;
>
> still works.
>
> Are there downsides to this combined approach?
On the plus side, it's pleasantly devious; I wouldn't have thought of
using a `for` loop for the initial test. But it doesn't work as written,
because (1) we don't need to guard against `list` being NULL, but rather
`list->items`; and (2) we don't have the liberty to set `list = NULL`
(or `list->items = NULL`, because `list` is owned by the caller and we
shouldn't modify it.
The following is a bit closer:
#define for_each_string_list_item(item,list) \
for (item = (list)->items; item; item = NULL) \
for (; item < (list)->items + (list)->nr; ++item)
But I think that also fails, because a callsite that does
for_each_string_list_item(myitem, mylist)
if (myitem.util)
break;
would expect that `myitem` is still set after breaking out of the loop,
whereas the outer `for` loop would reset it to NULL.
If `break` were an expression we could do something like
#define for_each_string_list_item(item,list) \
for (item = (list)->items; item; break) \
for (; item < (list)->items + (list)->nr; ++item)
So I think we're still left with the suggestions of Jonathan or Gábor.
Or the bigger change of initializing `string_list::items` to point at an
empty sentinal array (similar to `strbuf_slopbuf`) rather than NULL.
Personally, I think that Jonathan's approach makes the most sense,
unless somebody wants to jump in an implement a `string_list_slopbuf`.
By the way, I wonder if any open-coded loops over `string_lists` make
the same mistake as the macro?
Michael
