Re: [PATCH] range-diff: fix some 'hdr-check' and sparse warnings

From: Johannes Sixt <j6t@kdbg.org>
Date: Jul 14, 2019, 08:15 UTC
Message-ID: <9d6c7595-e320-7fa2-1f02-2f078b3831fa@kdbg.org>
In-Reply-To: <xmqq1rytpqse.fsf@gitster-ct.c.googlers.com>

Am 13.07.19 um 23:29 schrieb Junio C Hamano:

Show 24 quoted lines

> I do not think this position is maintainable, especially if you
> agree with me (and everybody else, including sparse) that this is a
> bad idea:
> 
>>   struct string_list dup_it = { 0, 0, 0, 1, 0 };
> 
> The way I read "6.7.8 Initialization" (sorry, I only have committee
> draft wg14/n1124 of 2005 handy) is that
> 
> 	struct patch patch = { 0 };
> 
> has an initializer for a structure with an automatic storage
> duration, and for each of the subsequent fields of the structure
> (i.e. we ran out the initializer after dealing with that single zero
> that talks about the first field), due to "all subobjects that are
> not initialized explicitly shall be initialized implicitly the same
> as objects that have static storage duration." rule, "if it has a
> pointer type, it is initialized to a null pointer", which is exactly
> in line with your (and our) position that the first example I left
> in the above (new_name gets assigned NULL).  So we are fine with the
> fields that are not speled out.
> 
> But then what about the explicitly spelled out 0 for the first
> field?

You are putting too much meaning in the token '0' when it appears in the token sequence '= { 0 }'. I understand this sequence as a signal to the reader of the code that "the whole struct is to be zero-initialized". NOT "the first member is set to zero and everything else the default zero value".

It just so happens that the compiler does the right thing with that '0' regardless of what type the first member has. (It even works when it is a struct, Peff!) That zero is a null pointer constant if the first member happens to be a pointer, i.e., the same value that is used for all other implicitly zero-initialized pointers.

Show 6 quoted lines

>  It is like an assignment, so by arguing that we should have
> 0 over there and not NULL, you are essentially arguing for
> 
> 	patch.new_name = 0; /* not NULL */
> 
> aren't you?

No,no. You are stretching my argument too far. I really only mean the sequence = { 0 } as a signal. When this form of zero-initialization becomes established, it takes away mental burden from the reader. It need not be decomposed into its parts; there is no question of what is it that is initialized by '0', and what happens to the rest of the struct. It means "zero, all of it" without thinking.

Show 5 quoted lines

> I wish if we could say
> 
> 	struct patch patch = {};
> 
> so that we an leave all fields to have their natural zero value

YES!

-- Hannes