Re: [PATCH] amdgpu/gmc : fix compile warning

From: Guenter Roeck
Date: Fri Oct 19 2018 - 09:08:55 EST

On 10/19/2018 01:53 AM, Daniel Vetter wrote:
On Mon, Oct 08, 2018 at 06:13:56PM +0000, Koenig, Christian wrote:
Am 08.10.2018 um 19:46 schrieb Guenter Roeck:
On Mon, Oct 08, 2018 at 05:22:24PM +0000, Koenig, Christian wrote:
Am 08.10.2018 um 17:57 schrieb Deucher, Alexander:
One thing I found missing in the discussion was the reference to the
C standard.
The C99 standard states in section 6.7.8 (Initialization) clause 19:
"... all
subobjects that are not initialized explicitly shall be initialized
implicitly the same as objects that have static storage duration".
Clause 21 makes further reference to partial initialization,
suggesting the same. Various online resources, including the gcc
documentation, all state the same. I don't find any reference to a
partial initialization which would leave members of a structure
undefined. It would be interesting for me to understand how and why
this does not apply here.

In this context, it is interesting that the other 48 instances of the
{ { 0 } } initialization in the same driver don't raise similar
concerns, nor seemed to have caused any operational problems.
Feel free to provide patches to replace those with memset().

Not me. As I see it, the problem, if it exists, would be a violation of the C
standard. I don't believe hacking around bad C compilers. I would rather
blacklist such compilers.
Well then you would need to blacklist basically all gcc variants of the
last decade or so.

Initializing only known members of structures is a perfectly valid
optimization and well known issue when you then compare the structure
with memcpy() or use the bytes for hashing or something similar.

Isn't that about padding ? That is a completely different issue.

Correct, yes. But that is the reason why I recommend using memset() for
zero initialization.

See we don't know the inner layout of the structure, could be another
structure or an union.

If it's a structure everything is fine because if you initialize one
structure member all other get their default type (whatever that means),
but if it's an union.....

Not sure if compilers still react allergic to that, but its the status
I've learned the hard way when the C99 standard came out and it still
seems like people are working around that so I recommend everybody to
stick with memset().

Went boom:

What went boom ? This patch wasn't accepted, and I don't immediately see
the correlation of the suggested revert with the rejected patch.


Can we revert?

Also, can we properly igt this so that intel-gfx-ci could test this before
it's all fireworks?

Thanks, Daniel