| Message ID | 20241026120950.72727-1-abhijeet.nkt@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] show-index: fix uninitialized hash function | expand |
On Sat, Oct 26, 2024 at 05:39:50PM +0530, Abhijeet Sonar wrote: > As stated in the docs, show-index should use SHA1 as the default hash algorithm > when run outsize of a repository. However, 'the_hash_algo' is currently left > uninitialized if we are not in a repository and no explicit hash function is > specified, causing a crash. Fix it by falling back to SHA1 when it is found > uninitialized. Also add test that verifies this behaviour. This commit description is good, and would benefit further from a bisection showing where the regression began. I don't think that it is a prerequisite for us moving this patch forward, though. > Signed-off-by: Abhijeet Sonar <abhijeet.nkt@gmail.com> > --- > builtin/show-index.c | 3 +++ > t/t5300-pack-object.sh | 4 ++++ > 2 files changed, 7 insertions(+) > > diff --git a/builtin/show-index.c b/builtin/show-index.c > index f164c01bbe..978ae70470 100644 > --- a/builtin/show-index.c > +++ b/builtin/show-index.c > @@ -38,6 +38,9 @@ int cmd_show_index(int argc, > repo_set_hash_algo(the_repository, hash_algo); > } > > + if (!the_hash_algo) > + repo_set_hash_algo(the_repository, GIT_HASH_SHA1); > + > hashsz = the_hash_algo->rawsz; > > if (fread(top_index, 2 * 4, 1, stdin) != 1) > diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh > index 3b9dae331a..51fed26cc4 100755 > --- a/t/t5300-pack-object.sh > +++ b/t/t5300-pack-object.sh > @@ -523,6 +523,10 @@ test_expect_success 'index-pack --strict <pack> works in non-repo' ' > test_path_is_file foo.idx > ' > > +test_expect_success SHA1 'show-index works OK outside a repository' ' > + nongit git show-index <foo.idx > +' > + > test_expect_success !PTHREADS,!FAIL_PREREQS \ > 'index-pack --threads=N or pack.threads=N warns when no pthreads' ' > test_must_fail git index-pack --threads=2 2>err && > -- > 2.47.0.107.g34b6ce9b30 These all look reasonable and as-expected to me. Patrick (CC'd) has been reviewing similar changes elsewhere, so I'd like him to chime in as well on whether or not this looks good to go. Thanks, Taylor
diff --git a/builtin/show-index.c b/builtin/show-index.c index f164c01bbe..978ae70470 100644 --- a/builtin/show-index.c +++ b/builtin/show-index.c @@ -38,6 +38,9 @@ int cmd_show_index(int argc, repo_set_hash_algo(the_repository, hash_algo); } + if (!the_hash_algo) + repo_set_hash_algo(the_repository, GIT_HASH_SHA1); + hashsz = the_hash_algo->rawsz; if (fread(top_index, 2 * 4, 1, stdin) != 1) diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh index 3b9dae331a..51fed26cc4 100755 --- a/t/t5300-pack-object.sh +++ b/t/t5300-pack-object.sh @@ -523,6 +523,10 @@ test_expect_success 'index-pack --strict <pack> works in non-repo' ' test_path_is_file foo.idx ' +test_expect_success SHA1 'show-index works OK outside a repository' ' + nongit git show-index <foo.idx +' + test_expect_success !PTHREADS,!FAIL_PREREQS \ 'index-pack --threads=N or pack.threads=N warns when no pthreads' ' test_must_fail git index-pack --threads=2 2>err &&
As stated in the docs, show-index should use SHA1 as the default hash algorithm when run outsize of a repository. However, 'the_hash_algo' is currently left uninitialized if we are not in a repository and no explicit hash function is specified, causing a crash. Fix it by falling back to SHA1 when it is found uninitialized. Also add test that verifies this behaviour. Signed-off-by: Abhijeet Sonar <abhijeet.nkt@gmail.com> --- builtin/show-index.c | 3 +++ t/t5300-pack-object.sh | 4 ++++ 2 files changed, 7 insertions(+)