Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

[Michael Haggerty <mhagger@alum.mit.edu>]

On 09/17/2017 02:59 AM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> If you pass a newly-initialized or newly-cleared `string_list` to
>> `for_each_string_list_item()`, then the latter does
>>
>>     for (
>>             item = (list)->items; /* note, this is NULL */
>>             item < (list)->items + (list)->nr; /* note: NULL + 0 */
>>             ++item)
>>
>> Even though this probably works almost everywhere, it is undefined
>> behavior, and it could plausibly cause highly-optimizing compilers to
>> misbehave.
>> ...
>> It would be a pain to have to change the signature of this macro, and
>> we'd prefer not to add overhead to each iteration of the loop. So
>> instead, whenever `list->items` is NULL, initialize `item` to point at
>> a dummy `string_list_item` created for the purpose.
>> ...
>> -#define for_each_string_list_item(item,list) \
>> -	for (item = (list)->items; item < (list)->items + (list)->nr; ++item)
>> +extern struct string_list_item dummy_string_list_item;
>> +#define for_each_string_list_item(item,list)                                 \
>> +	for (item = (list)->items ? (list)->items : &dummy_string_list_item; \
>> +	     item < (list)->items + (list)->nr;                              \
>> +	     ++item)
> 
> Sorry, but I am confused.
> 
> So when (list)->items is NULL, the loop termination condition that
> used to be
> 
> 	NULL < NULL + 0
> 
> that was problematic because NULL + 0 is problematic now becomes
> 
> 	&dummy < NULL + 0
> 
> in the new code?  What made NULL + 0 not problematic now?

*sigh* of course you're right. I should know better than to "fire off a
quick fix to the mailing list".

I guess the two proposals that are still in the running for rescuing
this macro are Jonathan's and Gábor's. I have no strong preference
either way.

Michael