{"thread":{"id":"64546","subject":"[PATCH] last-modified: fix bug caused by inproper initialized memory","startedAt":"2025-11-28T16:37:30Z","lastAt":"2025-12-09T12:18:09Z","messageCount":14,"participants":["Toon Claes","Jeff King","Anders Kaseorg","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"531402","messageId":"20251128-toon-big-endian-ci-v1-1-80da0f629c1e@iotcl.com","threadId":"64546","inReplyTo":null,"subject":"[PATCH] last-modified: fix bug caused by inproper initialized memory","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-11-28T16:37:13Z","receivedAt":"2025-11-28T16:37:30Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"git-last-modified(1) uses a scratch bitmap to keep track of paths that\nhave been changed between commits. To avoid reallocating a bitmap on\neach call of process_parent(), the scratch bitmap is kept and reused.\nAlthough, it seems an incorrect length is passed to memset(3).\n\n`struct bitmap` uses `eword_t` to for internal storage. This type is\ntypedef'd to uint64_t. To fully zero the memory used by the bitmap,\nmultiply the length (saved in `struct bitmap::word_alloc`) by the size\nof `eword_t`.\n\nReported-by: Anders Kaseorg \nHelped-by: Jeff King \nSigned-off-by: Toon Claes \n---\nIt was reported [1] the tests in t8020 fail on s390x. After some\nresearch, it seems it was related to s390x being big-endian. Well,\nactually, not really. Using big-endian simply uncovered the problem in\ntest.\n\n[1]: https://lore.kernel.org/git/4dc4c8cd-c0cc-4784-8fcf-defa3a051087@mit.edu/\n---\n builtin/last-modified.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex b0ecbdc540..cc5fd2e795 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -327,7 +327,7 @@ static void process_parent(struct last_modified *lm,\n \tif (!(parent->object.flags & PARENT1))\n \t\tactive_paths_free(lm, parent);\n \n-\tmemset(lm->scratch->words, 0x0, lm->scratch->word_alloc);\n+\tmemset(lm->scratch->words, 0x0, lm->scratch->word_alloc * sizeof(eword_t));\n \tdiff_queue_clear(&diff_queued_diff);\n }\n \n\n---\nbase-commit: 6ab38b7e9cc7adafc304f3204616a4debd49c6e9\nchange-id: 20251126-toon-big-endian-ci-fe62bb361974\n\n"},{"id":"531411","messageId":"20251128205514.GA605489@coredump.intra.peff.net","threadId":"64546","inReplyTo":"20251128-toon-big-endian-ci-v1-1-80da0f629c1e@iotcl.com","subject":"Re: [PATCH] last-modified: fix bug caused by inproper initialized memory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-11-28T20:55:14Z","receivedAt":"2025-11-28T20:55:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 28, 2025 at 05:37:13PM +0100, Toon Claes wrote:\n\n> git-last-modified(1) uses a scratch bitmap to keep track of paths that\n> have been changed between commits. To avoid reallocating a bitmap on\n> each call of process_parent(), the scratch bitmap is kept and reused.\n> Although, it seems an incorrect length is passed to memset(3).\n> \n> `struct bitmap` uses `eword_t` to for internal storage. This type is\n> typedef'd to uint64_t. To fully zero the memory used by the bitmap,\n> multiply the length (saved in `struct bitmap::word_alloc`) by the size\n> of `eword_t`.\n\nGood catch! When I was looking for casts that could be the culprit, I\ndidn't think about the implicit one we get through the void pointer of\nmemset().\n\n> diff --git a/builtin/last-modified.c b/builtin/last-modified.c\n> index b0ecbdc540..cc5fd2e795 100644\n> --- a/builtin/last-modified.c\n> +++ b/builtin/last-modified.c\n> @@ -327,7 +327,7 @@ static void process_parent(struct last_modified *lm,\n> \tif (!(parent->object.flags & PARENT1))\n> \t\tactive_paths_free(lm, parent);\n> \n> -\tmemset(lm->scratch->words, 0x0, lm->scratch->word_alloc);\n> +\tmemset(lm->scratch->words, 0x0, lm->scratch->word_alloc * sizeof(eword_t));\n> \tdiff_queue_clear(&diff_queued_diff);\n> }\n\nI think this patch makes sense as the most obvious and immediate fix.\nBut thinking on how we might have avoided this bug:\n\n - We have macros like ALLOC_ARRAY() and COPY_ARRAY() that\n automatically multiply the array length by the size of each element\n (by looking at the type of the array). We could in theory have a\n helper like:\n\n MEMSET_ARRAY(lm->scratch->words, 0x0, lm->scratch->word_alloc);\n\n that would have made this hard to get wrong. But that's actually a\n bit of a funny interface, because memset is inherently byte-oriented\n under the hood. So we are not setting each element to 0x0, but\n rather each byte. For a value of 0x0, that is the same thing. But if\n you chose, say \"0x1\", it is not.\n\n So it would probably have to be limited to something like:\n\n CLEAR_ARRAY(lm->scratch->words, lm->scratch->word_alloc);\n\n which I'd guess would cover most memset cases. But this is getting\n specific enough that maybe the macro is making things more confusing\n rather than less.\n\n - It's a little gross that we are reaching inside a \"struct bitmap\" in\n the first place, as it's a mostly opaque type. And the code here has\n to know that the alloc field is sized in eword_t's, not in bytes.\n\n It feels like there should be a bitmap_clear() function. Its\n implementation would also have to remember to multiply by\n sizeof(eword_t), but at least it would be encapsulated.\n\n I doubt the leaky abstraction matters that much, though. It seems\n unlikely that we would change it (and if we did, we'd perhaps give\n the field a new name).\n\n In the same vein, probably using \"sizeof(lm->scratch->words)\" is\n better than \"sizeof(eword_t)\". But again, I find it an unlikely\n detail for us to catch under the hood.\n\n-Peff\n"},{"id":"531413","messageId":"5699f2cc-5157-441e-af98-4d8df492ec72@mit.edu","threadId":"64546","inReplyTo":"20251128205514.GA605489@coredump.intra.peff.net","subject":"Re: [PATCH] last-modified: fix bug caused by inproper initialized memory","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2025-11-28T22:20:22Z","receivedAt":"2025-11-28T22:20:37Z","isPatch":true,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"On 11/28/25 12:55, Jeff King wrote:\n> In the same vein, probably using \"sizeof(lm->scratch->words)\" is \n> better than \"sizeof(eword_t)\". But again, I find it an unlikely \n> detail for us to catch under the hood.\n\nAs words is a pointer, you must have meant sizeof *lm->scratch->words or \nsizeof lm->scratch->words[0].\n\nAnders\n\n"},{"id":"531420","messageId":"xmqq8qfpioln.fsf@gitster.g","threadId":"64546","inReplyTo":"20251128-toon-big-endian-ci-v1-1-80da0f629c1e@iotcl.com","subject":"Re: [PATCH] last-modified: fix bug caused by inproper initialized memory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-29T02:01:24Z","receivedAt":"2025-11-29T02:01:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Toon Claes writes:\n\n> git-last-modified(1) uses a scratch bitmap to keep track of paths that\n> have been changed between commits. To avoid reallocating a bitmap on\n> each call of process_parent(), the scratch bitmap is kept and reused.\n> Although, it seems an incorrect length is passed to memset(3).\n>\n> `struct bitmap` uses `eword_t` to for internal storage. This type is\n> typedef'd to uint64_t. To fully zero the memory used by the bitmap,\n> multiply the length (saved in `struct bitmap::word_alloc`) by the size\n> of `eword_t`.\n>\n> Reported-by: Anders Kaseorg \n> Helped-by: Jeff King \n> Signed-off-by: Toon Claes \n> ---\n> It was reported [1] the tests in t8020 fail on s390x. After some\n> research, it seems it was related to s390x being big-endian. Well,\n> actually, not really. Using big-endian simply uncovered the problem in\n> test.\n>\n> [1]: https://lore.kernel.org/git/4dc4c8cd-c0cc-4784-8fcf-defa3a051087@mit.edu/\n> ---\n> builtin/last-modified.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n\nThis dates back to v2.52.0~4 and is clearly a maint material.\n\nThanks for finding and fixing.\n\n>\n> diff --git a/builtin/last-modified.c b/builtin/last-modified.c\n> index b0ecbdc540..cc5fd2e795 100644\n> --- a/builtin/last-modified.c\n> +++ b/builtin/last-modified.c\n> @@ -327,7 +327,7 @@ static void process_parent(struct last_modified *lm,\n> \tif (!(parent->object.flags & PARENT1))\n> \t\tactive_paths_free(lm, parent);\n> \n> -\tmemset(lm->scratch->words, 0x0, lm->scratch->word_alloc);\n> +\tmemset(lm->scratch->words, 0x0, lm->scratch->word_alloc * sizeof(eword_t));\n> \tdiff_queue_clear(&diff_queued_diff);\n> }\n> \n>\n> ---\n> base-commit: 6ab38b7e9cc7adafc304f3204616a4debd49c6e9\n> change-id: 20251126-toon-big-endian-ci-fe62bb361974\n"},{"id":"531421","messageId":"xmqqwm39h9kb.fsf@gitster.g","threadId":"64546","inReplyTo":"xmqq8qfpioln.fsf@gitster.g","subject":"Re: [PATCH] last-modified: fix bug caused by inproper initialized memory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-29T02:11:32Z","receivedAt":"2025-11-29T02:11:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> This dates back to v2.52.0~4 and is clearly a maint material.\n>\n> Thanks for finding and fixing.\n\n> Subject: Re: [PATCH] last-modified: fix bug caused by inproper initialized memory\n\nLet's retitle, as inproper is not a word. Is\n\n Subject: [PATCH] last-modified: fix use of uninitialized memory\n\ngood enough?\n"},{"id":"531426","messageId":"87ms4518n1.fsf@iotcl.com","threadId":"64546","inReplyTo":"xmqqwm39h9kb.fsf@gitster.g","subject":"Re: [PATCH] last-modified: fix bug caused by inproper initialized memory","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-11-29T09:38:10Z","receivedAt":"2025-11-29T09:38:41Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Junio C Hamano writes:\n\n> Junio C Hamano writes:\n>\n>> This dates back to v2.52.0~4 and is clearly a maint material.\n\nMakes sense. I appreciate it.\n\n>> Thanks for finding and fixing.\n\nYes, I'm happy Anders reported this, although I didn't expect it to have\nimpact on all platforms. It would have been a nasty bug to hunt down if\nusers would complain \"the results are incorrect\".\n\n>> Subject: Re: [PATCH] last-modified: fix bug caused by inproper initialized memory\n>\n> Let's retitle, as inproper is not a word. Is\n\nI wasn't sure about that. But my spell checker didn't pick it up, so I\nrolled with it.\n\n> Subject: [PATCH] last-modified: fix use of uninitialized memory\n>\n> good enough?\n\nAbsolutely.\n\n\n-- \nCheers,\nToon\n"},{"id":"531427","messageId":"20251129105023.GA646133@coredump.intra.peff.net","threadId":"64546","inReplyTo":"5699f2cc-5157-441e-af98-4d8df492ec72@mit.edu","subject":"Re: [PATCH] last-modified: fix bug caused by inproper initialized memory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-11-29T10:50:23Z","receivedAt":"2025-11-29T10:50:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 28, 2025 at 02:20:22PM -0800, Anders Kaseorg wrote:\n\n> On 11/28/25 12:55, Jeff King wrote:\n> > In the same vein, probably using \"sizeof(lm->scratch->words)\" is better\n> > than \"sizeof(eword_t)\". But again, I find it an unlikely detail for us\n> > to catch under the hood.\n> \n> As words is a pointer, you must have meant sizeof *lm->scratch->words or\n> sizeof lm->scratch->words[0].\n\nWhoops, yes. I prefer sizeof(*var) over sizeof(type) because it tracks\nchanges to the type of \"var\" automatically. But the opportunity to\nforget the \"*\" is perhaps a point against it. :)\n\n-Peff\n"},{"id":"531827","messageId":"20251208-toon-big-endian-ci-v2-1-76b46763a597@iotcl.com","threadId":"64546","inReplyTo":"20251128-toon-big-endian-ci-v1-1-80da0f629c1e@iotcl.com","subject":"[PATCH v2] last-modified: fix use of uninitialized memory","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-12-08T11:46:05Z","receivedAt":"2025-12-08T11:46:15Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"git-last-modified(1) uses a scratch bitmap to keep track of paths that\nhave been changed between commits. To avoid reallocating a bitmap on\neach call of process_parent(), the scratch bitmap is kept and reused.\nAlthough, between loops, the memory allocated for the 'scratch' bitmap\nisn't correctly wiped.\n\n`struct bitmap` uses `eword_t` to for internal storage. This type is\ntypedef'd to uint64_t. To fully zero the memory used by the bitmap, the\nlength (saved in `struct bitmap::word_alloc`) should be multiplied by\nthe size of a single item. To simplify zeroing an array, a macro\nMEMZERO_ARRAY() is defined and used.\n\nReported-by: Anders Kaseorg \nHelped-by: Jeff King \nSigned-off-by: Toon Claes \n---\nIt was reported [1] the tests in t8020 fail on s390x. After some\nresearch, it seems it was related to s390x being big-endian. Well,\nactually, not really. Using big-endian simply uncovered the problem in\ntest.\n\n[1]: https://lore.kernel.org/git/4dc4c8cd-c0cc-4784-8fcf-defa3a051087@mit.edu/\n---\nChanges in v2:\n- Defined and used MEMZERO_ARRAY() macro.\n- Fixed up title which used unexisting word\n- Link to v1: https://lore.kernel.org/r/20251128-toon-big-endian-ci-v1-1-80da0f629c1e@iotcl.com\n---\n builtin/last-modified.c | 2 +-\n git-compat-util.h | 1 +\n 2 files changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex b0ecbdc540..ac5387e861 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -327,7 +327,7 @@ static void process_parent(struct last_modified *lm,\n \tif (!(parent->object.flags & PARENT1))\n \t\tactive_paths_free(lm, parent);\n \n-\tmemset(lm->scratch->words, 0x0, lm->scratch->word_alloc);\n+\tMEMZERO_ARRAY(lm->scratch->words, lm->scratch->word_alloc);\n \tdiff_queue_clear(&diff_queued_diff);\n }\n \ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 398e0fac4f..2b8192fd2e 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -726,6 +726,7 @@ static inline uint64_t u64_add(uint64_t a, uint64_t b)\n #define ALLOC_ARRAY(x, alloc) (x) = xmalloc(st_mult(sizeof(*(x)), (alloc)))\n #define CALLOC_ARRAY(x, alloc) (x) = xcalloc((alloc), sizeof(*(x)))\n #define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), st_mult(sizeof(*(x)), (alloc)))\n+#define MEMZERO_ARRAY(x, alloc) memset((x), 0x0, st_mult(sizeof(*(x)), (alloc)))\n \n #define COPY_ARRAY(dst, src, n) copy_array((dst), (src), (n), sizeof(*(dst)) + \\\n \tBARF_UNLESS_COPYABLE((dst), (src)))\n\n---\nbase-commit: bdc5341ff65278a3cc80b2e8a02a2f02aa1fac06\nchange-id: 20251126-toon-big-endian-ci-fe62bb361974\n\n"},{"id":"531829","messageId":"87bjk9w5yv.fsf@iotcl.com","threadId":"64546","inReplyTo":"20251128205514.GA605489@coredump.intra.peff.net","subject":"Re: [PATCH] last-modified: fix bug caused by inproper initialized memory","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-12-08T11:47:20Z","receivedAt":"2025-12-08T11:47:52Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Jeff King writes:\n\n> I think this patch makes sense as the most obvious and immediate fix.\n> But thinking on how we might have avoided this bug:\n>\n> - We have macros like ALLOC_ARRAY() and COPY_ARRAY() that\n> automatically multiply the array length by the size of each element\n> (by looking at the type of the array). We could in theory have a\n> helper like:\n>\n> MEMSET_ARRAY(lm->scratch->words, 0x0, lm->scratch->word_alloc);\n>\n> that would have made this hard to get wrong. But that's actually a\n> bit of a funny interface, because memset is inherently byte-oriented\n> under the hood. So we are not setting each element to 0x0, but\n> rather each byte. For a value of 0x0, that is the same thing. But if\n> you chose, say \"0x1\", it is not.\n>\n> So it would probably have to be limited to something like:\n>\n> CLEAR_ARRAY(lm->scratch->words, lm->scratch->word_alloc);\n>\n> which I'd guess would cover most memset cases. But this is getting\n> specific enough that maybe the macro is making things more confusing\n> rather than less.\n\nI've submitted a v2 that introduces MEMZERO_ARRAY(). I'm curious what\nthe responses on this proposal are?\n\n-- \nCheers,\nToon\n"},{"id":"531831","messageId":"xmqqikehkstt.fsf@gitster.g","threadId":"64546","inReplyTo":"20251208-toon-big-endian-ci-v2-1-76b46763a597@iotcl.com","subject":"Re: [PATCH v2] last-modified: fix use of uninitialized memory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-12-08T13:26:38Z","receivedAt":"2025-12-08T13:26:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Toon Claes writes:\n\n> Changes in v2:\n> - Defined and used MEMZERO_ARRAY() macro.\n> - Fixed up title which used unexisting word\n> - Link to v1: https://lore.kernel.org/r/20251128-toon-big-endian-ci-v1-1-80da0f629c1e@iotcl.com\n\nSorry, but hasn't the old one already been cooking in 'next'?\n\n> ---\n> builtin/last-modified.c | 2 +-\n> git-compat-util.h | 1 +\n> 2 files changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/last-modified.c b/builtin/last-modified.c\n> index b0ecbdc540..ac5387e861 100644\n> --- a/builtin/last-modified.c\n> +++ b/builtin/last-modified.c\n> @@ -327,7 +327,7 @@ static void process_parent(struct last_modified *lm,\n> \tif (!(parent->object.flags & PARENT1))\n> \t\tactive_paths_free(lm, parent);\n> \n> -\tmemset(lm->scratch->words, 0x0, lm->scratch->word_alloc);\n> +\tMEMZERO_ARRAY(lm->scratch->words, lm->scratch->word_alloc);\n> \tdiff_queue_clear(&diff_queued_diff);\n> }\n> \n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index 398e0fac4f..2b8192fd2e 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -726,6 +726,7 @@ static inline uint64_t u64_add(uint64_t a, uint64_t b)\n> #define ALLOC_ARRAY(x, alloc) (x) = xmalloc(st_mult(sizeof(*(x)), (alloc)))\n> #define CALLOC_ARRAY(x, alloc) (x) = xcalloc((alloc), sizeof(*(x)))\n> #define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), st_mult(sizeof(*(x)), (alloc)))\n> +#define MEMZERO_ARRAY(x, alloc) memset((x), 0x0, st_mult(sizeof(*(x)), (alloc)))\n> \n> #define COPY_ARRAY(dst, src, n) copy_array((dst), (src), (n), sizeof(*(dst)) + \\\n> \tBARF_UNLESS_COPYABLE((dst), (src)))\n>\n> ---\n> base-commit: bdc5341ff65278a3cc80b2e8a02a2f02aa1fac06\n> change-id: 20251126-toon-big-endian-ci-fe62bb361974\n"},{"id":"531857","messageId":"20251208201501.GA216526@coredump.intra.peff.net","threadId":"64546","inReplyTo":"87bjk9w5yv.fsf@iotcl.com","subject":"Re: [PATCH] last-modified: fix bug caused by inproper initialized memory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-12-08T20:15:01Z","receivedAt":"2025-12-08T20:15:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 08, 2025 at 12:47:20PM +0100, Toon Claes wrote:\n\n> > So it would probably have to be limited to something like:\n> >\n> > CLEAR_ARRAY(lm->scratch->words, lm->scratch->word_alloc);\n> >\n> > which I'd guess would cover most memset cases. But this is getting\n> > specific enough that maybe the macro is making things more confusing\n> > rather than less.\n> \n> I've submitted a v2 that introduces MEMZERO_ARRAY(). I'm curious what\n> the responses on this proposal are?\n\nI think it looks fine, though as Junio noted, the original is already in\nnext so it would have to be a patch on top.\n\nIs such a macro worth it? I guess we'd be able to see if there are other\npossible sites with something like:\n\n git grep 'memset(.*0,.*\\* \\?sizeof'\n\nthat's looking for memsets of \"0\" that also multiply by sizeof. Looks\nlike there are a few:\n\n add-patch.c: memset(hunk + 1, 0, (splittable_into - 1) * sizeof(*hunk));\n builtin/last-modified.c: memset(lm->scratch->words, 0x0, lm->scratch->word_alloc * sizeof(eword_t));\n compat/simple-ipc/ipc-win32.c: memset(ea, 0, NR_EA * sizeof(EXPLICIT_ACCESS));\n diff-delta.c: memset(hash, 0, hsize * sizeof(*hash));\n hashmap.c: memset(map->table, 0, map->tablesize * sizeof(struct hashmap_entry *));\n pack-revindex.c: memset(pos, 0, BUCKETS * sizeof(*pos));\n\nThe first one is an oddball, but the other five could use it. So if we\nwere to do a patch adding MEMZERO_ARRAY(), it would probably make sense\nto convert those spots. I'd be OK either way.\n\n-Peff\n"},{"id":"531863","messageId":"xmqq5xaglhn3.fsf@gitster.g","threadId":"64546","inReplyTo":"20251208201501.GA216526@coredump.intra.peff.net","subject":"Re: [PATCH] last-modified: fix bug caused by inproper initialized memory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-12-08T22:42:56Z","receivedAt":"2025-12-08T22:42:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> git grep 'memset(.*0,.*\\* \\?sizeof'\n>\n> that's looking for memsets of \"0\" that also multiply by sizeof. Looks\n> like there are a few:\n>\n> add-patch.c: memset(hunk + 1, 0, (splittable_into - 1) * sizeof(*hunk));\n> builtin/last-modified.c: memset(lm->scratch->words, 0x0, lm->scratch->word_alloc * sizeof(eword_t));\n> compat/simple-ipc/ipc-win32.c: memset(ea, 0, NR_EA * sizeof(EXPLICIT_ACCESS));\n> diff-delta.c: memset(hash, 0, hsize * sizeof(*hash));\n> hashmap.c: memset(map->table, 0, map->tablesize * sizeof(struct hashmap_entry *));\n> pack-revindex.c: memset(pos, 0, BUCKETS * sizeof(*pos));\n>\n> The first one is an oddball, but the other five could use it. So if we\n> were to do a patch adding MEMZERO_ARRAY(), it would probably make sense\n> to convert those spots. I'd be OK either way.\n\nThanks for making an excellent suggestion while I was away from the\nkeyboard ;-)\n\nBetween MEMZERO_ARRAY() and CLEAR_ARRAY(), I am on the fence.\n"},{"id":"531896","messageId":"871pl4vyd5.fsf@iotcl.com","threadId":"64546","inReplyTo":"xmqqikehkstt.fsf@gitster.g","subject":"Re: [PATCH v2] last-modified: fix use of uninitialized memory","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-12-09T08:43:50Z","receivedAt":"2025-12-09T08:44:03Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Junio C Hamano writes:\n\n> Sorry, but hasn't the old one already been cooking in 'next'?\n\nOkay, fine by me. Let's abandon this v2 then.\n\n-- \nCheers,\nToon\n"},{"id":"531898","messageId":"xmqqpl8nkfwh.fsf@gitster.g","threadId":"64546","inReplyTo":"871pl4vyd5.fsf@iotcl.com","subject":"Re: [PATCH v2] last-modified: fix use of uninitialized memory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-12-09T12:18:06Z","receivedAt":"2025-12-09T12:18:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Toon Claes writes:\n\n> Junio C Hamano writes:\n>\n>> Sorry, but hasn't the old one already been cooking in 'next'?\n>\n> Okay, fine by me. Let's abandon this v2 then.\n\nUnderstood. I however agree with Patrick that rewriting\n\n memset(ptr, '\\0', sizeof(*ptr) * nr)\n\nto use CLEAR_ARRAY(), not limited to last-modified but everywhere in\nthe codebase, may not be a bad idea. It would be a good exercise to\nhone our Coccinelle skill ;-)\n"}]}