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)
commitac7da78ede468315ba22d9ee1265b61a744371ee
treef1ed610d69399db07f0af1ce4337a8bb89a6cc74
parent94c9fd268d4287f6fbfef84793288479905a7e48
for_each_string_list_item: avoid undefined behavior for empty list

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>
string-list.h