linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Koenig, Christian" <Christian.Koenig@amd.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: "Deucher, Alexander" <Alexander.Deucher@amd.com>,
	Peng Hao <peng.hao2@zte.com.cn>,
	"airlied@linux.ie" <airlied@linux.ie>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"amd-gfx@lists.freedesktop.org" <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] amdgpu/gmc : fix compile warning
Date: Mon, 8 Oct 2018 18:13:56 +0000	[thread overview]
Message-ID: <4a1faa3b-8c42-b742-9b55-9d2711f7ecc1@amd.com> (raw)
In-Reply-To: <20181008174628.GB11442@roeck-us.net>

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().

Christian.

>
> Guenter


  reply	other threads:[~2018-10-08 18:16 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-14 10:05 Peng Hao
2018-10-04 18:52 ` Guenter Roeck
2018-10-05  8:14   ` Koenig, Christian
2018-10-05  8:38     ` Guenter Roeck
2018-10-08  8:00       ` Christian König
2018-10-08 13:33         ` Guenter Roeck
2018-10-08 13:47           ` Koenig, Christian
2018-10-08 14:10             ` Guenter Roeck
2018-10-08 15:57               ` Deucher, Alexander
2018-10-08 17:22                 ` Koenig, Christian
2018-10-08 17:46                   ` Guenter Roeck
2018-10-08 18:13                     ` Koenig, Christian [this message]
2018-10-19  8:53                       ` Daniel Vetter
2018-10-19  8:56                         ` Daniel Vetter
2018-10-19 13:08                         ` Guenter Roeck
2018-10-19 15:30                           ` Alex Deucher
2018-10-08 17:41                 ` Guenter Roeck
2018-10-08 18:24                   ` Deucher, Alexander

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4a1faa3b-8c42-b742-9b55-9d2711f7ecc1@amd.com \
    --to=christian.koenig@amd.com \
    --cc=Alexander.Deucher@amd.com \
    --cc=airlied@linux.ie \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=peng.hao2@zte.com.cn \
    --subject='Re: [PATCH] amdgpu/gmc : fix compile warning' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).