{"thread":{"id":"62506","subject":"-Wunterminated-string-initialization warning with GCC 15 in object-file.c","startedAt":"2024-11-17T02:50:43Z","lastAt":"2024-11-18T12:50:00Z","messageCount":25,"participants":["Sam James","Jeff King","René Scharfe","brian m. carlson","Patrick Steinhardt","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"507426","messageId":"87wmh2o9og.fsf@gentoo.org","threadId":"62506","inReplyTo":null,"subject":"-Wunterminated-string-initialization warning with GCC 15 in object-file.c","fromName":"Sam James","fromEmail":"sam@gentoo.org","sentAt":"2024-11-17T02:50:39Z","receivedAt":"2024-11-17T02:50:43Z","isPatch":false,"sender":{"key":"sam@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/11667869?v=4"},"body":"With upcoming GCC 15, a new warning is added\n(-Wunterminated-string-initialization) that fires when building git:\n```\n CC object-file.o\nobject-file.c:52:9: warning: initializer-string for array of ‘unsigned char’ is too long [-Wunterminated-string-initialization]\n 52 | \"\\x6e\\xf1\\x9b\\x41\\x22\\x5c\\x53\\x69\\xf1\\xc1\" \\\n | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\nobject-file.c:79:17: note: in expansion of macro ‘EMPTY_TREE_SHA256_BIN_LITERAL’\n 79 | .hash = EMPTY_TREE_SHA256_BIN_LITERAL,\n | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~\nobject-file.c:61:9: warning: initializer-string for array of ‘unsigned char’ is too long [-Wunterminated-string-initialization]\n 61 | \"\\x47\\x3a\\x0f\\x4c\\x3b\\xe8\\xa9\\x36\\x81\\xa2\" \\\n | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\nobject-file.c:83:17: note: in expansion of macro ‘EMPTY_BLOB_SHA256_BIN_LITERAL’\n 83 | .hash = EMPTY_BLOB_SHA256_BIN_LITERAL,\n | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n```\n\nContext for the new warning is at https://gcc.gnu.org/PR115185.\n\nthanks,\nsam\n"},{"id":"507429","messageId":"20241117090329.GA2341486@coredump.intra.peff.net","threadId":"62506","inReplyTo":"87wmh2o9og.fsf@gentoo.org","subject":"Re: -Wunterminated-string-initialization warning with GCC 15 in object-file.c","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-11-17T09:03:29Z","receivedAt":"2024-11-17T09:03:39Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 17, 2024 at 02:50:39AM +0000, Sam James wrote:\n\n> With upcoming GCC 15, a new warning is added\n> (-Wunterminated-string-initialization) that fires when building git:\n> ```\n> CC object-file.o\n> object-file.c:52:9: warning: initializer-string for array of ‘unsigned char’ is too long [-Wunterminated-string-initialization]\n> 52 | \"\\x6e\\xf1\\x9b\\x41\\x22\\x5c\\x53\\x69\\xf1\\xc1\" \\\n> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n> object-file.c:79:17: note: in expansion of macro ‘EMPTY_TREE_SHA256_BIN_LITERAL’\n> 79 | .hash = EMPTY_TREE_SHA256_BIN_LITERAL,\n> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n> object-file.c:61:9: warning: initializer-string for array of ‘unsigned char’ is too long [-Wunterminated-string-initialization]\n> 61 | \"\\x47\\x3a\\x0f\\x4c\\x3b\\xe8\\xa9\\x36\\x81\\xa2\" \\\n> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n> object-file.c:83:17: note: in expansion of macro ‘EMPTY_BLOB_SHA256_BIN_LITERAL’\n> 83 | .hash = EMPTY_BLOB_SHA256_BIN_LITERAL,\n> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n> ```\n> \n> Context for the new warning is at https://gcc.gnu.org/PR115185.\n\nI think the warning is a false positive for us, but I don't begrudge\nthem for adding it. It could definitely catch real problems.\n\nHere are some patches. The first one should fix the warning (but I don't\nhave gcc-15 handy to test!). Please let me know if it works for you (and\nthank you for reporting).\n\nThe others are cleanups and future-proofing I found in the same area.\nNot strictly required, but IMHO worth doing.\n\n+cc brian since I think this is a continuation of some hash-algo\ncleanups he did earlier, plus he piped up in the other gcc-15 thread. ;)\n\n [1/5]: object-file: prefer array-of-bytes initializer for hash literals\n [2/5]: object-file: drop confusing oid initializer of empty_tree struct\n [3/5]: object-file: move empty_tree struct into find_cached_object()\n [4/5]: object-file: drop oid field from find_cached_object() return value\n [5/5]: object-file: inline empty tree and blob literals\n\n object-file.c | 77 ++++++++++++++++++++++++---------------------------\n 1 file changed, 36 insertions(+), 41 deletions(-)\n\n"},{"id":"507430","messageId":"20241117090814.GA3409496@coredump.intra.peff.net","threadId":"62506","inReplyTo":"20241117090329.GA2341486@coredump.intra.peff.net","subject":"[PATCH 1/5] object-file: prefer array-of-bytes initializer for hash literals","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-11-17T09:08:14Z","receivedAt":"2024-11-17T09:08:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We hard-code a few well-known hash values for empty trees and blobs in\nboth sha1 and sha256 formats. We do so with string literals like this:\n\n #define EMPTY_TREE_SHA256_BIN_LITERAL \\\n \"\\x6e\\xf1\\x9b\\x41\\x22\\x5c\\x53\\x69\\xf1\\xc1\" \\\n \"\\x04\\xd4\\x5d\\x8d\\x85\\xef\\xa9\\xb0\\x57\\xb5\" \\\n \"\\x3b\\x14\\xb4\\xb9\\xb9\\x39\\xdd\\x74\\xde\\xcc\" \\\n \"\\x53\\x21\"\n\nand then use it to initialize the hash field of an object_id struct.\nThat hash field is exactly 32 bytes long (the size we need for sha256).\nBut the string literal above is actually 33 bytes long due to the NUL\nterminator. It's legal in C to initialize from a longer string literal;\nthe extra bytes are just ignored.\n\nHowever, the upcoming gcc 15 will start warning about this:\n\n CC object-file.o\n object-file.c:52:9: warning: initializer-string for array of ‘unsigned char’ is too long [-Wunterminated-string-initialization]\n 52 | \"\\x6e\\xf1\\x9b\\x41\\x22\\x5c\\x53\\x69\\xf1\\xc1\" \\\n | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n object-file.c:79:17: note: in expansion of macro ‘EMPTY_TREE_SHA256_BIN_LITERAL’\n\nwhich is understandable. Even though this is not a bug for us, since we\ndo not care about the NUL terminator (and are just using the literal as\na convenient format), it would be easy to accidentally create an array\nthat was mistakenly unterminated.\n\nWe can avoid this warning by switching the initializer to an actual\narray of unsigned values. That arguably demonstrates our intent more\nclearly anyway.\n\nReported-by: Sam James \nSigned-off-by: Jeff King \n---\nI actually didn't find exact wording in the standard for using a\nlonger literal. But C99 section 6.7.8 (Initialization), para 32 shows\nthis exact case as \"example 8\".\n\nYou can view the diff with \"--color-words --word-diff-regex=.\" to more\nclearly see that the values themselves weren't changed.\n\n object-file.c | 38 +++++++++++++++++++++-----------------\n 1 file changed, 21 insertions(+), 17 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex b1a3463852..25ba54594b 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -45,23 +45,27 @@\n #define MAX_HEADER_LEN 32\n \n \n-#define EMPTY_TREE_SHA1_BIN_LITERAL \\\n-\t \"\\x4b\\x82\\x5d\\xc6\\x42\\xcb\\x6e\\xb9\\xa0\\x60\" \\\n-\t \"\\xe5\\x4b\\xf8\\xd6\\x92\\x88\\xfb\\xee\\x49\\x04\"\n-#define EMPTY_TREE_SHA256_BIN_LITERAL \\\n-\t\"\\x6e\\xf1\\x9b\\x41\\x22\\x5c\\x53\\x69\\xf1\\xc1\" \\\n-\t\"\\x04\\xd4\\x5d\\x8d\\x85\\xef\\xa9\\xb0\\x57\\xb5\" \\\n-\t\"\\x3b\\x14\\xb4\\xb9\\xb9\\x39\\xdd\\x74\\xde\\xcc\" \\\n-\t\"\\x53\\x21\"\n-\n-#define EMPTY_BLOB_SHA1_BIN_LITERAL \\\n-\t\"\\xe6\\x9d\\xe2\\x9b\\xb2\\xd1\\xd6\\x43\\x4b\\x8b\" \\\n-\t\"\\x29\\xae\\x77\\x5a\\xd8\\xc2\\xe4\\x8c\\x53\\x91\"\n-#define EMPTY_BLOB_SHA256_BIN_LITERAL \\\n-\t\"\\x47\\x3a\\x0f\\x4c\\x3b\\xe8\\xa9\\x36\\x81\\xa2\" \\\n-\t\"\\x67\\xe3\\xb1\\xe9\\xa7\\xdc\\xda\\x11\\x85\\x43\" \\\n-\t\"\\x6f\\xe1\\x41\\xf7\\x74\\x91\\x20\\xa3\\x03\\x72\" \\\n-\t\"\\x18\\x13\"\n+#define EMPTY_TREE_SHA1_BIN_LITERAL { \\\n+\t 0x4b, 0x82, 0x5d, 0xc6, 0x42, 0xcb, 0x6e, 0xb9, 0xa0, 0x60, \\\n+\t 0xe5, 0x4b, 0xf8, 0xd6, 0x92, 0x88, 0xfb, 0xee, 0x49, 0x04 \\\n+}\n+#define EMPTY_TREE_SHA256_BIN_LITERAL { \\\n+\t0x6e, 0xf1, 0x9b, 0x41, 0x22, 0x5c, 0x53, 0x69, 0xf1, 0xc1, \\\n+\t0x04, 0xd4, 0x5d, 0x8d, 0x85, 0xef, 0xa9, 0xb0, 0x57, 0xb5, \\\n+\t0x3b, 0x14, 0xb4, 0xb9, 0xb9, 0x39, 0xdd, 0x74, 0xde, 0xcc, \\\n+\t0x53, 0x21 \\\n+}\n+\n+#define EMPTY_BLOB_SHA1_BIN_LITERAL { \\\n+\t0xe6, 0x9d, 0xe2, 0x9b, 0xb2, 0xd1, 0xd6, 0x43, 0x4b, 0x8b, \\\n+\t0x29, 0xae, 0x77, 0x5a, 0xd8, 0xc2, 0xe4, 0x8c, 0x53, 0x91 \\\n+}\n+#define EMPTY_BLOB_SHA256_BIN_LITERAL { \\\n+\t0x47, 0x3a, 0x0f, 0x4c, 0x3b, 0xe8, 0xa9, 0x36, 0x81, 0xa2, \\\n+\t0x67, 0xe3, 0xb1, 0xe9, 0xa7, 0xdc, 0xda, 0x11, 0x85, 0x43, \\\n+\t0x6f, 0xe1, 0x41, 0xf7, 0x74, 0x91, 0x20, 0xa3, 0x03, 0x72, \\\n+\t0x18, 0x13 \\\n+}\n \n static const struct object_id empty_tree_oid = {\n \t.hash = EMPTY_TREE_SHA1_BIN_LITERAL,\n-- \n2.47.0.541.ge258d9a1f8\n\n"},{"id":"507431","messageId":"20241117090831.GB3409496@coredump.intra.peff.net","threadId":"62506","inReplyTo":"20241117090329.GA2341486@coredump.intra.peff.net","subject":"[PATCH 2/5] object-file: drop confusing oid initializer of empty_tree struct","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-11-17T09:08:31Z","receivedAt":"2024-11-17T09:08:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We treat the empty tree specially, providing an in-memory \"cached\" copy,\nwhich allows you to diff against it even if the object doesn't exist in\nthe repository. This is implemented as part of the larger cached_object\nsubsystem, but we use a stand-alone empty_tree struct.\n\nWe initialize the oid of that struct using EMPTY_TREE_SHA1_BIN_LITERAL.\nAt first glance, that seems like a bug; how could this ever work for\nsha256 repositories?\n\nThe answer is that we never look at the oid field! The oid field is used\nto look up entries added by pretend_object_file() to the cached_objects\narray. But for our stand-alone entry, we look for it independently using\nthe_hash_algo->empty_tree, which will point to the correct algo struct\nfor the repository.\n\nThis happened in 62ba93eaa9 (sha1_file: convert cached object code to\nstruct object_id, 2018-05-02), which even mentions that this field is\nnever used. Let's reduce confusion for anybody reading this code by\nreplacing the sha1 initializer with a comment. The resulting field will\nbe all-zeroes, so any violation of our assumption that the oid field is\nnot used will break equally for sha1 and sha256.\n\nSigned-off-by: Jeff King \n---\n object-file.c | 4 +---\n 1 file changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex 25ba54594b..b7c4fdcabd 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -326,9 +326,7 @@ static struct cached_object {\n static int cached_object_nr, cached_object_alloc;\n \n static struct cached_object empty_tree = {\n-\t.oid = {\n-\t\t.hash = EMPTY_TREE_SHA1_BIN_LITERAL,\n-\t},\n+\t/* no oid needed; we'll look it up manually based on the_hash_algo */\n \t.type = OBJ_TREE,\n \t.buf = \"\",\n };\n-- \n2.47.0.541.ge258d9a1f8\n\n"},{"id":"507432","messageId":"20241117090842.GC3409496@coredump.intra.peff.net","threadId":"62506","inReplyTo":"20241117090329.GA2341486@coredump.intra.peff.net","subject":"[PATCH 3/5] object-file: move empty_tree struct into find_cached_object()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-11-17T09:08:42Z","receivedAt":"2024-11-17T09:08:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The fake empty_tree struct is a static global, but the only code that\nlooks at it is find_cached_object(). The struct itself is a little odd,\nwith an invalid \"oid\" field that is handled specially by that function.\n\nSince it's really just an implementation detail, let's move it to a\nstatic within the function. That future-proofs against other code trying\nto use it and seeing the weird oid value.\n\nSigned-off-by: Jeff King \n---\n object-file.c | 11 +++++------\n 1 file changed, 5 insertions(+), 6 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex b7c4fdcabd..5fadd470c1 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -325,14 +325,13 @@ static struct cached_object {\n } *cached_objects;\n static int cached_object_nr, cached_object_alloc;\n \n-static struct cached_object empty_tree = {\n-\t/* no oid needed; we'll look it up manually based on the_hash_algo */\n-\t.type = OBJ_TREE,\n-\t.buf = \"\",\n-};\n-\n static struct cached_object *find_cached_object(const struct object_id *oid)\n {\n+\tstatic struct cached_object empty_tree = {\n+\t\t/* no oid needed; we'll look it up manually based on the_hash_algo */\n+\t\t.type = OBJ_TREE,\n+\t\t.buf = \"\",\n+\t};\n \tint i;\n \tstruct cached_object *co = cached_objects;\n \n-- \n2.47.0.541.ge258d9a1f8\n\n"},{"id":"507433","messageId":"20241117091024.GD3409496@coredump.intra.peff.net","threadId":"62506","inReplyTo":"20241117090329.GA2341486@coredump.intra.peff.net","subject":"[PATCH 4/5] object-file: drop oid field from find_cached_object() return value","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-11-17T09:10:24Z","receivedAt":"2024-11-17T09:10:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The pretend_object_file() function adds to an array mapping oids to\nobject contents, which are later retrieved with find_cached_object().\nWe naturally need to store the oid for each entry, since it's the lookup\nkey.\n\nBut find_cached_object() also returns a hard-coded empty_tree object.\nThere we don't care about its oid field and instead compare against\nthe_hash_algo->empty_tree. The oid field is left as all-zeroes.\n\nThis all works, but it means that the cached_object struct we return\nfrom find_cached_object() may or may not have a valid oid field, depend\nwhether it is the hard-coded tree or came from pretend_object_file().\n\nNobody looks at the field, so there's no bug. But let's future-proof it\nby returning only the object contents themselves, not the oid. We'll\ncontinue to call this \"struct cached_object\", and the array entry\nmapping the key to those contents will be a \"cached_object_entry\".\n\nThis would also let us swap out the array for a better data structure\n(like a hashmap) if we chose, but there's not much point. The only code\nthat adds an entry is git-blame, which adds at most a single entry per\nprocess.\n\nSigned-off-by: Jeff King \n---\n object-file.c | 23 ++++++++++++-----------\n 1 file changed, 12 insertions(+), 11 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex 5fadd470c1..e461e351ca 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -317,27 +317,28 @@ int hash_algo_by_length(int len)\n * to write them into the object store (e.g. a browse-only\n * application).\n */\n-static struct cached_object {\n+static struct cached_object_entry {\n \tstruct object_id oid;\n-\tenum object_type type;\n-\tconst void *buf;\n-\tunsigned long size;\n+\tstruct cached_object {\n+\t\tenum object_type type;\n+\t\tconst void *buf;\n+\t\tunsigned long size;\n+\t} value;\n } *cached_objects;\n static int cached_object_nr, cached_object_alloc;\n \n static struct cached_object *find_cached_object(const struct object_id *oid)\n {\n \tstatic struct cached_object empty_tree = {\n-\t\t/* no oid needed; we'll look it up manually based on the_hash_algo */\n \t\t.type = OBJ_TREE,\n \t\t.buf = \"\",\n \t};\n \tint i;\n-\tstruct cached_object *co = cached_objects;\n+\tstruct cached_object_entry *co = cached_objects;\n \n \tfor (i = 0; i < cached_object_nr; i++, co++) {\n \t\tif (oideq(&co->oid, oid))\n-\t\t\treturn co;\n+\t\t\treturn &co->value;\n \t}\n \tif (oideq(oid, the_hash_algo->empty_tree))\n \t\treturn &empty_tree;\n@@ -1850,7 +1851,7 @@ int oid_object_info(struct repository *r,\n int pretend_object_file(void *buf, unsigned long len, enum object_type type,\n \t\t\tstruct object_id *oid)\n {\n-\tstruct cached_object *co;\n+\tstruct cached_object_entry *co;\n \tchar *co_buf;\n \n \thash_object_file(the_hash_algo, buf, len, type, oid);\n@@ -1859,11 +1860,11 @@ int pretend_object_file(void *buf, unsigned long len, enum object_type type,\n \t\treturn 0;\n \tALLOC_GROW(cached_objects, cached_object_nr + 1, cached_object_alloc);\n \tco = &cached_objects[cached_object_nr++];\n-\tco->size = len;\n-\tco->type = type;\n+\tco->value.size = len;\n+\tco->value.type = type;\n \tco_buf = xmalloc(len);\n \tmemcpy(co_buf, buf, len);\n-\tco->buf = co_buf;\n+\tco->value.buf = co_buf;\n \toidcpy(&co->oid, oid);\n \treturn 0;\n }\n-- \n2.47.0.541.ge258d9a1f8\n\n"},{"id":"507434","messageId":"20241117091423.GE3409496@coredump.intra.peff.net","threadId":"62506","inReplyTo":"20241117090329.GA2341486@coredump.intra.peff.net","subject":"[PATCH 5/5] object-file: inline empty tree and blob literals","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-11-17T09:14:23Z","receivedAt":"2024-11-17T09:14:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We define macros with the bytes of the empty trees and blobs for sha1\nand sha256. But since e1ccd7e2b1 (sha1_file: only expose empty object\nconstants through git_hash_algo, 2018-05-02), those are used only for\ninitializing the git_hash_algo entries. Any other code using the macros\ndirectly would be suspicious, since a hash_algo pointer is the level of\nindirection we use to make everything work with both sha1 and sha256.\n\nSo let's future proof against code doing the wrong thing by dropping the\nmacros entirely and just initializing the structs directly.\n\nSigned-off-by: Jeff King \n---\nSadly you can't use --word-diff to make sense of this one, and\n--color-moved is foiled by dropping the trailing backslashes. Anybody is\nwelcome to verify that the values did not change. :)\n\nWe're still left with split-out structs for empty_tree_oid_sha256, etc\n(and confusingly the sha1 ones do not even say \"sha1\"). I think we could\ntake this all one step further and inline those directly into the\ngit_hash_algo. But that would have repercussions throughout the tree,\nsince many spots would switch from:\n\n repo->hash_algo->empty_tree\n\nto:\n\n &repo->hash_algo->empty_tree\n\nNot sure if it's worth it (it's also one less pointer chase, but I kind\nof doubt that accessing the empty tree oid is ever a hot code path).\n\n object-file.c | 47 ++++++++++++++++++++---------------------------\n 1 file changed, 20 insertions(+), 27 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex e461e351ca..e325a52be5 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -44,47 +44,40 @@\n /* The maximum size for an object header. */\n #define MAX_HEADER_LEN 32\n \n-\n-#define EMPTY_TREE_SHA1_BIN_LITERAL { \\\n-\t 0x4b, 0x82, 0x5d, 0xc6, 0x42, 0xcb, 0x6e, 0xb9, 0xa0, 0x60, \\\n-\t 0xe5, 0x4b, 0xf8, 0xd6, 0x92, 0x88, 0xfb, 0xee, 0x49, 0x04 \\\n-}\n-#define EMPTY_TREE_SHA256_BIN_LITERAL { \\\n-\t0x6e, 0xf1, 0x9b, 0x41, 0x22, 0x5c, 0x53, 0x69, 0xf1, 0xc1, \\\n-\t0x04, 0xd4, 0x5d, 0x8d, 0x85, 0xef, 0xa9, 0xb0, 0x57, 0xb5, \\\n-\t0x3b, 0x14, 0xb4, 0xb9, 0xb9, 0x39, 0xdd, 0x74, 0xde, 0xcc, \\\n-\t0x53, 0x21 \\\n-}\n-\n-#define EMPTY_BLOB_SHA1_BIN_LITERAL { \\\n-\t0xe6, 0x9d, 0xe2, 0x9b, 0xb2, 0xd1, 0xd6, 0x43, 0x4b, 0x8b, \\\n-\t0x29, 0xae, 0x77, 0x5a, 0xd8, 0xc2, 0xe4, 0x8c, 0x53, 0x91 \\\n-}\n-#define EMPTY_BLOB_SHA256_BIN_LITERAL { \\\n-\t0x47, 0x3a, 0x0f, 0x4c, 0x3b, 0xe8, 0xa9, 0x36, 0x81, 0xa2, \\\n-\t0x67, 0xe3, 0xb1, 0xe9, 0xa7, 0xdc, 0xda, 0x11, 0x85, 0x43, \\\n-\t0x6f, 0xe1, 0x41, 0xf7, 0x74, 0x91, 0x20, 0xa3, 0x03, 0x72, \\\n-\t0x18, 0x13 \\\n-}\n-\n static const struct object_id empty_tree_oid = {\n-\t.hash = EMPTY_TREE_SHA1_BIN_LITERAL,\n+\t.hash ={\n+\t\t0x4b, 0x82, 0x5d, 0xc6, 0x42, 0xcb, 0x6e, 0xb9, 0xa0, 0x60,\n+\t\t0xe5, 0x4b, 0xf8, 0xd6, 0x92, 0x88, 0xfb, 0xee, 0x49, 0x04\n+\t},\n \t.algo = GIT_HASH_SHA1,\n };\n static const struct object_id empty_blob_oid = {\n-\t.hash = EMPTY_BLOB_SHA1_BIN_LITERAL,\n+\t.hash = {\n+\t\t0xe6, 0x9d, 0xe2, 0x9b, 0xb2, 0xd1, 0xd6, 0x43, 0x4b, 0x8b,\n+\t\t0x29, 0xae, 0x77, 0x5a, 0xd8, 0xc2, 0xe4, 0x8c, 0x53, 0x91\n+\t},\n \t.algo = GIT_HASH_SHA1,\n };\n static const struct object_id null_oid_sha1 = {\n \t.hash = {0},\n \t.algo = GIT_HASH_SHA1,\n };\n static const struct object_id empty_tree_oid_sha256 = {\n-\t.hash = EMPTY_TREE_SHA256_BIN_LITERAL,\n+\t.hash = {\n+\t\t0x6e, 0xf1, 0x9b, 0x41, 0x22, 0x5c, 0x53, 0x69, 0xf1, 0xc1,\n+\t\t0x04, 0xd4, 0x5d, 0x8d, 0x85, 0xef, 0xa9, 0xb0, 0x57, 0xb5,\n+\t\t0x3b, 0x14, 0xb4, 0xb9, 0xb9, 0x39, 0xdd, 0x74, 0xde, 0xcc,\n+\t\t0x53, 0x21\n+\t},\n \t.algo = GIT_HASH_SHA256,\n };\n static const struct object_id empty_blob_oid_sha256 = {\n-\t.hash = EMPTY_BLOB_SHA256_BIN_LITERAL,\n+\t.hash = {\n+\t\t0x47, 0x3a, 0x0f, 0x4c, 0x3b, 0xe8, 0xa9, 0x36, 0x81, 0xa2,\n+\t\t0x67, 0xe3, 0xb1, 0xe9, 0xa7, 0xdc, 0xda, 0x11, 0x85, 0x43,\n+\t\t0x6f, 0xe1, 0x41, 0xf7, 0x74, 0x91, 0x20, 0xa3, 0x03, 0x72,\n+\t\t0x18, 0x13\n+\t},\n \t.algo = GIT_HASH_SHA256,\n };\n static const struct object_id null_oid_sha256 = {\n-- \n2.47.0.541.ge258d9a1f8\n"},{"id":"507435","messageId":"85d78ade-dbf4-4116-836e-a49c33324947@web.de","threadId":"62506","inReplyTo":"20241117090814.GA3409496@coredump.intra.peff.net","subject":"Re: [PATCH 1/5] object-file: prefer array-of-bytes initializer for hash literals","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2024-11-17T09:52:40Z","receivedAt":"2024-11-17T09:53:12Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 17.11.24 um 10:08 schrieb Jeff King:\n> We hard-code a few well-known hash values for empty trees and blobs in\n> both sha1 and sha256 formats. We do so with string literals like this:\n>\n> #define EMPTY_TREE_SHA256_BIN_LITERAL \\\n> \"\\x6e\\xf1\\x9b\\x41\\x22\\x5c\\x53\\x69\\xf1\\xc1\" \\\n> \"\\x04\\xd4\\x5d\\x8d\\x85\\xef\\xa9\\xb0\\x57\\xb5\" \\\n> \"\\x3b\\x14\\xb4\\xb9\\xb9\\x39\\xdd\\x74\\xde\\xcc\" \\\n> \"\\x53\\x21\"\n>\n> and then use it to initialize the hash field of an object_id struct.\n> That hash field is exactly 32 bytes long (the size we need for sha256).\n> But the string literal above is actually 33 bytes long due to the NUL\n> terminator. It's legal in C to initialize from a longer string literal;\n> the extra bytes are just ignored.\n>\n> However, the upcoming gcc 15 will start warning about this:\n>\n> CC object-file.o\n> object-file.c:52:9: warning: initializer-string for array of ‘unsigned char’ is too long [-Wunterminated-string-initialization]\n> 52 | \"\\x6e\\xf1\\x9b\\x41\\x22\\x5c\\x53\\x69\\xf1\\xc1\" \\\n> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n> object-file.c:79:17: note: in expansion of macro ‘EMPTY_TREE_SHA256_BIN_LITERAL’\n>\n> which is understandable. Even though this is not a bug for us, since we\n> do not care about the NUL terminator (and are just using the literal as\n> a convenient format), it would be easy to accidentally create an array\n> that was mistakenly unterminated.\n>\n> We can avoid this warning by switching the initializer to an actual\n> array of unsigned values. That arguably demonstrates our intent more\n> clearly anyway.\n\nOK.\n\n> Reported-by: Sam James \n> Signed-off-by: Jeff King \n> ---\n> I actually didn't find exact wording in the standard for using a\n> longer literal. But C99 section 6.7.8 (Initialization), para 32 shows\n> this exact case as \"example 8\".\n>\n> You can view the diff with \"--color-words --word-diff-regex=.\" to more\n> clearly see that the values themselves weren't changed.\n>\n> object-file.c | 38 +++++++++++++++++++++-----------------\n> 1 file changed, 21 insertions(+), 17 deletions(-)\n>\n> diff --git a/object-file.c b/object-file.c\n> index b1a3463852..25ba54594b 100644\n> --- a/object-file.c\n> +++ b/object-file.c\n> @@ -45,23 +45,27 @@\n> #define MAX_HEADER_LEN 32\n>\n>\n> -#define EMPTY_TREE_SHA1_BIN_LITERAL \\\n> -\t \"\\x4b\\x82\\x5d\\xc6\\x42\\xcb\\x6e\\xb9\\xa0\\x60\" \\\n> -\t \"\\xe5\\x4b\\xf8\\xd6\\x92\\x88\\xfb\\xee\\x49\\x04\"\n> -#define EMPTY_TREE_SHA256_BIN_LITERAL \\\n> -\t\"\\x6e\\xf1\\x9b\\x41\\x22\\x5c\\x53\\x69\\xf1\\xc1\" \\\n> -\t\"\\x04\\xd4\\x5d\\x8d\\x85\\xef\\xa9\\xb0\\x57\\xb5\" \\\n> -\t\"\\x3b\\x14\\xb4\\xb9\\xb9\\x39\\xdd\\x74\\xde\\xcc\" \\\n> -\t\"\\x53\\x21\"\n> -\n> -#define EMPTY_BLOB_SHA1_BIN_LITERAL \\\n> -\t\"\\xe6\\x9d\\xe2\\x9b\\xb2\\xd1\\xd6\\x43\\x4b\\x8b\" \\\n> -\t\"\\x29\\xae\\x77\\x5a\\xd8\\xc2\\xe4\\x8c\\x53\\x91\"\n> -#define EMPTY_BLOB_SHA256_BIN_LITERAL \\\n> -\t\"\\x47\\x3a\\x0f\\x4c\\x3b\\xe8\\xa9\\x36\\x81\\xa2\" \\\n> -\t\"\\x67\\xe3\\xb1\\xe9\\xa7\\xdc\\xda\\x11\\x85\\x43\" \\\n> -\t\"\\x6f\\xe1\\x41\\xf7\\x74\\x91\\x20\\xa3\\x03\\x72\" \\\n> -\t\"\\x18\\x13\"\n> +#define EMPTY_TREE_SHA1_BIN_LITERAL { \\\n> +\t 0x4b, 0x82, 0x5d, 0xc6, 0x42, 0xcb, 0x6e, 0xb9, 0xa0, 0x60, \\\n> +\t 0xe5, 0x4b, 0xf8, 0xd6, 0x92, 0x88, 0xfb, 0xee, 0x49, 0x04 \\\n\nThe added space at the beginning looks seems unintended.\n\nThe two spaces before the backslash look odd. One space, one tab or\nlining up the backslashes with spaces would look better.\n\nPatch 5 does away with those spaces, thankfully. :)\n\n> +}\n> +#define EMPTY_TREE_SHA256_BIN_LITERAL { \\\n> +\t0x6e, 0xf1, 0x9b, 0x41, 0x22, 0x5c, 0x53, 0x69, 0xf1, 0xc1, \\\n> +\t0x04, 0xd4, 0x5d, 0x8d, 0x85, 0xef, 0xa9, 0xb0, 0x57, 0xb5, \\\n> +\t0x3b, 0x14, 0xb4, 0xb9, 0xb9, 0x39, 0xdd, 0x74, 0xde, 0xcc, \\\n> +\t0x53, 0x21 \\\n> +}\n> +\n> +#define EMPTY_BLOB_SHA1_BIN_LITERAL { \\\n> +\t0xe6, 0x9d, 0xe2, 0x9b, 0xb2, 0xd1, 0xd6, 0x43, 0x4b, 0x8b, \\\n> +\t0x29, 0xae, 0x77, 0x5a, 0xd8, 0xc2, 0xe4, 0x8c, 0x53, 0x91 \\\n> +}\n> +#define EMPTY_BLOB_SHA256_BIN_LITERAL { \\\n> +\t0x47, 0x3a, 0x0f, 0x4c, 0x3b, 0xe8, 0xa9, 0x36, 0x81, 0xa2, \\\n> +\t0x67, 0xe3, 0xb1, 0xe9, 0xa7, 0xdc, 0xda, 0x11, 0x85, 0x43, \\\n> +\t0x6f, 0xe1, 0x41, 0xf7, 0x74, 0x91, 0x20, 0xa3, 0x03, 0x72, \\\n> +\t0x18, 0x13 \\\n> +}\n>\n> static const struct object_id empty_tree_oid = {\n> \t.hash = EMPTY_TREE_SHA1_BIN_LITERAL,\n\n"},{"id":"507443","messageId":"ZzoT03rsx7MTqSFl@tapette.crustytoothpaste.net","threadId":"62506","inReplyTo":"20241117090329.GA2341486@coredump.intra.peff.net","subject":"Re: -Wunterminated-string-initialization warning with GCC 15 in object-file.c","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2024-11-17T16:03:31Z","receivedAt":"2024-11-17T16:03:34Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2024-11-17 at 09:03:29, Jeff King wrote:\n> Here are some patches. The first one should fix the warning (but I don't\n> have gcc-15 handy to test!). Please let me know if it works for you (and\n> thank you for reporting).\n\nJust so you know, since I believe you also use Debian unstable, you can\ninstall the gcc-snapshot package (which is, admittedly, rather large)\nand use `CC=/usr/lib/gcc-snapshot/bin/gcc`.\n\n> The others are cleanups and future-proofing I found in the same area.\n> Not strictly required, but IMHO worth doing.\n> \n> +cc brian since I think this is a continuation of some hash-algo\n> cleanups he did earlier, plus he piped up in the other gcc-15 thread. ;)\n\nOther than the issue that René noticed, this seems reasonable to me.\n-- \nbrian m. carlson (they/them or he/him)\nToronto, Ontario, CA\n"},{"id":"507462","messageId":"ZzrvciXuOfw_V6ox@pks.im","threadId":"62506","inReplyTo":"20241117090842.GC3409496@coredump.intra.peff.net","subject":"Re: [PATCH 3/5] object-file: move empty_tree struct into find_cached_object()","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-18T07:40:34Z","receivedAt":"2024-11-18T07:40:48Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sun, Nov 17, 2024 at 04:08:42AM -0500, Jeff King wrote:\n> diff --git a/object-file.c b/object-file.c\n> index b7c4fdcabd..5fadd470c1 100644\n> --- a/object-file.c\n> +++ b/object-file.c\n> @@ -325,14 +325,13 @@ static struct cached_object {\n> } *cached_objects;\n> static int cached_object_nr, cached_object_alloc;\n> \n> -static struct cached_object empty_tree = {\n> -\t/* no oid needed; we'll look it up manually based on the_hash_algo */\n> -\t.type = OBJ_TREE,\n> -\t.buf = \"\",\n> -};\n> -\n> static struct cached_object *find_cached_object(const struct object_id *oid)\n> {\n> +\tstatic struct cached_object empty_tree = {\n> +\t\t/* no oid needed; we'll look it up manually based on the_hash_algo */\n> +\t\t.type = OBJ_TREE,\n> +\t\t.buf = \"\",\n> +\t};\n> \tint i;\n> \tstruct cached_object *co = cached_objects;\n\nI was wondering whether we want to also mark this as `const` so that no\ncaller ever gets the idea of modifying the struct. Something like the\nbelow patch (which applies on \"master\", so it of course would have to\nadapt to your changes).\n\nPatrick\n\ndiff --git a/object-file.c b/object-file.c\nindex b1a3463852..f15a3f6a5f 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -321,7 +321,7 @@ static struct cached_object {\n } *cached_objects;\n static int cached_object_nr, cached_object_alloc;\n \n-static struct cached_object empty_tree = {\n+static const struct cached_object empty_tree = {\n \t.oid = {\n \t\t.hash = EMPTY_TREE_SHA1_BIN_LITERAL,\n \t},\n@@ -329,7 +329,7 @@ static struct cached_object empty_tree = {\n \t.buf = \"\",\n };\n \n-static struct cached_object *find_cached_object(const struct object_id *oid)\n+static const struct cached_object *find_cached_object(const struct object_id *oid)\n {\n \tint i;\n \tstruct cached_object *co = cached_objects;\n@@ -1627,7 +1627,7 @@ static int do_oid_object_info_extended(struct repository *r,\n \t\t\t\t struct object_info *oi, unsigned flags)\n {\n \tstatic struct object_info blank_oi = OBJECT_INFO_INIT;\n-\tstruct cached_object *co;\n+\tconst struct cached_object *co;\n \tstruct pack_entry e;\n \tint rtype;\n \tconst struct object_id *real = oid;\n"},{"id":"507463","messageId":"Zzrvdhl6m04QMBNo@pks.im","threadId":"62506","inReplyTo":"20241117091423.GE3409496@coredump.intra.peff.net","subject":"Re: [PATCH 5/5] object-file: inline empty tree and blob literals","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-18T07:40:38Z","receivedAt":"2024-11-18T07:40:50Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sun, Nov 17, 2024 at 04:14:23AM -0500, Jeff King wrote:\n> static const struct object_id empty_tree_oid = {\n> -\t.hash = EMPTY_TREE_SHA1_BIN_LITERAL,\n> +\t.hash ={\n\nThere's a missing space here.\n\nPatrick\n"},{"id":"507464","messageId":"ZzrvecZnS-b0M-1p@pks.im","threadId":"62506","inReplyTo":"20241117090329.GA2341486@coredump.intra.peff.net","subject":"Re: -Wunterminated-string-initialization warning with GCC 15 in object-file.c","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-18T07:40:41Z","receivedAt":"2024-11-18T07:40:53Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sun, Nov 17, 2024 at 04:03:29AM -0500, Jeff King wrote:\n> On Sun, Nov 17, 2024 at 02:50:39AM +0000, Sam James wrote:\n> \n> > With upcoming GCC 15, a new warning is added\n> > (-Wunterminated-string-initialization) that fires when building git:\n> > ```\n> > CC object-file.o\n> > object-file.c:52:9: warning: initializer-string for array of ‘unsigned char’ is too long [-Wunterminated-string-initialization]\n> > 52 | \"\\x6e\\xf1\\x9b\\x41\\x22\\x5c\\x53\\x69\\xf1\\xc1\" \\\n> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n> > object-file.c:79:17: note: in expansion of macro ‘EMPTY_TREE_SHA256_BIN_LITERAL’\n> > 79 | .hash = EMPTY_TREE_SHA256_BIN_LITERAL,\n> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n> > object-file.c:61:9: warning: initializer-string for array of ‘unsigned char’ is too long [-Wunterminated-string-initialization]\n> > 61 | \"\\x47\\x3a\\x0f\\x4c\\x3b\\xe8\\xa9\\x36\\x81\\xa2\" \\\n> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n> > object-file.c:83:17: note: in expansion of macro ‘EMPTY_BLOB_SHA256_BIN_LITERAL’\n> > 83 | .hash = EMPTY_BLOB_SHA256_BIN_LITERAL,\n> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n> > ```\n> > \n> > Context for the new warning is at https://gcc.gnu.org/PR115185.\n> \n> I think the warning is a false positive for us, but I don't begrudge\n> them for adding it. It could definitely catch real problems.\n> \n> Here are some patches. The first one should fix the warning (but I don't\n> have gcc-15 handy to test!). Please let me know if it works for you (and\n> thank you for reporting).\n> \n> The others are cleanups and future-proofing I found in the same area.\n> Not strictly required, but IMHO worth doing.\n> \n> +cc brian since I think this is a continuation of some hash-algo\n> cleanups he did earlier, plus he piped up in the other gcc-15 thread. ;)\n\nI've got two comments, but other than that this looks like a nice\ncleanup to me. Thanks!\n\nPatrick\n"},{"id":"507475","messageId":"20241118090633.GA3984843@coredump.intra.peff.net","threadId":"62506","inReplyTo":"85d78ade-dbf4-4116-836e-a49c33324947@web.de","subject":"Re: [PATCH 1/5] object-file: prefer array-of-bytes initializer for hash literals","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-11-18T09:06:33Z","receivedAt":"2024-11-18T09:06:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 17, 2024 at 10:52:40AM +0100, René Scharfe wrote:\n\n> > -#define EMPTY_BLOB_SHA256_BIN_LITERAL \\\n> > -\t\"\\x47\\x3a\\x0f\\x4c\\x3b\\xe8\\xa9\\x36\\x81\\xa2\" \\\n> > -\t\"\\x67\\xe3\\xb1\\xe9\\xa7\\xdc\\xda\\x11\\x85\\x43\" \\\n> > -\t\"\\x6f\\xe1\\x41\\xf7\\x74\\x91\\x20\\xa3\\x03\\x72\" \\\n> > -\t\"\\x18\\x13\"\n> > +#define EMPTY_TREE_SHA1_BIN_LITERAL { \\\n> > +\t 0x4b, 0x82, 0x5d, 0xc6, 0x42, 0xcb, 0x6e, 0xb9, 0xa0, 0x60, \\\n> > +\t 0xe5, 0x4b, 0xf8, 0xd6, 0x92, 0x88, 0xfb, 0xee, 0x49, 0x04 \\\n> \n> The added space at the beginning looks seems unintended.\n\nThat was from the original, which had a tab followed by a space (maybe\nto line up with the \"E\" in \"EMPTY\"?). I did s/\"// and s/\\\\x\\(..\\)/0x\\1, /\nwhich left it.\n\n> The two spaces before the backslash look odd. One space, one tab or\n> lining up the backslashes with spaces would look better.\n\nThis one is my fault, though. My regex left an extra comma at the end,\nwhich I somehow managed to bungle removing. ;)\n\n> Patch 5 does away with those spaces, thankfully. :)\n\nYep. Looks like there might be some whitespace oddities left over,\nthough, so I'll fix this on re-roll.\n\nThanks.\n\n-Peff\n"},{"id":"507476","messageId":"20241118091742.GB3984843@coredump.intra.peff.net","threadId":"62506","inReplyTo":"ZzrvciXuOfw_V6ox@pks.im","subject":"Re: [PATCH 3/5] object-file: move empty_tree struct into find_cached_object()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-11-18T09:17:42Z","receivedAt":"2024-11-18T09:17:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 18, 2024 at 08:40:34AM +0100, Patrick Steinhardt wrote:\n\n> > static struct cached_object *find_cached_object(const struct object_id *oid)\n> > {\n> > +\tstatic struct cached_object empty_tree = {\n> > +\t\t/* no oid needed; we'll look it up manually based on the_hash_algo */\n> > +\t\t.type = OBJ_TREE,\n> > +\t\t.buf = \"\",\n> > +\t};\n> > \tint i;\n> > \tstruct cached_object *co = cached_objects;\n> \n> I was wondering whether we want to also mark this as `const` so that no\n> caller ever gets the idea of modifying the struct. Something like the\n> below patch (which applies on \"master\", so it of course would have to\n> adapt to your changes).\n\nThis seems like a fairly unlikely bug to me, just because it would be\nweird for somebody to want to write to the response (whereas the other\nfuture-proofing was against somebody reading a private-ish value).\nStill, I agree that \"const\" is the right thing, and it's not hard to add\nit in to my series.\n\n> diff --git a/object-file.c b/object-file.c\n> index b1a3463852..f15a3f6a5f 100644\n> --- a/object-file.c\n> +++ b/object-file.c\n> @@ -321,7 +321,7 @@ static struct cached_object {\n> } *cached_objects;\n> static int cached_object_nr, cached_object_alloc;\n> \n> -static struct cached_object empty_tree = {\n> +static const struct cached_object empty_tree = {\n> \t.oid = {\n> \t\t.hash = EMPTY_TREE_SHA1_BIN_LITERAL,\n> \t},\n\nThis hunk is technically not needed since we can implicitly cast from\nnon-const to const when returning. I included it, though, along with\nmaking the iteration pointer in find_cached_object() const since that\nbetter represents the intent.\n\n-Peff\n"},{"id":"507477","messageId":"20241118091948.GC3984843@coredump.intra.peff.net","threadId":"62506","inReplyTo":"ZzoT03rsx7MTqSFl@tapette.crustytoothpaste.net","subject":"Re: -Wunterminated-string-initialization warning with GCC 15 in object-file.c","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-11-18T09:19:48Z","receivedAt":"2024-11-18T09:19:49Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 17, 2024 at 04:03:31PM +0000, brian m. carlson wrote:\n\n> On 2024-11-17 at 09:03:29, Jeff King wrote:\n> > Here are some patches. The first one should fix the warning (but I don't\n> > have gcc-15 handy to test!). Please let me know if it works for you (and\n> > thank you for reporting).\n> \n> Just so you know, since I believe you also use Debian unstable, you can\n> install the gcc-snapshot package (which is, admittedly, rather large)\n> and use `CC=/usr/lib/gcc-snapshot/bin/gcc`.\n\nThanks, I was stupidly looking for a \"gcc-15\" package in experimental,\nnot realizing it had not actually been released yet. I reproduced the\nproblem with the snapshot (which is from 20241004) and verified that my\nseries fixes it.\n\n-Peff\n"},{"id":"507480","messageId":"20241118095423.GA3990835@coredump.intra.peff.net","threadId":"62506","inReplyTo":"20241117090329.GA2341486@coredump.intra.peff.net","subject":"[PATCH 0/6] -Wunterminated-string-initialization warning + cleanups","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-11-18T09:54:23Z","receivedAt":"2024-11-18T09:54:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"> Here are some patches. The first one should fix the warning (but I don't\n> have gcc-15 handy to test!). Please let me know if it works for you (and\n> thank you for reporting).\n\nAnd here's a minor re-roll from comments on the list. I was able to\nreproduce and test myself this time; the patch indeed fixes the problem.\n\nChanges from v1:\n\n - add more standards explanation to the first commit (thanks for\n pointers from Chris Torek off-list)\n\n - fixes small whitespace issues in patches 1 and 6\n\n - new patch (5) to add \"const\" as appropriate\n\n [1/6]: object-file: prefer array-of-bytes initializer for hash literals\n [2/6]: object-file: drop confusing oid initializer of empty_tree struct\n [3/6]: object-file: move empty_tree struct into find_cached_object()\n [4/6]: object-file: drop oid field from find_cached_object() return value\n [5/6]: object-file: treat cached_object values as const\n [6/6]: object-file: inline empty tree and blob literals\n\n object-file.c | 81 ++++++++++++++++++++++++---------------------------\n 1 file changed, 38 insertions(+), 43 deletions(-)\n\n1: da69342eba ! 1: ec76b9eebb object-file: prefer array-of-bytes initializer for hash literals\n @@ Commit message\n and then use it to initialize the hash field of an object_id struct.\n That hash field is exactly 32 bytes long (the size we need for sha256).\n But the string literal above is actually 33 bytes long due to the NUL\n - terminator. It's legal in C to initialize from a longer string literal;\n - the extra bytes are just ignored.\n + terminator. This is legal in C, and the NUL is ignored.\n \n - However, the upcoming gcc 15 will start warning about this:\n + Side note on legality: in general excess initializer elements are\n + forbidden, and gcc will warn on both of these:\n +\n + char foo[3] = { 'h', 'u', 'g', 'e' };\n + char bar[3] = \"VeryLongString\";\n +\n + I couldn't find specific language in the standard allowing\n + initialization from a string literal where _just_ the NUL is ignored,\n + but C99 section 6.7.8 (Initialization), paragraph 32 shows this exact\n + case as \"example 8\".\n +\n + However, the upcoming gcc 15 will start warning for this case (when\n + compiled with -Wextra via DEVELOPER=1):\n \n CC object-file.o\n object-file.c:52:9: warning: initializer-string for array of ‘unsigned char’ is too long [-Wunterminated-string-initialization]\n @@ object-file.c\n -\t\"\\x6f\\xe1\\x41\\xf7\\x74\\x91\\x20\\xa3\\x03\\x72\" \\\n -\t\"\\x18\\x13\"\n +#define EMPTY_TREE_SHA1_BIN_LITERAL { \\\n -+\t 0x4b, 0x82, 0x5d, 0xc6, 0x42, 0xcb, 0x6e, 0xb9, 0xa0, 0x60, \\\n -+\t 0xe5, 0x4b, 0xf8, 0xd6, 0x92, 0x88, 0xfb, 0xee, 0x49, 0x04 \\\n ++\t0x4b, 0x82, 0x5d, 0xc6, 0x42, 0xcb, 0x6e, 0xb9, 0xa0, 0x60, \\\n ++\t0xe5, 0x4b, 0xf8, 0xd6, 0x92, 0x88, 0xfb, 0xee, 0x49, 0x04 \\\n +}\n +#define EMPTY_TREE_SHA256_BIN_LITERAL { \\\n -+\t0x6e, 0xf1, 0x9b, 0x41, 0x22, 0x5c, 0x53, 0x69, 0xf1, 0xc1, \\\n -+\t0x04, 0xd4, 0x5d, 0x8d, 0x85, 0xef, 0xa9, 0xb0, 0x57, 0xb5, \\\n -+\t0x3b, 0x14, 0xb4, 0xb9, 0xb9, 0x39, 0xdd, 0x74, 0xde, 0xcc, \\\n ++\t0x6e, 0xf1, 0x9b, 0x41, 0x22, 0x5c, 0x53, 0x69, 0xf1, 0xc1, \\\n ++\t0x04, 0xd4, 0x5d, 0x8d, 0x85, 0xef, 0xa9, 0xb0, 0x57, 0xb5, \\\n ++\t0x3b, 0x14, 0xb4, 0xb9, 0xb9, 0x39, 0xdd, 0x74, 0xde, 0xcc, \\\n +\t0x53, 0x21 \\\n +}\n +\n +#define EMPTY_BLOB_SHA1_BIN_LITERAL { \\\n -+\t0xe6, 0x9d, 0xe2, 0x9b, 0xb2, 0xd1, 0xd6, 0x43, 0x4b, 0x8b, \\\n ++\t0xe6, 0x9d, 0xe2, 0x9b, 0xb2, 0xd1, 0xd6, 0x43, 0x4b, 0x8b, \\\n +\t0x29, 0xae, 0x77, 0x5a, 0xd8, 0xc2, 0xe4, 0x8c, 0x53, 0x91 \\\n +}\n +#define EMPTY_BLOB_SHA256_BIN_LITERAL { \\\n -+\t0x47, 0x3a, 0x0f, 0x4c, 0x3b, 0xe8, 0xa9, 0x36, 0x81, 0xa2, \\\n -+\t0x67, 0xe3, 0xb1, 0xe9, 0xa7, 0xdc, 0xda, 0x11, 0x85, 0x43, \\\n -+\t0x6f, 0xe1, 0x41, 0xf7, 0x74, 0x91, 0x20, 0xa3, 0x03, 0x72, \\\n ++\t0x47, 0x3a, 0x0f, 0x4c, 0x3b, 0xe8, 0xa9, 0x36, 0x81, 0xa2, \\\n ++\t0x67, 0xe3, 0xb1, 0xe9, 0xa7, 0xdc, 0xda, 0x11, 0x85, 0x43, \\\n ++\t0x6f, 0xe1, 0x41, 0xf7, 0x74, 0x91, 0x20, 0xa3, 0x03, 0x72, \\\n +\t0x18, 0x13 \\\n +}\n \n2: b8416b33d2 = 2: 0beaf2d65e object-file: drop confusing oid initializer of empty_tree struct\n3: 8f5a9f5e30 = 3: d0c28cb1c9 object-file: move empty_tree struct into find_cached_object()\n4: e2d0c9b56d = 4: 551e5938d5 object-file: drop oid field from find_cached_object() return value\n-: ---------- > 5: d5641358a2 object-file: treat cached_object values as const\n5: 7ebc8d2d2c ! 6: 82c43bfc78 object-file: inline empty tree and blob literals\n @@ object-file.c\n \n -\n -#define EMPTY_TREE_SHA1_BIN_LITERAL { \\\n --\t 0x4b, 0x82, 0x5d, 0xc6, 0x42, 0xcb, 0x6e, 0xb9, 0xa0, 0x60, \\\n --\t 0xe5, 0x4b, 0xf8, 0xd6, 0x92, 0x88, 0xfb, 0xee, 0x49, 0x04 \\\n +-\t0x4b, 0x82, 0x5d, 0xc6, 0x42, 0xcb, 0x6e, 0xb9, 0xa0, 0x60, \\\n +-\t0xe5, 0x4b, 0xf8, 0xd6, 0x92, 0x88, 0xfb, 0xee, 0x49, 0x04 \\\n -}\n -#define EMPTY_TREE_SHA256_BIN_LITERAL { \\\n --\t0x6e, 0xf1, 0x9b, 0x41, 0x22, 0x5c, 0x53, 0x69, 0xf1, 0xc1, \\\n --\t0x04, 0xd4, 0x5d, 0x8d, 0x85, 0xef, 0xa9, 0xb0, 0x57, 0xb5, \\\n --\t0x3b, 0x14, 0xb4, 0xb9, 0xb9, 0x39, 0xdd, 0x74, 0xde, 0xcc, \\\n +-\t0x6e, 0xf1, 0x9b, 0x41, 0x22, 0x5c, 0x53, 0x69, 0xf1, 0xc1, \\\n +-\t0x04, 0xd4, 0x5d, 0x8d, 0x85, 0xef, 0xa9, 0xb0, 0x57, 0xb5, \\\n +-\t0x3b, 0x14, 0xb4, 0xb9, 0xb9, 0x39, 0xdd, 0x74, 0xde, 0xcc, \\\n -\t0x53, 0x21 \\\n -}\n -\n -#define EMPTY_BLOB_SHA1_BIN_LITERAL { \\\n --\t0xe6, 0x9d, 0xe2, 0x9b, 0xb2, 0xd1, 0xd6, 0x43, 0x4b, 0x8b, \\\n +-\t0xe6, 0x9d, 0xe2, 0x9b, 0xb2, 0xd1, 0xd6, 0x43, 0x4b, 0x8b, \\\n -\t0x29, 0xae, 0x77, 0x5a, 0xd8, 0xc2, 0xe4, 0x8c, 0x53, 0x91 \\\n -}\n -#define EMPTY_BLOB_SHA256_BIN_LITERAL { \\\n --\t0x47, 0x3a, 0x0f, 0x4c, 0x3b, 0xe8, 0xa9, 0x36, 0x81, 0xa2, \\\n --\t0x67, 0xe3, 0xb1, 0xe9, 0xa7, 0xdc, 0xda, 0x11, 0x85, 0x43, \\\n --\t0x6f, 0xe1, 0x41, 0xf7, 0x74, 0x91, 0x20, 0xa3, 0x03, 0x72, \\\n +-\t0x47, 0x3a, 0x0f, 0x4c, 0x3b, 0xe8, 0xa9, 0x36, 0x81, 0xa2, \\\n +-\t0x67, 0xe3, 0xb1, 0xe9, 0xa7, 0xdc, 0xda, 0x11, 0x85, 0x43, \\\n +-\t0x6f, 0xe1, 0x41, 0xf7, 0x74, 0x91, 0x20, 0xa3, 0x03, 0x72, \\\n -\t0x18, 0x13 \\\n -}\n -\n static const struct object_id empty_tree_oid = {\n -\t.hash = EMPTY_TREE_SHA1_BIN_LITERAL,\n -+\t.hash ={\n ++\t.hash = {\n +\t\t0x4b, 0x82, 0x5d, 0xc6, 0x42, 0xcb, 0x6e, 0xb9, 0xa0, 0x60,\n +\t\t0xe5, 0x4b, 0xf8, 0xd6, 0x92, 0x88, 0xfb, 0xee, 0x49, 0x04\n +\t},\n"},{"id":"507481","messageId":"20241118095440.GA3992317@coredump.intra.peff.net","threadId":"62506","inReplyTo":"20241118095423.GA3990835@coredump.intra.peff.net","subject":"[PATCH 1/6] object-file: prefer array-of-bytes initializer for hash literals","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-11-18T09:54:40Z","receivedAt":"2024-11-18T09:54:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We hard-code a few well-known hash values for empty trees and blobs in\nboth sha1 and sha256 formats. We do so with string literals like this:\n\n #define EMPTY_TREE_SHA256_BIN_LITERAL \\\n \"\\x6e\\xf1\\x9b\\x41\\x22\\x5c\\x53\\x69\\xf1\\xc1\" \\\n \"\\x04\\xd4\\x5d\\x8d\\x85\\xef\\xa9\\xb0\\x57\\xb5\" \\\n \"\\x3b\\x14\\xb4\\xb9\\xb9\\x39\\xdd\\x74\\xde\\xcc\" \\\n \"\\x53\\x21\"\n\nand then use it to initialize the hash field of an object_id struct.\nThat hash field is exactly 32 bytes long (the size we need for sha256).\nBut the string literal above is actually 33 bytes long due to the NUL\nterminator. This is legal in C, and the NUL is ignored.\n\n Side note on legality: in general excess initializer elements are\n forbidden, and gcc will warn on both of these:\n\n char foo[3] = { 'h', 'u', 'g', 'e' };\n char bar[3] = \"VeryLongString\";\n\n I couldn't find specific language in the standard allowing\n initialization from a string literal where _just_ the NUL is ignored,\n but C99 section 6.7.8 (Initialization), paragraph 32 shows this exact\n case as \"example 8\".\n\nHowever, the upcoming gcc 15 will start warning for this case (when\ncompiled with -Wextra via DEVELOPER=1):\n\n CC object-file.o\n object-file.c:52:9: warning: initializer-string for array of ‘unsigned char’ is too long [-Wunterminated-string-initialization]\n 52 | \"\\x6e\\xf1\\x9b\\x41\\x22\\x5c\\x53\\x69\\xf1\\xc1\" \\\n | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n object-file.c:79:17: note: in expansion of macro ‘EMPTY_TREE_SHA256_BIN_LITERAL’\n\nwhich is understandable. Even though this is not a bug for us, since we\ndo not care about the NUL terminator (and are just using the literal as\na convenient format), it would be easy to accidentally create an array\nthat was mistakenly unterminated.\n\nWe can avoid this warning by switching the initializer to an actual\narray of unsigned values. That arguably demonstrates our intent more\nclearly anyway.\n\nReported-by: Sam James \nSigned-off-by: Jeff King \n---\n object-file.c | 38 +++++++++++++++++++++-----------------\n 1 file changed, 21 insertions(+), 17 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex b1a3463852..8101585616 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -45,23 +45,27 @@\n #define MAX_HEADER_LEN 32\n \n \n-#define EMPTY_TREE_SHA1_BIN_LITERAL \\\n-\t \"\\x4b\\x82\\x5d\\xc6\\x42\\xcb\\x6e\\xb9\\xa0\\x60\" \\\n-\t \"\\xe5\\x4b\\xf8\\xd6\\x92\\x88\\xfb\\xee\\x49\\x04\"\n-#define EMPTY_TREE_SHA256_BIN_LITERAL \\\n-\t\"\\x6e\\xf1\\x9b\\x41\\x22\\x5c\\x53\\x69\\xf1\\xc1\" \\\n-\t\"\\x04\\xd4\\x5d\\x8d\\x85\\xef\\xa9\\xb0\\x57\\xb5\" \\\n-\t\"\\x3b\\x14\\xb4\\xb9\\xb9\\x39\\xdd\\x74\\xde\\xcc\" \\\n-\t\"\\x53\\x21\"\n-\n-#define EMPTY_BLOB_SHA1_BIN_LITERAL \\\n-\t\"\\xe6\\x9d\\xe2\\x9b\\xb2\\xd1\\xd6\\x43\\x4b\\x8b\" \\\n-\t\"\\x29\\xae\\x77\\x5a\\xd8\\xc2\\xe4\\x8c\\x53\\x91\"\n-#define EMPTY_BLOB_SHA256_BIN_LITERAL \\\n-\t\"\\x47\\x3a\\x0f\\x4c\\x3b\\xe8\\xa9\\x36\\x81\\xa2\" \\\n-\t\"\\x67\\xe3\\xb1\\xe9\\xa7\\xdc\\xda\\x11\\x85\\x43\" \\\n-\t\"\\x6f\\xe1\\x41\\xf7\\x74\\x91\\x20\\xa3\\x03\\x72\" \\\n-\t\"\\x18\\x13\"\n+#define EMPTY_TREE_SHA1_BIN_LITERAL { \\\n+\t0x4b, 0x82, 0x5d, 0xc6, 0x42, 0xcb, 0x6e, 0xb9, 0xa0, 0x60, \\\n+\t0xe5, 0x4b, 0xf8, 0xd6, 0x92, 0x88, 0xfb, 0xee, 0x49, 0x04 \\\n+}\n+#define EMPTY_TREE_SHA256_BIN_LITERAL { \\\n+\t0x6e, 0xf1, 0x9b, 0x41, 0x22, 0x5c, 0x53, 0x69, 0xf1, 0xc1, \\\n+\t0x04, 0xd4, 0x5d, 0x8d, 0x85, 0xef, 0xa9, 0xb0, 0x57, 0xb5, \\\n+\t0x3b, 0x14, 0xb4, 0xb9, 0xb9, 0x39, 0xdd, 0x74, 0xde, 0xcc, \\\n+\t0x53, 0x21 \\\n+}\n+\n+#define EMPTY_BLOB_SHA1_BIN_LITERAL { \\\n+\t0xe6, 0x9d, 0xe2, 0x9b, 0xb2, 0xd1, 0xd6, 0x43, 0x4b, 0x8b, \\\n+\t0x29, 0xae, 0x77, 0x5a, 0xd8, 0xc2, 0xe4, 0x8c, 0x53, 0x91 \\\n+}\n+#define EMPTY_BLOB_SHA256_BIN_LITERAL { \\\n+\t0x47, 0x3a, 0x0f, 0x4c, 0x3b, 0xe8, 0xa9, 0x36, 0x81, 0xa2, \\\n+\t0x67, 0xe3, 0xb1, 0xe9, 0xa7, 0xdc, 0xda, 0x11, 0x85, 0x43, \\\n+\t0x6f, 0xe1, 0x41, 0xf7, 0x74, 0x91, 0x20, 0xa3, 0x03, 0x72, \\\n+\t0x18, 0x13 \\\n+}\n \n static const struct object_id empty_tree_oid = {\n \t.hash = EMPTY_TREE_SHA1_BIN_LITERAL,\n-- \n2.47.0.547.g778689293a\n\n"},{"id":"507482","messageId":"20241118095507.GB3992317@coredump.intra.peff.net","threadId":"62506","inReplyTo":"20241118095423.GA3990835@coredump.intra.peff.net","subject":"[PATCH 2/6] object-file: drop confusing oid initializer of empty_tree struct","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-11-18T09:55:07Z","receivedAt":"2024-11-18T09:55:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We treat the empty tree specially, providing an in-memory \"cached\" copy,\nwhich allows you to diff against it even if the object doesn't exist in\nthe repository. This is implemented as part of the larger cached_object\nsubsystem, but we use a stand-alone empty_tree struct.\n\nWe initialize the oid of that struct using EMPTY_TREE_SHA1_BIN_LITERAL.\nAt first glance, that seems like a bug; how could this ever work for\nsha256 repositories?\n\nThe answer is that we never look at the oid field! The oid field is used\nto look up entries added by pretend_object_file() to the cached_objects\narray. But for our stand-alone entry, we look for it independently using\nthe_hash_algo->empty_tree, which will point to the correct algo struct\nfor the repository.\n\nThis happened in 62ba93eaa9 (sha1_file: convert cached object code to\nstruct object_id, 2018-05-02), which even mentions that this field is\nnever used. Let's reduce confusion for anybody reading this code by\nreplacing the sha1 initializer with a comment. The resulting field will\nbe all-zeroes, so any violation of our assumption that the oid field is\nnot used will break equally for sha1 and sha256.\n\nSigned-off-by: Jeff King \n---\n object-file.c | 4 +---\n 1 file changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex 8101585616..19fc4afa43 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -326,9 +326,7 @@ static struct cached_object {\n static int cached_object_nr, cached_object_alloc;\n \n static struct cached_object empty_tree = {\n-\t.oid = {\n-\t\t.hash = EMPTY_TREE_SHA1_BIN_LITERAL,\n-\t},\n+\t/* no oid needed; we'll look it up manually based on the_hash_algo */\n \t.type = OBJ_TREE,\n \t.buf = \"\",\n };\n-- \n2.47.0.547.g778689293a\n\n"},{"id":"507483","messageId":"20241118095511.GC3992317@coredump.intra.peff.net","threadId":"62506","inReplyTo":"20241118095423.GA3990835@coredump.intra.peff.net","subject":"[PATCH 3/6] object-file: move empty_tree struct into find_cached_object()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-11-18T09:55:11Z","receivedAt":"2024-11-18T09:55:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The fake empty_tree struct is a static global, but the only code that\nlooks at it is find_cached_object(). The struct itself is a little odd,\nwith an invalid \"oid\" field that is handled specially by that function.\n\nSince it's really just an implementation detail, let's move it to a\nstatic within the function. That future-proofs against other code trying\nto use it and seeing the weird oid value.\n\nSigned-off-by: Jeff King \n---\n object-file.c | 11 +++++------\n 1 file changed, 5 insertions(+), 6 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex 19fc4afa43..4d4280543e 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -325,14 +325,13 @@ static struct cached_object {\n } *cached_objects;\n static int cached_object_nr, cached_object_alloc;\n \n-static struct cached_object empty_tree = {\n-\t/* no oid needed; we'll look it up manually based on the_hash_algo */\n-\t.type = OBJ_TREE,\n-\t.buf = \"\",\n-};\n-\n static struct cached_object *find_cached_object(const struct object_id *oid)\n {\n+\tstatic struct cached_object empty_tree = {\n+\t\t/* no oid needed; we'll look it up manually based on the_hash_algo */\n+\t\t.type = OBJ_TREE,\n+\t\t.buf = \"\",\n+\t};\n \tint i;\n \tstruct cached_object *co = cached_objects;\n \n-- \n2.47.0.547.g778689293a\n\n"},{"id":"507484","messageId":"20241118095515.GD3992317@coredump.intra.peff.net","threadId":"62506","inReplyTo":"20241118095423.GA3990835@coredump.intra.peff.net","subject":"[PATCH 4/6] object-file: drop oid field from find_cached_object() return value","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-11-18T09:55:15Z","receivedAt":"2024-11-18T09:55:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The pretend_object_file() function adds to an array mapping oids to\nobject contents, which are later retrieved with find_cached_object().\nWe naturally need to store the oid for each entry, since it's the lookup\nkey.\n\nBut find_cached_object() also returns a hard-coded empty_tree object.\nThere we don't care about its oid field and instead compare against\nthe_hash_algo->empty_tree. The oid field is left as all-zeroes.\n\nThis all works, but it means that the cached_object struct we return\nfrom find_cached_object() may or may not have a valid oid field, depend\nwhether it is the hard-coded tree or came from pretend_object_file().\n\nNobody looks at the field, so there's no bug. But let's future-proof it\nby returning only the object contents themselves, not the oid. We'll\ncontinue to call this \"struct cached_object\", and the array entry\nmapping the key to those contents will be a \"cached_object_entry\".\n\nThis would also let us swap out the array for a better data structure\n(like a hashmap) if we chose, but there's not much point. The only code\nthat adds an entry is git-blame, which adds at most a single entry per\nprocess.\n\nSigned-off-by: Jeff King \n---\n object-file.c | 23 ++++++++++++-----------\n 1 file changed, 12 insertions(+), 11 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex 4d4280543e..67a6731066 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -317,27 +317,28 @@ int hash_algo_by_length(int len)\n * to write them into the object store (e.g. a browse-only\n * application).\n */\n-static struct cached_object {\n+static struct cached_object_entry {\n \tstruct object_id oid;\n-\tenum object_type type;\n-\tconst void *buf;\n-\tunsigned long size;\n+\tstruct cached_object {\n+\t\tenum object_type type;\n+\t\tconst void *buf;\n+\t\tunsigned long size;\n+\t} value;\n } *cached_objects;\n static int cached_object_nr, cached_object_alloc;\n \n static struct cached_object *find_cached_object(const struct object_id *oid)\n {\n \tstatic struct cached_object empty_tree = {\n-\t\t/* no oid needed; we'll look it up manually based on the_hash_algo */\n \t\t.type = OBJ_TREE,\n \t\t.buf = \"\",\n \t};\n \tint i;\n-\tstruct cached_object *co = cached_objects;\n+\tstruct cached_object_entry *co = cached_objects;\n \n \tfor (i = 0; i < cached_object_nr; i++, co++) {\n \t\tif (oideq(&co->oid, oid))\n-\t\t\treturn co;\n+\t\t\treturn &co->value;\n \t}\n \tif (oideq(oid, the_hash_algo->empty_tree))\n \t\treturn &empty_tree;\n@@ -1850,7 +1851,7 @@ int oid_object_info(struct repository *r,\n int pretend_object_file(void *buf, unsigned long len, enum object_type type,\n \t\t\tstruct object_id *oid)\n {\n-\tstruct cached_object *co;\n+\tstruct cached_object_entry *co;\n \tchar *co_buf;\n \n \thash_object_file(the_hash_algo, buf, len, type, oid);\n@@ -1859,11 +1860,11 @@ int pretend_object_file(void *buf, unsigned long len, enum object_type type,\n \t\treturn 0;\n \tALLOC_GROW(cached_objects, cached_object_nr + 1, cached_object_alloc);\n \tco = &cached_objects[cached_object_nr++];\n-\tco->size = len;\n-\tco->type = type;\n+\tco->value.size = len;\n+\tco->value.type = type;\n \tco_buf = xmalloc(len);\n \tmemcpy(co_buf, buf, len);\n-\tco->buf = co_buf;\n+\tco->value.buf = co_buf;\n \toidcpy(&co->oid, oid);\n \treturn 0;\n }\n-- \n2.47.0.547.g778689293a\n\n"},{"id":"507485","messageId":"20241118095519.GE3992317@coredump.intra.peff.net","threadId":"62506","inReplyTo":"20241118095423.GA3990835@coredump.intra.peff.net","subject":"[PATCH 5/6] object-file: treat cached_object values as const","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-11-18T09:55:19Z","receivedAt":"2024-11-18T09:55:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The cached-object API maps oids to in-memory entries. Once inserted,\nthese entries should be immutable. Let's return them from the\nfind_cached_object() call with a const tag to make this clear.\n\nSuggested-by: Patrick Steinhardt \nSigned-off-by: Jeff King \n---\n object-file.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex 67a6731066..ec62e5fb3b 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -327,14 +327,14 @@ static struct cached_object_entry {\n } *cached_objects;\n static int cached_object_nr, cached_object_alloc;\n \n-static struct cached_object *find_cached_object(const struct object_id *oid)\n+static const struct cached_object *find_cached_object(const struct object_id *oid)\n {\n-\tstatic struct cached_object empty_tree = {\n+\tstatic const struct cached_object empty_tree = {\n \t\t.type = OBJ_TREE,\n \t\t.buf = \"\",\n \t};\n \tint i;\n-\tstruct cached_object_entry *co = cached_objects;\n+\tconst struct cached_object_entry *co = cached_objects;\n \n \tfor (i = 0; i < cached_object_nr; i++, co++) {\n \t\tif (oideq(&co->oid, oid))\n@@ -1629,7 +1629,7 @@ static int do_oid_object_info_extended(struct repository *r,\n \t\t\t\t struct object_info *oi, unsigned flags)\n {\n \tstatic struct object_info blank_oi = OBJECT_INFO_INIT;\n-\tstruct cached_object *co;\n+\tconst struct cached_object *co;\n \tstruct pack_entry e;\n \tint rtype;\n \tconst struct object_id *real = oid;\n-- \n2.47.0.547.g778689293a\n\n"},{"id":"507486","messageId":"20241118095522.GF3992317@coredump.intra.peff.net","threadId":"62506","inReplyTo":"20241118095423.GA3990835@coredump.intra.peff.net","subject":"[PATCH 6/6] object-file: inline empty tree and blob literals","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-11-18T09:55:22Z","receivedAt":"2024-11-18T09:55:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We define macros with the bytes of the empty trees and blobs for sha1\nand sha256. But since e1ccd7e2b1 (sha1_file: only expose empty object\nconstants through git_hash_algo, 2018-05-02), those are used only for\ninitializing the git_hash_algo entries. Any other code using the macros\ndirectly would be suspicious, since a hash_algo pointer is the level of\nindirection we use to make everything work with both sha1 and sha256.\n\nSo let's future proof against code doing the wrong thing by dropping the\nmacros entirely and just initializing the structs directly.\n\nSigned-off-by: Jeff King \n---\n object-file.c | 47 ++++++++++++++++++++---------------------------\n 1 file changed, 20 insertions(+), 27 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex ec62e5fb3b..891eaa2b4b 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -44,47 +44,40 @@\n /* The maximum size for an object header. */\n #define MAX_HEADER_LEN 32\n \n-\n-#define EMPTY_TREE_SHA1_BIN_LITERAL { \\\n-\t0x4b, 0x82, 0x5d, 0xc6, 0x42, 0xcb, 0x6e, 0xb9, 0xa0, 0x60, \\\n-\t0xe5, 0x4b, 0xf8, 0xd6, 0x92, 0x88, 0xfb, 0xee, 0x49, 0x04 \\\n-}\n-#define EMPTY_TREE_SHA256_BIN_LITERAL { \\\n-\t0x6e, 0xf1, 0x9b, 0x41, 0x22, 0x5c, 0x53, 0x69, 0xf1, 0xc1, \\\n-\t0x04, 0xd4, 0x5d, 0x8d, 0x85, 0xef, 0xa9, 0xb0, 0x57, 0xb5, \\\n-\t0x3b, 0x14, 0xb4, 0xb9, 0xb9, 0x39, 0xdd, 0x74, 0xde, 0xcc, \\\n-\t0x53, 0x21 \\\n-}\n-\n-#define EMPTY_BLOB_SHA1_BIN_LITERAL { \\\n-\t0xe6, 0x9d, 0xe2, 0x9b, 0xb2, 0xd1, 0xd6, 0x43, 0x4b, 0x8b, \\\n-\t0x29, 0xae, 0x77, 0x5a, 0xd8, 0xc2, 0xe4, 0x8c, 0x53, 0x91 \\\n-}\n-#define EMPTY_BLOB_SHA256_BIN_LITERAL { \\\n-\t0x47, 0x3a, 0x0f, 0x4c, 0x3b, 0xe8, 0xa9, 0x36, 0x81, 0xa2, \\\n-\t0x67, 0xe3, 0xb1, 0xe9, 0xa7, 0xdc, 0xda, 0x11, 0x85, 0x43, \\\n-\t0x6f, 0xe1, 0x41, 0xf7, 0x74, 0x91, 0x20, 0xa3, 0x03, 0x72, \\\n-\t0x18, 0x13 \\\n-}\n-\n static const struct object_id empty_tree_oid = {\n-\t.hash = EMPTY_TREE_SHA1_BIN_LITERAL,\n+\t.hash = {\n+\t\t0x4b, 0x82, 0x5d, 0xc6, 0x42, 0xcb, 0x6e, 0xb9, 0xa0, 0x60,\n+\t\t0xe5, 0x4b, 0xf8, 0xd6, 0x92, 0x88, 0xfb, 0xee, 0x49, 0x04\n+\t},\n \t.algo = GIT_HASH_SHA1,\n };\n static const struct object_id empty_blob_oid = {\n-\t.hash = EMPTY_BLOB_SHA1_BIN_LITERAL,\n+\t.hash = {\n+\t\t0xe6, 0x9d, 0xe2, 0x9b, 0xb2, 0xd1, 0xd6, 0x43, 0x4b, 0x8b,\n+\t\t0x29, 0xae, 0x77, 0x5a, 0xd8, 0xc2, 0xe4, 0x8c, 0x53, 0x91\n+\t},\n \t.algo = GIT_HASH_SHA1,\n };\n static const struct object_id null_oid_sha1 = {\n \t.hash = {0},\n \t.algo = GIT_HASH_SHA1,\n };\n static const struct object_id empty_tree_oid_sha256 = {\n-\t.hash = EMPTY_TREE_SHA256_BIN_LITERAL,\n+\t.hash = {\n+\t\t0x6e, 0xf1, 0x9b, 0x41, 0x22, 0x5c, 0x53, 0x69, 0xf1, 0xc1,\n+\t\t0x04, 0xd4, 0x5d, 0x8d, 0x85, 0xef, 0xa9, 0xb0, 0x57, 0xb5,\n+\t\t0x3b, 0x14, 0xb4, 0xb9, 0xb9, 0x39, 0xdd, 0x74, 0xde, 0xcc,\n+\t\t0x53, 0x21\n+\t},\n \t.algo = GIT_HASH_SHA256,\n };\n static const struct object_id empty_blob_oid_sha256 = {\n-\t.hash = EMPTY_BLOB_SHA256_BIN_LITERAL,\n+\t.hash = {\n+\t\t0x47, 0x3a, 0x0f, 0x4c, 0x3b, 0xe8, 0xa9, 0x36, 0x81, 0xa2,\n+\t\t0x67, 0xe3, 0xb1, 0xe9, 0xa7, 0xdc, 0xda, 0x11, 0x85, 0x43,\n+\t\t0x6f, 0xe1, 0x41, 0xf7, 0x74, 0x91, 0x20, 0xa3, 0x03, 0x72,\n+\t\t0x18, 0x13\n+\t},\n \t.algo = GIT_HASH_SHA256,\n };\n static const struct object_id null_oid_sha256 = {\n-- \n2.47.0.547.g778689293a\n"},{"id":"507488","messageId":"87v7wkkgmr.fsf@gentoo.org","threadId":"62506","inReplyTo":"20241118091948.GC3984843@coredump.intra.peff.net","subject":"Re: -Wunterminated-string-initialization warning with GCC 15 in object-file.c","fromName":"Sam James","fromEmail":"sam@gentoo.org","sentAt":"2024-11-18T09:58:36Z","receivedAt":"2024-11-18T09:58:40Z","isPatch":false,"sender":{"key":"sam@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/11667869?v=4"},"body":"Jeff King writes:\n\n> On Sun, Nov 17, 2024 at 04:03:31PM +0000, brian m. carlson wrote:\n>\n>> On 2024-11-17 at 09:03:29, Jeff King wrote:\n>> > Here are some patches. The first one should fix the warning (but I don't\n>> > have gcc-15 handy to test!). Please let me know if it works for you (and\n>> > thank you for reporting).\n>> \n>> Just so you know, since I believe you also use Debian unstable, you can\n>> install the gcc-snapshot package (which is, admittedly, rather large)\n>> and use `CC=/usr/lib/gcc-snapshot/bin/gcc`.\n>\n> Thanks, I was stupidly looking for a \"gcc-15\" package in experimental,\n> not realizing it had not actually been released yet. I reproduced the\n> problem with the snapshot (which is from 20241004) and verified that my\n> series fixes it.\n\nSorry for not saying that explicitly -- I always try to balance some\nlong blurb of background and FYIs with not being verbose :(\n\nI'll include it in future reports, sorry again!\n\n>\n> -Peff\n"},{"id":"507490","messageId":"ZzscKHq6HN0pThV_@pks.im","threadId":"62506","inReplyTo":"20241118095423.GA3990835@coredump.intra.peff.net","subject":"Re: [PATCH 0/6] -Wunterminated-string-initialization warning + cleanups","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-18T10:51:20Z","receivedAt":"2024-11-18T10:51:35Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Nov 18, 2024 at 04:54:23AM -0500, Jeff King wrote:\n> > Here are some patches. The first one should fix the warning (but I don't\n> > have gcc-15 handy to test!). Please let me know if it works for you (and\n> > thank you for reporting).\n> \n> And here's a minor re-roll from comments on the list. I was able to\n> reproduce and test myself this time; the patch indeed fixes the problem.\n\nThanks, this version looks good to me!\n\nPatrick\n"},{"id":"507494","messageId":"xmqqbjyck8p6.fsf@gitster.g","threadId":"62506","inReplyTo":"ZzscKHq6HN0pThV_@pks.im","subject":"Re: [PATCH 0/6] -Wunterminated-string-initialization warning + cleanups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-11-18T12:49:57Z","receivedAt":"2024-11-18T12:50:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Mon, Nov 18, 2024 at 04:54:23AM -0500, Jeff King wrote:\n>> > Here are some patches. The first one should fix the warning (but I don't\n>> > have gcc-15 handy to test!). Please let me know if it works for you (and\n>> > thank you for reporting).\n>> \n>> And here's a minor re-roll from comments on the list. I was able to\n>> reproduce and test myself this time; the patch indeed fixes the problem.\n>\n> Thanks, this version looks good to me!\n\nThanks, both. Queued.\n"}]}