From: Toon Claes Date: Fri, 19 Dec 2025 09:17:33 GMT Subject: Re: [PATCH 1/2] git-compat-util: introduce MEMZERO_ARRAY() macro Message-ID: <878qeyhlua.fsf@iotcl.com> In-Reply-To: René Scharfe writes: > On 12/11/25 4:18 AM, Junio C Hamano wrote: >> Toon Claes 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