Re: [PATCH 1/2] git-compat-util: introduce MEMZERO_ARRAY() macro

[Toon Claes <toon@iotcl.com>]

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