* [PATCH 0/2] Add MEMZERO_ARRAY() macro and use it in coccinelle
@ 2025-12-10 13:13 Toon Claes
2025-12-10 13:13 ` [PATCH 1/2] git-compat-util: introduce MEMZERO_ARRAY() macro Toon Claes
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Toon Claes @ 2025-12-10 13:13 UTC (permalink / raw)
To: git; +Cc: Jeff King, Toon Claes
A bug was found[1] in git-last-modified(1), caused by uninitialized
memory. While the bug is fixed, in the discussion after that[2] the
suggestion was made to introduce a macro that simplifies zeroing a
dynamically allocated array.
In the first patch I'm addressing the outcome of the discussion on the
patch, and in the second patch I'm fixing an edge-case I've encountered
while using coccinelle.
There's one /oddball/ in add-patch.c that doesn't get caught by the
coccinelle rules, around line 960:
memset(hunk + 1, 0, (splittable_into - 1) * sizeof(*hunk));
Because there's some quirky pointer math going on, it think it's better
to keep it like it is.
There were some mixed opinions about naming it either CLEAR_ARRAY() or
MEMZERO_ARRAY(). I choose the latter because I wanted to avoid confusion
that "clear" would shrink the array to zero elements.
[1]: <4dc4c8cd-c0cc-4784-8fcf-defa3a051087@mit.edu>
[2]: <20251208201501.GA216526@coredump.intra.peff.net>
Signed-off-by: Toon Claes <toon@iotcl.com>
---
Toon Claes (2):
git-compat-util: introduce MEMZERO_ARRAY() macro
contrib/coccinelle: pass include paths to spatch(1)
Makefile | 2 +-
builtin/last-modified.c | 2 +-
compat/simple-ipc/ipc-win32.c | 2 +-
contrib/coccinelle/array.cocci | 20 ++++++++++++++++++++
contrib/coccinelle/meson.build | 6 ++++++
diff-delta.c | 2 +-
ewah/bitmap.c | 7 +++----
git-compat-util.h | 1 +
hashmap.c | 2 +-
pack-revindex.c | 2 +-
10 files changed, 36 insertions(+), 10 deletions(-)
---
base-commit: 011ce54c26318d725db1d8971d157656eb965d88
change-id: 20251210-toon-cocci-memzero-2b6185e08ac4
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/2] git-compat-util: introduce MEMZERO_ARRAY() macro 2025-12-10 13:13 [PATCH 0/2] Add MEMZERO_ARRAY() macro and use it in coccinelle Toon Claes @ 2025-12-10 13:13 ` Toon Claes 2025-12-11 3:18 ` Junio C Hamano 2025-12-10 13:13 ` [PATCH 2/2] contrib/coccinelle: pass include paths to spatch(1) Toon Claes 2025-12-13 0:18 ` [PATCH 0/2] Add MEMZERO_ARRAY() macro and use it in coccinelle Junio C Hamano 2 siblings, 1 reply; 10+ messages in thread From: Toon Claes @ 2025-12-10 13:13 UTC (permalink / raw) To: git; +Cc: Jeff King, Toon Claes Introduce a new macro MEMZERO_ARRAY() that zeroes the memory allocated by ALLOC_ARRAY() and friends. And add coccinelle rule to enforce the use of this macro. Signed-off-by: Toon Claes <toon@iotcl.com> --- builtin/last-modified.c | 2 +- compat/simple-ipc/ipc-win32.c | 2 +- contrib/coccinelle/array.cocci | 20 ++++++++++++++++++++ diff-delta.c | 2 +- ewah/bitmap.c | 7 +++---- git-compat-util.h | 1 + hashmap.c | 2 +- pack-revindex.c | 2 +- 8 files changed, 29 insertions(+), 9 deletions(-) diff --git a/builtin/last-modified.c b/builtin/last-modified.c index cc5fd2e795..ac5387e861 100644 --- a/builtin/last-modified.c +++ b/builtin/last-modified.c @@ -327,7 +327,7 @@ static void process_parent(struct last_modified *lm, if (!(parent->object.flags & PARENT1)) active_paths_free(lm, parent); - memset(lm->scratch->words, 0x0, lm->scratch->word_alloc * sizeof(eword_t)); + MEMZERO_ARRAY(lm->scratch->words, lm->scratch->word_alloc); diff_queue_clear(&diff_queued_diff); } diff --git a/compat/simple-ipc/ipc-win32.c b/compat/simple-ipc/ipc-win32.c index a8fc812adf..4a3e7df9c7 100644 --- a/compat/simple-ipc/ipc-win32.c +++ b/compat/simple-ipc/ipc-win32.c @@ -686,7 +686,7 @@ static LPSECURITY_ATTRIBUTES get_sa(struct my_sa_data *d) goto fail; } - memset(ea, 0, NR_EA * sizeof(EXPLICIT_ACCESS)); + MEMZERO_ARRAY(ea, NR_EA); ea[0].grfAccessPermissions = GENERIC_READ | GENERIC_WRITE; ea[0].grfAccessMode = SET_ACCESS; diff --git a/contrib/coccinelle/array.cocci b/contrib/coccinelle/array.cocci index 27a3b479c9..d306f6a21e 100644 --- a/contrib/coccinelle/array.cocci +++ b/contrib/coccinelle/array.cocci @@ -101,3 +101,23 @@ expression dst, src, n; -ALLOC_ARRAY(dst, n); -COPY_ARRAY(dst, src, n); +DUP_ARRAY(dst, src, n); + +@@ +type T; +T *ptr; +expression n; +@@ +- memset(ptr, \( 0x0 \| 0 \), n * \( sizeof(T) +- \| sizeof(*ptr) +- \) ) ++ MEMZERO_ARRAY(ptr, n) + +@@ +type T; +T[] ptr; +expression n; +@@ +- memset(ptr, \( 0x0 \| 0 \), n * \( sizeof(T) +- \| sizeof(*ptr) +- \) ) ++ MEMZERO_ARRAY(ptr, n) diff --git a/diff-delta.c b/diff-delta.c index 71d37368d6..43c339f010 100644 --- a/diff-delta.c +++ b/diff-delta.c @@ -171,7 +171,7 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize) mem = hash + hsize; entry = mem; - memset(hash, 0, hsize * sizeof(*hash)); + MEMZERO_ARRAY(hash, hsize); /* allocate an array to count hash entries */ hash_count = calloc(hsize, sizeof(*hash_count)); diff --git a/ewah/bitmap.c b/ewah/bitmap.c index 55928dada8..bf878bf876 100644 --- a/ewah/bitmap.c +++ b/ewah/bitmap.c @@ -46,8 +46,7 @@ static void bitmap_grow(struct bitmap *self, size_t word_alloc) { size_t old_size = self->word_alloc; ALLOC_GROW(self->words, word_alloc, self->word_alloc); - memset(self->words + old_size, 0x0, - (self->word_alloc - old_size) * sizeof(eword_t)); + MEMZERO_ARRAY(self->words + old_size, (self->word_alloc - old_size)); } void bitmap_set(struct bitmap *self, size_t pos) @@ -192,8 +191,8 @@ void bitmap_or_ewah(struct bitmap *self, struct ewah_bitmap *other) if (self->word_alloc < other_final) { self->word_alloc = other_final; REALLOC_ARRAY(self->words, self->word_alloc); - memset(self->words + original_size, 0x0, - (self->word_alloc - original_size) * sizeof(eword_t)); + MEMZERO_ARRAY(self->words + original_size, + (self->word_alloc - original_size)); } ewah_iterator_init(&it, other); diff --git a/git-compat-util.h b/git-compat-util.h index 398e0fac4f..2b8192fd2e 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -726,6 +726,7 @@ static inline uint64_t u64_add(uint64_t a, uint64_t b) #define ALLOC_ARRAY(x, alloc) (x) = xmalloc(st_mult(sizeof(*(x)), (alloc))) #define CALLOC_ARRAY(x, alloc) (x) = xcalloc((alloc), sizeof(*(x))) #define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), st_mult(sizeof(*(x)), (alloc))) +#define MEMZERO_ARRAY(x, alloc) memset((x), 0x0, st_mult(sizeof(*(x)), (alloc))) #define COPY_ARRAY(dst, src, n) copy_array((dst), (src), (n), sizeof(*(dst)) + \ BARF_UNLESS_COPYABLE((dst), (src))) diff --git a/hashmap.c b/hashmap.c index a711377853..3b5d6f14bc 100644 --- a/hashmap.c +++ b/hashmap.c @@ -194,7 +194,7 @@ void hashmap_partial_clear_(struct hashmap *map, ssize_t entry_offset) return; if (entry_offset >= 0) /* called by hashmap_clear_entries */ free_individual_entries(map, entry_offset); - memset(map->table, 0, map->tablesize * sizeof(struct hashmap_entry *)); + MEMZERO_ARRAY(map->table, map->tablesize); map->shrink_at = 0; map->private_size = 0; } diff --git a/pack-revindex.c b/pack-revindex.c index d0791cc493..8598b941c8 100644 --- a/pack-revindex.c +++ b/pack-revindex.c @@ -75,7 +75,7 @@ static void sort_revindex(struct revindex_entry *entries, unsigned n, off_t max) for (bits = 0; max >> bits; bits += DIGIT_SIZE) { unsigned i; - memset(pos, 0, BUCKETS * sizeof(*pos)); + MEMZERO_ARRAY(pos, BUCKETS); /* * We want pos[i] to store the index of the last element that -- 2.52.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] git-compat-util: introduce MEMZERO_ARRAY() macro 2025-12-10 13:13 ` [PATCH 1/2] git-compat-util: introduce MEMZERO_ARRAY() macro Toon Claes @ 2025-12-11 3:18 ` Junio C Hamano 2025-12-12 13:02 ` René Scharfe 0 siblings, 1 reply; 10+ messages in thread From: Junio C Hamano @ 2025-12-11 3:18 UTC (permalink / raw) To: Toon Claes; +Cc: git, Jeff King Toon Claes <toon@iotcl.com> writes: > +@@ > +- memset(ptr, \( 0x0 \| 0 \), n * \( sizeof(T) > +- \| sizeof(*ptr) > +- \) ) > ++ MEMZERO_ARRAY(ptr, n) Shouldn't we be also catching memset(array, '\0', sizeof(array[0]) * ARRAY_SIZE(array)); in addition to "0" and "0x0"? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] git-compat-util: introduce MEMZERO_ARRAY() macro 2025-12-11 3:18 ` Junio C Hamano @ 2025-12-12 13:02 ` René Scharfe 2025-12-19 9:17 ` Toon Claes 0 siblings, 1 reply; 10+ messages in thread From: René Scharfe @ 2025-12-12 13:02 UTC (permalink / raw) To: Junio C Hamano, Toon Claes; +Cc: git, Jeff King On 12/11/25 4:18 AM, Junio C Hamano wrote: > Toon Claes <toon@iotcl.com> writes: > >> +@@ >> +- memset(ptr, \( 0x0 \| 0 \), n * \( sizeof(T) >> +- \| sizeof(*ptr) >> +- \) ) >> ++ MEMZERO_ARRAY(ptr, n) > > Shouldn't we be also catching > > memset(array, '\0', sizeof(array[0]) * ARRAY_SIZE(array)); > > in addition to "0" and "0x0"? Good idea to match "sizeof(ptr[...])", even though we currently don't have matching code. Good idea also to match "'\0'". There's code with that pattern in compat/regex/. You can drop "0x0", though, "0" matches it already (at least for me, I have "spatch version 1.3-dirty compiled with OCaml version 5.1.1" from Homebrew). If you put parentheses around "n" in the pre-image then Coccinelle will remove them if present and still match code without them. They are no longer needed without the multiplication. Their removal would improve the result for ewah/bitmap.c. René ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] git-compat-util: introduce MEMZERO_ARRAY() macro 2025-12-12 13:02 ` René Scharfe @ 2025-12-19 9:17 ` Toon Claes 0 siblings, 0 replies; 10+ messages in thread From: Toon Claes @ 2025-12-19 9:17 UTC (permalink / raw) To: René Scharfe, Junio C Hamano; +Cc: git, Jeff King René Scharfe <l.s.r@web.de> writes: > On 12/11/25 4:18 AM, Junio C Hamano wrote: >> Toon Claes <toon@iotcl.com> writes: >> >>> +@@ >>> +- memset(ptr, \( 0x0 \| 0 \), n * \( sizeof(T) >>> +- \| sizeof(*ptr) >>> +- \) ) >>> ++ MEMZERO_ARRAY(ptr, n) >> >> Shouldn't we be also catching >> >> memset(array, '\0', sizeof(array[0]) * ARRAY_SIZE(array)); >> >> in addition to "0" and "0x0"? > > Good idea to match "sizeof(ptr[...])", even though we currently don't have > matching code. I didn't include that because I didn't see any case that would be covered by that. But it's good to include it anyway to capture future code. > Good idea also to match "'\0'". There's code with that pattern in > compat/regex/. Good find, I didn't think about that. > You can drop "0x0", though, "0" matches it already (at least for me, I have > "spatch version 1.3-dirty compiled with OCaml version 5.1.1" from > Homebrew). Nice! > If you put parentheses around "n" in the pre-image then Coccinelle will > remove them if present and still match code without them. Well, that's not what I am seeing. With parentheses around "n", it didn't match the case in builtin/last-modified.c on my machine. I'm using spatch v1.3 > They are no longer needed without the multiplication. Their removal > would improve the result for ewah/bitmap.c. I agree it would be an improvement. Using `\( (n) \| n \)` does the trick for all cases I've found. -- Cheers, Toon ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] contrib/coccinelle: pass include paths to spatch(1) 2025-12-10 13:13 [PATCH 0/2] Add MEMZERO_ARRAY() macro and use it in coccinelle Toon Claes 2025-12-10 13:13 ` [PATCH 1/2] git-compat-util: introduce MEMZERO_ARRAY() macro Toon Claes @ 2025-12-10 13:13 ` Toon Claes 2025-12-11 6:19 ` Patrick Steinhardt 2025-12-13 1:22 ` Junio C Hamano 2025-12-13 0:18 ` [PATCH 0/2] Add MEMZERO_ARRAY() macro and use it in coccinelle Junio C Hamano 2 siblings, 2 replies; 10+ messages in thread From: Toon Claes @ 2025-12-10 13:13 UTC (permalink / raw) To: git; +Cc: Jeff King, Toon Claes In the previous commit a new coccinelle rule is added. But neiter `make coccicheck` nor `meson compile coccicheck` did detect a case in builtin/last-modified.c. This case involves the field `scratch` in `struct last_modified`. This field is of type `struct bitmap` and that struct has a member `eword_t *words`. Both are defined in `ewah/ewok.h`. Now, while builtin/last-modified.c does include that header (with the subdir in the #include directive), it seems coccinelle does not process it. So it's unaware of the type of `words` in the bitmap, and it doesn't recognize the rule from previous commit that uses: type T; T *ptr; Fix coccicheck by passing all possible include paths inside the Git project so spatch(1) can find the headers and can determine the types. Signed-off-by: Toon Claes <toon@iotcl.com> --- Makefile | 2 +- contrib/coccinelle/meson.build | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 6fc322ff88..46d07b2d52 100644 --- a/Makefile +++ b/Makefile @@ -981,7 +981,7 @@ SANITIZE_LEAK = SANITIZE_ADDRESS = # For the 'coccicheck' target -SPATCH_INCLUDE_FLAGS = --all-includes +SPATCH_INCLUDE_FLAGS = --all-includes $(addprefix -I ,compat ewah refs sha256 trace2 win32 xdiff) SPATCH_FLAGS = SPATCH_TEST_FLAGS = diff --git a/contrib/coccinelle/meson.build b/contrib/coccinelle/meson.build index dc3f73c2e7..ae7f5b5460 100644 --- a/contrib/coccinelle/meson.build +++ b/contrib/coccinelle/meson.build @@ -50,6 +50,11 @@ foreach header : headers_to_check coccinelle_headers += meson.project_source_root() / header endforeach +coccinelle_includes = [] +foreach path : ['compat', 'ewah', 'refs', 'sha256', 'trace2', 'win32', 'xdiff'] + coccinelle_includes += ['-I', meson.project_source_root() / path] +endforeach + patches = [ ] foreach source : coccinelle_sources patches += custom_target( @@ -58,6 +63,7 @@ foreach source : coccinelle_sources '--all-includes', '--sp-file', concatenated_rules, '--patch', meson.project_source_root(), + coccinelle_includes, '@INPUT@', ], input: meson.project_source_root() / source, -- 2.52.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] contrib/coccinelle: pass include paths to spatch(1) 2025-12-10 13:13 ` [PATCH 2/2] contrib/coccinelle: pass include paths to spatch(1) Toon Claes @ 2025-12-11 6:19 ` Patrick Steinhardt 2025-12-12 5:04 ` Junio C Hamano 2025-12-13 1:22 ` Junio C Hamano 1 sibling, 1 reply; 10+ messages in thread From: Patrick Steinhardt @ 2025-12-11 6:19 UTC (permalink / raw) To: Toon Claes; +Cc: git, Jeff King On Wed, Dec 10, 2025 at 02:13:02PM +0100, Toon Claes wrote: > In the previous commit a new coccinelle rule is added. But neiter > `make coccicheck` nor `meson compile coccicheck` did detect a case in > builtin/last-modified.c. > > This case involves the field `scratch` in `struct last_modified`. This > field is of type `struct bitmap` and that struct has a member > `eword_t *words`. Both are defined in `ewah/ewok.h`. Now, while > builtin/last-modified.c does include that header (with the subdir in the > #include directive), it seems coccinelle does not process it. So it's > unaware of the type of `words` in the bitmap, and it doesn't recognize > the rule from previous commit that uses: > > type T; > T *ptr; > > Fix coccicheck by passing all possible include paths inside the Git > project so spatch(1) can find the headers and can determine the types. > > Signed-off-by: Toon Claes <toon@iotcl.com> > --- > Makefile | 2 +- > contrib/coccinelle/meson.build | 6 ++++++ > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index 6fc322ff88..46d07b2d52 100644 > --- a/Makefile > +++ b/Makefile > @@ -981,7 +981,7 @@ SANITIZE_LEAK = > SANITIZE_ADDRESS = > > # For the 'coccicheck' target > -SPATCH_INCLUDE_FLAGS = --all-includes > +SPATCH_INCLUDE_FLAGS = --all-includes $(addprefix -I ,compat ewah refs sha256 trace2 win32 xdiff) This feels weird to me. We never pass any of these includes to the compiler, either. So why should Coccinelle require them? Coming back to your example of `eword_t`, Git knows to always include "ewah/ewok.h", and that include is relative to the root directory of Git itself. And as the header doesn't have any includes itself, this cannot be the root cause, either. So I'm a bit puzzled why this patch would fix the observed issue. Patrick ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] contrib/coccinelle: pass include paths to spatch(1) 2025-12-11 6:19 ` Patrick Steinhardt @ 2025-12-12 5:04 ` Junio C Hamano 0 siblings, 0 replies; 10+ messages in thread From: Junio C Hamano @ 2025-12-12 5:04 UTC (permalink / raw) To: Patrick Steinhardt; +Cc: Toon Claes, git, Jeff King Patrick Steinhardt <ps@pks.im> writes: >> # For the 'coccicheck' target >> -SPATCH_INCLUDE_FLAGS = --all-includes >> +SPATCH_INCLUDE_FLAGS = --all-includes $(addprefix -I ,compat ewah refs sha256 trace2 win32 xdiff) > > This feels weird to me. We never pass any of these includes to the > compiler, either. So why should Coccinelle require them? > > Coming back to your example of `eword_t`, Git knows to always include > "ewah/ewok.h", and that include is relative to the root directory of Git > itself. And as the header doesn't have any includes itself, this cannot > be the root cause, either. > > So I'm a bit puzzled why this patch would fix the observed issue. Indeed it is puzzling.. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] contrib/coccinelle: pass include paths to spatch(1) 2025-12-10 13:13 ` [PATCH 2/2] contrib/coccinelle: pass include paths to spatch(1) Toon Claes 2025-12-11 6:19 ` Patrick Steinhardt @ 2025-12-13 1:22 ` Junio C Hamano 1 sibling, 0 replies; 10+ messages in thread From: Junio C Hamano @ 2025-12-13 1:22 UTC (permalink / raw) To: Toon Claes, René Scharfe; +Cc: git, Jeff King Toon Claes <toon@iotcl.com> writes: > In the previous commit a new coccinelle rule is added. But neiter > `make coccicheck` nor `meson compile coccicheck` did detect a case in > builtin/last-modified.c. I can reproduce this. I started with only git-compat-util.h and contrib/coccinelle/array.cocci from your [1/2] and without [2/2], and "make coccicheck" produced all other changes contained in [1/2] and the leftover changes to diffcore-delta.c, linear-assignment.c and shallow.c I reported earlier in a separate message, but the one in last-modified.c is left intact. There are successful rewrites that involve eword_t in other files, so I am not sure what the problem is. I used the following instead of your [1/2], as suggested by René in an earlier exchange. I did not see any changes but I did not expect to, either. contrib/coccinelle/array.cocci | 22 ++++++++++++++++++++++ git-compat-util.h | 1 + 2 files changed, 23 insertions(+) diff --git a/contrib/coccinelle/array.cocci b/contrib/coccinelle/array.cocci index 27a3b479c9..ae23114b68 100644 --- a/contrib/coccinelle/array.cocci +++ b/contrib/coccinelle/array.cocci @@ -101,3 +101,25 @@ expression dst, src, n; -ALLOC_ARRAY(dst, n); -COPY_ARRAY(dst, src, n); +DUP_ARRAY(dst, src, n); + +@@ +type T; +T *ptr; +expression n; +@@ +- memset(ptr, \( '\0' \| 0 \), n * \( sizeof(T) +- \| sizeof(*ptr) +- \| sizeof(ptr[0]) +- \) ) ++ MEMZERO_ARRAY(ptr, n) + +@@ +type T; +T[] ptr; +expression n; +@@ +- memset(ptr, \( '\0' \| 0 \), n * \( sizeof(T) +- \| sizeof(*ptr) +- \| sizeof(ptr[0]) +- \) ) ++ MEMZERO_ARRAY(ptr, n) diff --git a/git-compat-util.h b/git-compat-util.h index 398e0fac4f..2b8192fd2e 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -726,6 +726,7 @@ static inline uint64_t u64_add(uint64_t a, uint64_t b) #define ALLOC_ARRAY(x, alloc) (x) = xmalloc(st_mult(sizeof(*(x)), (alloc))) #define CALLOC_ARRAY(x, alloc) (x) = xcalloc((alloc), sizeof(*(x))) #define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), st_mult(sizeof(*(x)), (alloc))) +#define MEMZERO_ARRAY(x, alloc) memset((x), 0x0, st_mult(sizeof(*(x)), (alloc))) #define COPY_ARRAY(dst, src, n) copy_array((dst), (src), (n), sizeof(*(dst)) + \ BARF_UNLESS_COPYABLE((dst), (src))) ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] Add MEMZERO_ARRAY() macro and use it in coccinelle 2025-12-10 13:13 [PATCH 0/2] Add MEMZERO_ARRAY() macro and use it in coccinelle Toon Claes 2025-12-10 13:13 ` [PATCH 1/2] git-compat-util: introduce MEMZERO_ARRAY() macro Toon Claes 2025-12-10 13:13 ` [PATCH 2/2] contrib/coccinelle: pass include paths to spatch(1) Toon Claes @ 2025-12-13 0:18 ` Junio C Hamano 2 siblings, 0 replies; 10+ messages in thread From: Junio C Hamano @ 2025-12-13 0:18 UTC (permalink / raw) To: Toon Claes; +Cc: git, Jeff King Toon Claes <toon@iotcl.com> writes: > In the first patch I'm addressing the outcome of the discussion on the > patch, and in the second patch I'm fixing an edge-case I've encountered > while using coccinelle. Without the attached, the failure at CI is impossible to diagnose. A test that emits "you got some error messages", without showing what they are, is useless. Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git i/Makefile w/Makefile index 0bd502dc01..52e413f59a 100644 --- i/Makefile +++ w/Makefile @@ -3534,7 +3534,7 @@ else COCCICHECK_PATCH_MUST_BE_EMPTY_FILES = $(COCCICHECK_PATCHES_INTREE) endif coccicheck: $(COCCICHECK_PATCH_MUST_BE_EMPTY_FILES) - ! grep -q ^ $(COCCICHECK_PATCH_MUST_BE_EMPTY_FILES) /dev/null + ! grep ^ $(COCCICHECK_PATCH_MUST_BE_EMPTY_FILES) /dev/null # See contrib/coccinelle/README coccicheck-pending: coccicheck-test And FYI, with this series merged in, I get the following from 'seen'. I did not check if they contain any false positives (in which case the new coccinelle rules for this rewrite may have to be marked as "pending", not for real checking), or they are real improvements we should adopt. Thanks. diff -u -p a/diffcore-delta.c b/diffcore-delta.c --- a/diffcore-delta.c +++ b/diffcore-delta.c @@ -56,7 +56,7 @@ static struct spanhash_top *spanhash_reh st_mult(sizeof(struct spanhash), sz))); new_spanhash->alloc_log2 = orig->alloc_log2 + 1; new_spanhash->free = INITIAL_FREE(new_spanhash->alloc_log2); - memset(new_spanhash->data, 0, sizeof(struct spanhash) * sz); + MEMZERO_ARRAY(new_spanhash->data, sz); for (i = 0; i < osz; i++) { struct spanhash *o = &(orig->data[i]); int bucket; @@ -135,7 +135,7 @@ static struct spanhash_top *hash_chars(s st_mult(sizeof(struct spanhash), (size_t)1 << i))); hash->alloc_log2 = i; hash->free = INITIAL_FREE(i); - memset(hash->data, 0, sizeof(struct spanhash) * ((size_t)1 << i)); + MEMZERO_ARRAY(hash->data, ((size_t)1 << i)); n = 0; accum1 = accum2 = 0; diff -u -p a/linear-assignment.c b/linear-assignment.c --- a/linear-assignment.c +++ b/linear-assignment.c @@ -20,8 +20,8 @@ void compute_assignment(int column_count int i, j, phase; if (column_count < 2) { - memset(column2row, 0, sizeof(int) * column_count); - memset(row2column, 0, sizeof(int) * row_count); + MEMZERO_ARRAY(column2row, column_count); + MEMZERO_ARRAY(row2column, row_count); return; } diff -u -p a/shallow.c b/shallow.c --- a/shallow.c +++ b/shallow.c @@ -745,7 +745,7 @@ void assign_shallow_commits_to_refs(stru if (used) { int bitmap_size = DIV_ROUND_UP(pi.nr_bits, 32) * sizeof(uint32_t); - memset(used, 0, sizeof(*used) * info->shallow->nr); + MEMZERO_ARRAY(used, info->shallow->nr); for (i = 0; i < nr_shallow; i++) { const struct commit *c = lookup_commit(the_repository, &oid[shallow[i]]); @@ -810,7 +810,7 @@ static void post_assign_shallow(struct s trace_printf_key(&trace_shallow, "shallow: post_assign_shallow\n"); if (ref_status) - memset(ref_status, 0, sizeof(*ref_status) * info->ref->nr); + MEMZERO_ARRAY(ref_status, info->ref->nr); /* Remove unreachable shallow commits from "theirs" */ for (i = dst = 0; i < info->nr_theirs; i++) { ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-12-19 9:17 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-12-10 13:13 [PATCH 0/2] Add MEMZERO_ARRAY() macro and use it in coccinelle Toon Claes 2025-12-10 13:13 ` [PATCH 1/2] git-compat-util: introduce MEMZERO_ARRAY() macro Toon Claes 2025-12-11 3:18 ` Junio C Hamano 2025-12-12 13:02 ` René Scharfe 2025-12-19 9:17 ` Toon Claes 2025-12-10 13:13 ` [PATCH 2/2] contrib/coccinelle: pass include paths to spatch(1) Toon Claes 2025-12-11 6:19 ` Patrick Steinhardt 2025-12-12 5:04 ` Junio C Hamano 2025-12-13 1:22 ` Junio C Hamano 2025-12-13 0:18 ` [PATCH 0/2] Add MEMZERO_ARRAY() macro and use it in coccinelle Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).