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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 12, 2019, 16:44 UTC
Message-ID: <xmqqy313p5hn.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <41a60e60-d2c0-7d54-5456-e44d106548a4@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

Show 17 quoted lines

> Am 12.07.19 um 00:03 schrieb Ramsay Jones:
>> diff --git a/range-diff.c b/range-diff.c
>> index ba1e9a4265..0f24a4ad12 100644
>> --- a/range-diff.c
>> +++ b/range-diff.c
>> @@ -102,7 +102,7 @@ static int read_patches(const char *range, struct string_list *list)
>>  		}
>>  
>>  		if (starts_with(line, "diff --git")) {
>> -			struct patch patch = { 0 };
>> +			struct patch patch = { NULL };
>
> There is nothing wrong with 0 here. IMHO, zero-initialization should
> *always* be written as = { 0 } and nothing else. Changing 0 to NULL to
> pacify sparse encourages a wrong style.
>
> -- Hannes

Hmm, care to elaborate a bit? Certainly, we have a clear preference between these two:

	struct patch patch;
	patch.new_name = 0;
	patch.new_name = NULL;

where the "char *new_name" field is the first one in the structure. We always try to write the latter, even though we know they ought to be equivalent to the language lawyers.

Is the reason why you say 0 is fine here because we consider

	struct patch patch, *dpatch;
	memset(&patch, 0, sizeof(patch));
	dpatch = xcalloc(1, sizeof(patch));

are perfectly good way to trivially iniitialize an instance of the struct?

Do we want to talk to sparse folks about this?