Re: [PATCH v2 2/4] builtin/pack-objects.c: avoid redundant NULL check

From: Junio C Hamano <gitster@pobox.com>
Date: May 24, 2022, 21:44 UTC
Message-ID: <xmqq8rqqfvwa.fsf@gitster.g>
In-Reply-To: <2719d33f328e03239cdb2f5cca2fef9a4e9cbb93.1653418457.git.me@ttaylorr.com>

Taylor Blau <me@ttaylorr.com> writes:

Show 10 quoted lines

> Before calling `for_each_object_in_pack()`, the caller
> `read_packs_list_from_stdin()` loops through each of the `include_packs`
> and checks that its `->util` pointer (which is used to store the `struct
> packed_git *` itself) is non-NULL.
>
> This check is redundant, because `read_packs_list_from_stdin()` already
> checks that the included packs are non-NULL earlier on in the same
> function (and it does not add any new entries in between).
>
> Remove this check, since it is not doing anything in the meantime.

Will it start doing something soon in a later step?

Oh, did you mean that after the earlier for_each_string_list_item() iteration over include_packs that died on an item with a NULL .util member, the code did not do anything to cause this second iteration over the same list to suddenly start seeing an item with NULL .util?

I am puzzled by the mention of "in the meantime".

The patch itself looks correct, of course.

Thanks.

Show 19 quoted lines

> Co-authored-by: Victoria Dye <vdye@github.com>
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
>  builtin/pack-objects.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index 014dcd4bc9..ec3193fd95 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -3369,8 +3369,6 @@ static void read_packs_list_from_stdin(void)
>  
>  	for_each_string_list_item(item, &include_packs) {
>  		struct packed_git *p = item->util;
> -		if (!p)
> -			die(_("could not find pack '%s'"), item->string);
>  		for_each_object_in_pack(p,
>  					add_object_entry_from_pack,
>  					&revs,