diff options
author | Ævar Arnfjörð Bjarmason <avarab@gmail.com> | 2021-07-09 12:13:48 +0200 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2021-07-09 11:53:40 -0700 |
commit | 561fa03529202a36e0d77548fdcb5d81422c1865 (patch) | |
tree | f3dd9c9b67154d4cb45fe46bb780ea7528e62632 /t/t5300-pack-object.sh | |
parent | fb20d4b1268d97646ae24f07661892cf6da64c31 (diff) | |
download | git-561fa03529202a36e0d77548fdcb5d81422c1865.tar.gz |
pack-objects: fix segfault in --stdin-packs option
Fix a segfault in the --stdin-packs option added in
339bce27f4f (builtin/pack-objects.c: add '--stdin-packs' option,
2021-02-22).
The read_packs_list_from_stdin() function didn't check that the lines
it was reading were valid packs, and thus when doing the QSORT() with
pack_mtime_cmp() we'd have a NULL "util" field. The "util" field is
used to associate the names of included/excluded packs with the
packed_git structs they correspond to.
The logic error was in assuming that we could iterate all packs and
annotate the excluded and included packs we got, as opposed to
checking the lines we got on stdin. There was a check for excluded
packs, but included packs were simply assumed to be valid.
As noted in the test we'll not report the first bad line, but whatever
line sorted first according to the string-list.c API. In this case I
think that's fine. There was further discussion of alternate
approaches in [1].
Even though we're being lazy let's assert the line we do expect to get
in the test, since whoever changes this code in the future might miss
this case, and would want to update the test and comments.
1. http://lore.kernel.org/git/YND3h2l10PlnSNGJ@nand.local
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 't/t5300-pack-object.sh')
-rwxr-xr-x | t/t5300-pack-object.sh | 19 |
1 files changed, 19 insertions, 0 deletions
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh index 65e991e370..e13a884207 100755 --- a/t/t5300-pack-object.sh +++ b/t/t5300-pack-object.sh @@ -119,6 +119,25 @@ test_expect_success 'pack-object <stdin parsing: [|--revs] with --stdin' ' test_cmp err.expect err.actual ' +test_expect_success 'pack-object <stdin parsing: --stdin-packs handles garbage' ' + cat >in <<-EOF && + $(git -C pack-object-stdin rev-parse one) + $(git -C pack-object-stdin rev-parse two) + EOF + + # That we get "two" and not "one" has to do with OID + # ordering. It happens to be the same here under SHA-1 and + # SHA-256. See commentary in pack-objects.c + cat >err.expect <<-EOF && + fatal: could not find pack '"'"'$(git -C pack-object-stdin rev-parse two)'"'"' + EOF + test_must_fail git \ + -C pack-object-stdin \ + pack-objects stdin-with-stdin-option --stdin-packs \ + <in 2>err.actual && + test_cmp err.expect err.actual +' + # usage: check_deltas <stderr_from_pack_objects> <cmp_op> <nr_deltas> # e.g.: check_deltas stderr -gt 0 check_deltas() { |