Subject: Re: [PATCH] revision: mark blobs needed for resolve-undo as reachable

From: Junio C Hamano <[email-masked]>
Date: Mon, 13 Jun 2022 13:11:16 -0700

Derrick Stolee <[email-masked]> writes:

> 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.

Heh, no such deep thought went into this.  I just copied what is
done in builtin/ls-files.c::show_ru_info() that grabs these blobs
and does something interesting and the only difference is this does
something else interesting.

We _could_ refactor these to into "take a callback and iterate over
resolve-undo information" shell plus two callback functions, one to
print them and the other to mark them still reachable, but the
iterator being relatively short, I doubt that it is worth it.

> The patch looks good. I liked the comments for the three phases
> of the test.

Thanks.