for_each_string_list_item: avoid undefined behavior for empty list
authorMichael Haggerty <mhagger@alum.mit.edu>
Wed, 20 Sep 2017 05:27:05 +0000 (22:27 -0700)
committerJunio C Hamano <gitster@pobox.com>
Wed, 20 Sep 2017 05:41:08 +0000 (14:41 +0900)
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]

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
No differences found