| Message ID | 986bef29b5f33d32fd366aa9370d439175a9b605.1744757204.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | repack: avoid MIDX'ing cruft pack(s) where possible | expand |
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. > 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
On Tue, Apr 15, 2025 at 3:46 PM Taylor Blau <me@ttaylorr.com> wrote: > > 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 | 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; This change is half of what you mention in your commit message. > struct object_info oi = OBJECT_INFO_INIT; > - > oi.typep = &type; > + This is an unrelated, distracting change that I think was accidental and came from trying to back out the other change that was part of v1/v2 but not quite backing it out completely. > 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; This is the other half of what you mention in the commit message. > /* > * commits in included packs are used as starting points for the > * subsequent revision walk > -- > 2.49.0.230.ga662d77f78
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; + 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
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 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)