Re: [PATCH] revision: mark blobs needed for resolve-undo as reachable
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Jun 14, 2022, 00:24 UTC
- Message-ID
- <220614.86czfcytlz.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <7cd41846-e6ef-7a24-0426-6031a529360f@github.com>
On Mon, Jun 13 2022, Derrick Stolee wrote:
Show 17 quoted lines
> On 6/9/2022 7:44 PM, Junio C Hamano wrote:
>
>> + 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...