Re: [PATCH v3 2/9] pack-objects: limit scope in 'add_object_entry_from_pack()'

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 16, 2025, 00:58 UTC
Message-ID: <xmqqtt6okjm8.fsf@gitster.g>
In-Reply-To: <986bef29b5f33d32fd366aa9370d439175a9b605.1744757204.git.me@ttaylorr.com>

Taylor Blau <me@ttaylorr.com> writes:

> In add_object_entry_from_pack() we declare 'revs' (given to us through
> the miscellaneous context argument) earlier in the "if (p)" conditional
> than is necessary.  Move it down as far as it can go to reduce its
> scope.

That makes sense, but ...

Show 18 quoted lines

> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
>  builtin/pack-objects.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index 20dd870bbf..4ab695a3aa 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -3490,14 +3490,14 @@ static int add_object_entry_from_pack(const struct object_id *oid,
>  		return 0;
>  
>  	if (p) {
> -		struct rev_info *revs = _data;
>  		struct object_info oi = OBJECT_INFO_INIT;
> -
>  		oi.typep = &type;
> +

Isn't this change about spacing around oi's decl and the first statement in the block strictly worsening the code? At least it is an unrelated change.

Show 8 quoted lines

>  		if (packed_object_info(the_repository, p, ofs, &oi) < 0) {
>  			die(_("could not get type of object %s in pack %s"),
>  			    oid_to_hex(oid), p->pack_name);
>  		} else if (type == OBJ_COMMIT) {
> +			struct rev_info *revs = _data;
>  			/*
>  			 * commits in included packs are used as starting points for the
>  			 * subsequent revision walk