Re: [PATCH 02/11] submodule--helper: replace memset() with { 0 }-initialization

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Wed, Jul 13 2022, Glen Choo wrote:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Glen Choo <chooglen@google.com> writes:
>>
>>> Ok. I wonder if we could reduce this kind of churn in the future by
>>> adding this to CodingGuidelines, e.g. "always use { 0 } for stack
>>> variables". Tangentially, do we require { NULL } when the first element
>>> is a pointer? (I'm not sure because this isn't in CodingGuidelines
>>> either AFAICT.)
>>
>> A valiable can legitimately be left uninitialized, or initialized to
>> a known value that is not zero using designated initializers.  So
>> saying something like
>>
>>     When zero-initializing an auto variable that is a struct or
>>     union in its definition, use "{ 0 }", whether the first member
>>     in the struct is of a number, a pointer, or a compound type.
>>
>> may be OK.  I do not think we would want to say "always use X", as
>> the world is not that simple..
>
> Thanks. Also, I made a typo here, I meant to be more specific with
> regards to memset(), like:
>
>   When zero-initializing an auto variable that is a struct or
>   union in its definition, use "{ 0 }", whether the first member
>   in the struct is of a number, a pointer, or a compound type. _Do not
>   use memset()._

It's curious that the { 0 } v.s. { NULL } form jumps out at people, but
seemingly not that you don't memset(&x, NULL, ...). I.e. that we're
already dealing with a form where C's "0 is NULL in pointer context"
rules kick in :)

So I wonder if we should say anything about the first member at all. On
the other hand this is quite an odd bit of C syntax, and reveals a bit
of on edge case or wart that I'm not happy with.

It's why I believe GCC has a syntax extension so you can just do:

	struct foo x = {};

I.e. 6.7.8.10 and 6.7.8.21 disuss the semantics of this, basically that
if there are fewer initializers than elements or members that the
structure is initialized as though it were "static".

But for the first member we rely on 0 and NULL being the same in pointer
context (even though NULL my not be (void*)NULL!). So it's a tad nasty,
it's basically doing the same as:

    const char *foo = 0; /* not NULL */

As an aside I wonder if we should say "C says this is undefined, but
c'mon, let's just assume '#define NULL (void*)0'". I think in practice
it's like the recently standardized 2's compliment, in that almost
nobody can even remember a platform where it isn't true, and none are in
current use (but maybe I'm wrong...).

Anyway, I was curious about this so I tried the following locally:
	
	@@
	type T;
	identifier I;
	@@
	- T I;
	+ T I = { 0 };
	... when strict
	    when != \( I \| &I \)
	(
	- memset(&I, 0, sizeof(I));
	|
	- memset(&I, 0, sizeof(T));
	)
	

Which aside from whitespace issues (that I've asked the cocci ML about)
yields a sane result.

What *doesn't* yield a sane result is getting rid of the "when strict"
there, i.e. we must not do this in xdiff/xhistogram.c's
histogram_diff(), as we "goto" to the memset() to re-zero-out the
variable.

But removing the "when strict" yields a change to 45 more files, the
initial one with "when strict" is 76 files. With the exception of that
histogram_diff() case (and I manually skimmed the code for the rest) it
passes all tests at least... :)

> Unless there really is a legitimate reason to use memset(&my_auto_var, 0
> sizeof(my_autovar)) that I've missed?

Basically no, except when you need to do the init N times, as noted
above.

>> We do favor designated initializers over traditional initialization
>> in the order of members these days, so something like
>>
>>     When declaring a struct/union variable or an array with initial
>>     value to some members or elements, consider using designated
>>     initializers, instead of listing the values in the order of
>>     members in the definition of the struct.
>>
>> would also be good.
>
> Thanks. If I have time I'll send that proposal to CodingGuidelines, or
> someone else can send it (I don't mind either way).

Sounds good!