On Mon, Jun 13 2022, Derrick Stolee wrote:

On 6/9/2022 7:44 PM, Junio C Hamano wrote:

quoted

+	struct string_list *resolve_undo = istate->resolve_undo;
+
+	if (!resolve_undo)
+		return 0;
+
+	for_each_string_list_item(item, resolve_undo) {

I see this is necessary since for_each_string_list_item() does
not handle NULL lists. After attempting to allow it to handle
NULL lists, I see that the compiler complains about the cases
where it would _never_ be NULL, so that change appears to be
impossible.
 
The patch looks good. I liked the comments for the three phases
of the test.

I think it's probably good to keep for_each_string_list_item()
implemented the way it is, given that all existing callers of it feed
non-NULL lists to it.

But why is it impossible to make it handle NULL lists? This works for
me, and passes the tests:
	
	diff --git a/builtin/fsck.c b/builtin/fsck.c
	index 4b17ccc3f44..4160bb50ad0 100644
	--- a/builtin/fsck.c
	+++ b/builtin/fsck.c
	@@ -763,9 +763,6 @@ static int fsck_resolve_undo(struct index_state *istate)
	 	struct string_list_item *item;
	 	struct string_list *resolve_undo = istate->resolve_undo;
	 
	-	if (!resolve_undo)
	-		return 0;
	-
	 	for_each_string_list_item(item, resolve_undo) {
	 		const char *path = item->string;
	 		struct resolve_undo_info *ru = item->util;
	diff --git a/string-list.h b/string-list.h
	index d5a744e1438..aa15aea8c2b 100644
	--- a/string-list.h
	+++ b/string-list.h
	@@ -143,7 +143,7 @@ int for_each_string_list(struct string_list *list,
	 
	 /** Iterate over each item, as a macro. */
	 #define for_each_string_list_item(item,list)            \
	-	for (item = (list)->items;                      \
	+	for (item = (((list) && (list)->items) ? ((list)->items) : NULL); \
	 	     item && item < (list)->items + (list)->nr; \
	 	     ++item)
	 

Perhaps you tried to convert it to a do/while macro with an "if", which
won't work as we need the "for" to be used for the subsequent {} (or a
single statement)..

But under the hood this is all just syntax sugar for "goto", so if we
can avoid dereferencing "list" we're good to go...