threads / patch / 57237

patchreftable: avoid initializing structs from structs

Subject: [PATCH] reftable: avoid initializing structs from structs

## tl;dr

7 messages between Jan 13, 2022 and Jan 17, 2022. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Han-Wen Nienhuys via GitGitGadget· Jan 13, 2022, 16:55 UTC · lore
From: Han-Wen Nienhuys <hanwen@google.com>
Apparently, the IBM xlc compiler doesn't like this.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
    reftable: avoid initializing structs from structs
    
    Apparently, the IBM xlc compiler doesn't like this.
    
    Signed-off-by: Han-Wen Nienhuys hanwen@google.com
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1188%2Fhanwen%2Freftable-xlc-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1188/hanwen/reftable-xlc-v1
Pull-Request: https://github.com/git/git/pull/1188
 reftable/merged_test.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)
Show changes to reftable/merged_test.c +11 −11
diff --git a/reftable/merged_test.c b/reftable/merged_test.c
index 24461e8a802..abd34849fca 100644
--- a/reftable/merged_test.c
+++ b/reftable/merged_test.c
@@ -207,11 +207,11 @@ static void test_merged(void)
 		},
 	};
 
-	struct reftable_ref_record want[] = {
-		r2[0],
-		r1[1],
-		r3[0],
-		r3[1],
+	struct reftable_ref_record *want[] = {
+		&r2[0],
+		&r1[1],
+		&r3[0],
+		&r3[1],
 	};
 
 	struct reftable_ref_record *refs[] = { r1, r2, r3 };
@@ -250,7 +250,7 @@ static void test_merged(void)
 
 	EXPECT(ARRAY_SIZE(want) == len);
 	for (i = 0; i < len; i++) {
-		EXPECT(reftable_ref_record_equal(&want[i], &out[i],
+		EXPECT(reftable_ref_record_equal(want[i], &out[i],
 						 GIT_SHA1_RAWSZ));
 	}
 	for (i = 0; i < len; i++) {
@@ -345,10 +345,10 @@ static void test_merged_logs(void)
 			.value_type = REFTABLE_LOG_DELETION,
 		},
 	};
-	struct reftable_log_record want[] = {
-		r2[0],
-		r3[0],
-		r1[1],
+	struct reftable_log_record *want[] = {
+		&r2[0],
+		&r3[0],
+		&r1[1],
 	};
 
 	struct reftable_log_record *logs[] = { r1, r2, r3 };
@@ -387,7 +387,7 @@ static void test_merged_logs(void)
 
 	EXPECT(ARRAY_SIZE(want) == len);
 	for (i = 0; i < len; i++) {
-		EXPECT(reftable_log_record_equal(&want[i], &out[i],
+		EXPECT(reftable_log_record_equal(want[i], &out[i],
 						 GIT_SHA1_RAWSZ));
 	}
 

base-commit: 1ffcbaa1a5f10c9f706314d77f88de20a4a498c2
-- 
gitgitgadget
Ævar Arnfjörð Bjarmason· Jan 13, 2022, 17:13 UTC · re: Han-Wen Nienhuys via GitGitGadget · lore

Re: [PATCH] reftable: avoid initializing structs from structs

On Thu, Jan 13 2022, Han-Wen Nienhuys via GitGitGadget wrote:
> From: Han-Wen Nienhuys <hanwen@google.com>

Ah, nevermind <220113.86v8yntxfb.gmgdl@evledraar.gmail.com>, so you meant *want[] :)

I can confirm that this works on the xlc version that errored on this before, the reftable tests even pass!

> Apparently, the IBM xlc compiler doesn't like this.

Would make sense to steal the compiler version etc. details from my <patch-1.1-7425b64c0a0-20220113T113821Z-avarab@gmail.com>. I.e. eventually we'll be able to change this & other code back, as nobody will care about that older compiler version. It worked before in the pre-image on a more recent xlc.

Show 71 quoted lines
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
>     reftable: avoid initializing structs from structs
>     
>     Apparently, the IBM xlc compiler doesn't like this.
>     
>     Signed-off-by: Han-Wen Nienhuys hanwen@google.com
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1188%2Fhanwen%2Freftable-xlc-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1188/hanwen/reftable-xlc-v1
> Pull-Request: https://github.com/git/git/pull/1188
>
>  reftable/merged_test.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/reftable/merged_test.c b/reftable/merged_test.c
> index 24461e8a802..abd34849fca 100644
> --- a/reftable/merged_test.c
> +++ b/reftable/merged_test.c
> @@ -207,11 +207,11 @@ static void test_merged(void)
>  		},
>  	};
>  
> -	struct reftable_ref_record want[] = {
> -		r2[0],
> -		r1[1],
> -		r3[0],
> -		r3[1],
> +	struct reftable_ref_record *want[] = {
> +		&r2[0],
> +		&r1[1],
> +		&r3[0],
> +		&r3[1],
>  	};
>  
>  	struct reftable_ref_record *refs[] = { r1, r2, r3 };
> @@ -250,7 +250,7 @@ static void test_merged(void)
>  
>  	EXPECT(ARRAY_SIZE(want) == len);
>  	for (i = 0; i < len; i++) {
> -		EXPECT(reftable_ref_record_equal(&want[i], &out[i],
> +		EXPECT(reftable_ref_record_equal(want[i], &out[i],
>  						 GIT_SHA1_RAWSZ));
>  	}
>  	for (i = 0; i < len; i++) {
> @@ -345,10 +345,10 @@ static void test_merged_logs(void)
>  			.value_type = REFTABLE_LOG_DELETION,
>  		},
>  	};
> -	struct reftable_log_record want[] = {
> -		r2[0],
> -		r3[0],
> -		r1[1],
> +	struct reftable_log_record *want[] = {
> +		&r2[0],
> +		&r3[0],
> +		&r1[1],
>  	};
>  
>  	struct reftable_log_record *logs[] = { r1, r2, r3 };
> @@ -387,7 +387,7 @@ static void test_merged_logs(void)
>  
>  	EXPECT(ARRAY_SIZE(want) == len);
>  	for (i = 0; i < len; i++) {
> -		EXPECT(reftable_log_record_equal(&want[i], &out[i],
> +		EXPECT(reftable_log_record_equal(want[i], &out[i],
>  						 GIT_SHA1_RAWSZ));
>  	}
>  
>
> base-commit: 1ffcbaa1a5f10c9f706314d77f88de20a4a498c2
Han-Wen Nienhuys· Jan 13, 2022, 17:40 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH] reftable: avoid initializing structs from structs

On Thu, Jan 13, 2022 at 6:14 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 10 quoted lines
> I can confirm that this works on the xlc version that errored on this
> before, the reftable tests even pass!
>
> > Apparently, the IBM xlc compiler doesn't like this.
>
> Would make sense to steal the compiler version etc. details from my
> <patch-1.1-7425b64c0a0-20220113T113821Z-avarab@gmail.com>. I.e. eventually
> we'll be able to change this & other code back, as nobody will care
> about that older compiler version. It worked before in the pre-image on
> a more recent xlc.
Feel free to butcher this in any way you like for your series. :)
-- 
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--

Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich

Registergericht und -nummer: Hamburg, HRB 86891

Sitz der Gesellschaft: Hamburg

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Junio C Hamano· Jan 13, 2022, 19:15 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH] reftable: avoid initializing structs from structs

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 5 quoted lines
> Would make sense to steal the compiler version etc. details from my
> <patch-1.1-7425b64c0a0-20220113T113821Z-avarab@gmail.com>. I.e. eventually
> we'll be able to change this & other code back, as nobody will care
> about that older compiler version. It worked before in the pre-image on
> a more recent xlc.

If so, wouldn't it be a better option not to worry about such an old compiler at all from the get-go? Even with an unnecessary "turn an array of structs into an array of pointers to structs", the resulting code becomes less natural to follow. And after all, this may be part of our tree but is not yet integrated with our system, no?

Thanks.
Junio C Hamano· Jan 13, 2022, 20:00 UTC · re: Junio C Hamano · lore

Re: [PATCH] reftable: avoid initializing structs from structs

Junio C Hamano <gitster@pobox.com> writes:
Show 10 quoted lines
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> Would make sense to steal the compiler version etc. details from my
>> <patch-1.1-7425b64c0a0-20220113T113821Z-avarab@gmail.com>. I.e. eventually
>> we'll be able to change this & other code back, as nobody will care
>> about that older compiler version. It worked before in the pre-image on
>> a more recent xlc.
>
> If so, wouldn't it be a better option not to worry about such an old
> compiler at all from the get-go?

The above was a genuine question. If that "nobody will care about the old compiler" will happen only after a few years, then it may not work to just ignore the version of xlc which might still have a meaningful number of users. I just am not in a good position to judge that.

Thanks.
Han-Wen Nienhuys· Jan 17, 2022, 13:07 UTC · re: Junio C Hamano · lore

Re: [PATCH] reftable: avoid initializing structs from structs

On Thu, Jan 13, 2022 at 9:00 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 14 quoted lines
> >> Would make sense to steal the compiler version etc. details from my
> >> <patch-1.1-7425b64c0a0-20220113T113821Z-avarab@gmail.com>. I.e. eventually
> >> we'll be able to change this & other code back, as nobody will care
> >> about that older compiler version. It worked before in the pre-image on
> >> a more recent xlc.
> >
> > If so, wouldn't it be a better option not to worry about such an old
> > compiler at all from the get-go?
>
> The above was a genuine question.  If that "nobody will care about
> the old compiler" will happen only after a few years, then it may
> not work to just ignore the version of xlc which might still have
> a meaningful number of users.  I just am not in a good position to
> judge that.

I'm all for not worrying too much about ancient compilers, but there is no downside to this patch, so it seems fine to let this one go through.

-- 
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--

Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich

Registergericht und -nummer: Hamburg, HRB 86891

Sitz der Gesellschaft: Hamburg

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Junio C Hamano· Jan 17, 2022, 19:08 UTC · re: Han-Wen Nienhuys · lore

Re: [PATCH] reftable: avoid initializing structs from structs

Han-Wen Nienhuys <hanwen@google.com> writes:
Show 19 quoted lines
> On Thu, Jan 13, 2022 at 9:00 PM Junio C Hamano <gitster@pobox.com> wrote:
>> >> Would make sense to steal the compiler version etc. details from my
>> >> <patch-1.1-7425b64c0a0-20220113T113821Z-avarab@gmail.com>. I.e. eventually
>> >> we'll be able to change this & other code back, as nobody will care
>> >> about that older compiler version. It worked before in the pre-image on
>> >> a more recent xlc.
>> >
>> > If so, wouldn't it be a better option not to worry about such an old
>> > compiler at all from the get-go?
>>
>> The above was a genuine question.  If that "nobody will care about
>> the old compiler" will happen only after a few years, then it may
>> not work to just ignore the version of xlc which might still have
>> a meaningful number of users.  I just am not in a good position to
>> judge that.
>
> I'm all for not worrying too much about ancient compilers, but there
> is no downside to this patch, so it seems fine to let this one go
> through.
Yup, I think this already is part of -rc1.
Thanks.

← back to recent threads