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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Johannes Sixt <j6t@xxxxxxxx> writes:

>> 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.
>
> I'm not questioning this case; the latter form is clearly preferable.
>
> Using only = { 0 } for zero-initialization makes the code immune to
> rearrangements of the struct members. That is not the case with = { NULL
> } because it requires that the first member is a pointer; if
> rearrangement makes the first member a non-pointer, the initializations
> must be adjusted.

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?  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?

As already pointed out downthread, sparse will catch an
initialization for an arithmetic type that is left to be NULL, but
which should have been spelled 0, when fields are reordered anyway,
so I do not think your "only when initializing the first field of
the structure with a zero value while leaving the initializer for
the remainder unspelled, we should say 0 not NULL even when the
field has pointer type" stance is not maintainable.

And I agree with you that the calloc()/memset() that fills it with
0-bit pattern is "evil-doing" as you say.  Compared to that, the
initialization rule for objects with static storage duration is
quite sane---a pointer field gets a NULL pointer and arithmetic
field gets a (positive or unsigned) zero.

I wish if we could say

	struct patch patch = {};

so that we an leave all fields to have their natural zero value like
fields in a static variable without explicit initialization do ;-)
but according to "6.7.8 Initialization / Syntax #1", the
initializer-list inside the braces must have at least one
initializer, so that won't be possible X-<.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux