On 10/08/2018 06:47 AM, Koenig, Christian wrote:
Am 08.10.2018 um 15:33 schrieb Guenter Roeck:
On 10/08/2018 01:00 AM, Christian König wrote:
Am 05.10.2018 um 10:38 schrieb Guenter Roeck:
On 10/05/2018 01:14 AM, Koenig, Christian wrote:
Am 04.10.2018 um 20:52 schrieb Guenter Roeck:
Hi,
On Fri, Sep 14, 2018 at 06:05:52PM +0800, Peng Hao wrote:
drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c:
In function ‘gmc_v8_0_process_interrupt’:
drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c:1447:10:
warning: missing braces around initializer [-Wmissing-braces]
Signed-off-by: Peng Hao <peng.hao2@xxxxxxxxxx>
Was there any feedback on this patch ? The problem does affect us,
and we'll need a fix.
Well as discussed using "{ { 0 } }" is as wrong as using "{ 0 }".
Ah, sorry, I must have missed the discussion.
It is for sure not the best solution, but at least it compiles, and
it seems
to be proliferating.
Yeah, and exactly that's the problem. As the discussion showed "{ { 0
} }" is buggy because it tells the compiler to only initialize the
first member of the structure, but not all of it.
That is incorrect and rather dangerous cause it can lead to
unforeseen results and should probably trigger a warning.
$ git grep "{ *{ *0 *} *}" | wc
144 1180 11802
$ git grep "{ *{ *0 *} *}" drivers/gpu/drm/amd/ | wc
50 459 5239
We should either use only "{ }" or even better make nails with
heads and
use memset().
I'd rather leave it up to the compiler to decide what is most
efficient.
And I would rather prefer to have a working driver :)
So { } isn't correct either ?
Yes, initializing structures with { } is known to be problematic as well.
It doesn't necessary initialize all bytes when you have padding causing
random failures when structures are memcmp().
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.
Anyway, I fixed up the code in our tree (with { }), so I'll leave it
up to you folks to decide what if anything to do about it.
Well considering the known problems with {} initialization I'm certainly
rejecting all patches which turns memset() into {}.
Please point me to specific instances of this problem.
Thanks,
Guenter
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
