Re: [PATCH 05/19] reftable: improve const correctness when assigning string constants
From: Patrick Steinhardt <hidden>
Date: 2024-05-30 11:30:33
Attachments
- signature.asc [application/pgp-signature] 833 bytes
From: Patrick Steinhardt <hidden>
Date: 2024-05-30 11:30:33
On Wed, May 29, 2024 at 10:43:47AM -0700, Junio C Hamano wrote:
Patrick Steinhardt [off-list ref] writes:quoted
diff --git a/reftable/basics_test.c b/reftable/basics_test.c index 997c4d9e01..af9209d535 100644 --- a/reftable/basics_test.c +++ b/reftable/basics_test.c@@ -58,8 +58,8 @@ static void test_binsearch(void) static void test_names_length(void) { - char *a[] = { "a", "b", NULL }; - EXPECT(names_length(a) == 2); + char *names[] = { (char *)"a", (char *)"b", NULL }; + EXPECT(names_length(names) == 2); }I would have preferred to see this kind of rewrite more than separate and clearly writable variables that are initialied with the constant contents e.g. branches[] = "refs/heads/*", we saw in earlier steps. Wouldn't that approach, combined with making the literal constants stored in read-only segment to trigger runtime failure when a bug causes the "unfortunately non-const" variables to be written, give us a better result?
Depends on what we mean by "better", I guess. But yeah, I was torn myself when writing this commit because there are so many string constants in the reftable tests that we assign to non-constant fields. I didn't find the result particularly easy to read when putting each of the constants into a separate variable. Revisiting this again though I don't think it's all that bad. I'll adapt accordingly. Patrick