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>

diff --git a/string-list.h b/string-list.h
index 29bfb7ae45931686e14c91b1a47a703e7b32fe52..79ae567cbc3bfbd7574b8156aa9fcf56581cad83 100644 (file)
--- a/string-list.h
+++ b/string-list.h
@@ -32,8 +32,10 @@ void string_list_clear_func(struct string_list *list, string_list_clear_func_t c
 typedef int (*string_list_each_func_t)(struct string_list_item *, void *);
 int for_each_string_list(struct string_list *list,
                         string_list_each_func_t, void *cb_data);
-#define for_each_string_list_item(item,list) \
-       for (item = (list)->items; item < (list)->items + (list)->nr; ++item)
+#define for_each_string_list_item(item,list)            \
+       for (item = (list)->items;                      \
+            item && item < (list)->items + (list)->nr; \
+            ++item)
 
 /*
  * Apply want to each item in list, retaining only the ones for which