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

[Taylor Blau <me@ttaylorr.com>]

On Tue, Apr 15, 2025 at 05:58:23PM -0700, Junio C Hamano wrote:
> 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 ...
>
> > 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.

Yeah, this is cruft that I thought I had expunged while rebasing. Here's
a better version of the patch, but I'm happy to send a new round of the
series if it would be more convenient for you:

--- 8< ---

Subject: [PATCH] pack-objects: limit scope in 'add_object_entry_from_pack()'

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.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 builtin/pack-objects.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 20dd870bbf..682e80be40 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3490,7 +3490,6 @@ 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;
@@ -3498,6 +3497,7 @@ static int add_object_entry_from_pack(const struct object_id *oid,
 			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
--
2.49.0.230.ga662d77f78

Thanks,
Taylor