On 09/20/2017 07:27 AM, Jonathan Nieder wrote: > From: Michael Haggerty <mhag...@alum.mit.edu> > > 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; /* NULL */ > item < (list)->items + (list)->nr; /* NULL + 0 */ > ++item) > > Even though this probably works almost everywhere, it is undefined > behavior, and it could plausibly cause highly-optimizing compilers to > misbehave. C99 section 6.5.6 paragraph 8 explains: > > If both the pointer operand and the result point to elements > of the same array object, or one past the last element of the > array object, the evaluation shall not produce an overflow; > otherwise, the behavior is undefined. > > and (6.3.2.3.3) a null pointer does not point to anything. > > Guard the loop with a NULL check to make the intent crystal clear to > even the most pedantic compiler. A suitably clever compiler could let > the NULL check only run in the first iteration, but regardless, this > overhead is likely to be dwarfed by the work to be done on each item. > > This problem was noticed by Coverity. > > [jn: using a NULL check instead of a placeholder empty list; > fleshed out the commit message based on mailing list discussion]
Thanks for taking this over. This version LGTM. > [...] Michael
