Re: [PATCH 18/21] t/helper: fix segfault in "oid-array" command without repository

From: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Date: Jun 11, 2024, 13:23 UTC
Message-ID: <cc2go3x24cftqml2byx5oh3ggvsqdz32yufdpn2box2jpehtlo@hnsubup3sefn>
In-Reply-To: <2774b8500fdecb0d73543dbfc5ac18c6bb8d8090.1718106285.git.ps@pks.im>

On Tue, 11 Jun 2024, Patrick Steinhardt <ps@pks.im> wrote:

Show 8 quoted lines

> The "oid-array" test helper can supposedly work without a Git
> repository, but will in fact crash because `the_repository->hash_algo`
> is not initialized. This is because `oid_pos()`, which is used by
> `oid_array_lookup()`, depends on `the_hash_algo->rawsz`.
> 
> Ideally, we'd adapt `oid_pos()` to not depend on `the_hash_algo`
> anymore. That is a bigger untertaking though, so instead we fall back to
> SHA1 when there is no repository.

Thanks for working on this. This would make t-oid-array migration smoother.

Show 22 quoted lines

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  t/helper/test-oid-array.c |  4 ++++
>  t/t0064-oid-array.sh      | 18 ++++++++++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/t/helper/test-oid-array.c b/t/helper/test-oid-array.c
> index aafe398ef0..076b849cbf 100644
> --- a/t/helper/test-oid-array.c
> +++ b/t/helper/test-oid-array.c
> @@ -1,3 +1,5 @@
> +#define USE_THE_REPOSITORY_VARIABLE
> +
>  #include "test-tool.h"
>  #include "hex.h"
>  #include "oid-array.h"
> @@ -17,6 +19,8 @@ int cmd__oid_array(int argc UNUSED, const char **argv UNUSED)
>  	int nongit_ok;
>  
>  	setup_git_directory_gently(&nongit_ok);
> +	if (nongit_ok)
> +		repo_set_hash_algo(the_repository, GIT_HASH_SHA1);

Ideally we would want to test with both SHA1 and SHA256 without the repo, but it is not a necessity for this patch as it would get solved when we migrate to the unit testing framework.

Show 34 quoted lines

>  	while (strbuf_getline(&line, stdin) != EOF) {
>  		const char *arg;
> diff --git a/t/t0064-oid-array.sh b/t/t0064-oid-array.sh
> index 88c89e8f48..de74b692d0 100755
> --- a/t/t0064-oid-array.sh
> +++ b/t/t0064-oid-array.sh
> @@ -15,6 +15,24 @@ echoid () {
>  	done
>  }
>  
> +test_expect_success 'without repository' '
> +	cat >expect <<-EOF &&
> +	4444444444444444444444444444444444444444
> +	5555555555555555555555555555555555555555
> +	8888888888888888888888888888888888888888
> +	aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
> +	EOF
> +	cat >input <<-EOF &&
> +	append 4444444444444444444444444444444444444444
> +	append 5555555555555555555555555555555555555555
> +	append 8888888888888888888888888888888888888888
> +	append aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
> +	for_each_unique
> +	EOF
> +	nongit test-tool oid-array <input >actual &&
> +	test_cmp expect actual
> +'
> +
>  test_expect_success 'ordered enumeration' '
>  	echoid "" 44 55 88 aa >expect &&
>  	{
> -- 
> 2.45.2.436.gcd77e87115.dirty
>