| Message ID | 2719d33f328e03239cdb2f5cca2fef9a4e9cbb93.1653418457.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 58a6abb7bae98357677657825631de6652256be9 |
| Headers | show |
| Series | pack-objects: fix a pair of MIDX bitmap-related races | expand |
Taylor Blau <me@ttaylorr.com> writes: > 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. > 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,
On Tue, May 24, 2022 at 02:44:37PM -0700, Junio C Hamano wrote: > Taylor Blau <me@ttaylorr.com> writes: > > > 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 latter, sorry for the confusion. > The patch itself looks correct, of course. Thanks. Taylor
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,
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. 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(-)