Re: [PATCH] drm/amdgpu: Add braces to initialize task_info subojects
On Wed, Sep 12, 2018 at 1:24 PM Richard Smith wrote:
>
> On Wed, Sep 12, 2018 at 10:38 AM Nick Desaulniers
> wrote:
>>
>> On Tue, Sep 11, 2018 at 5:26 PM Nathan Chancellor
>> wrote:
>> >
>> > Clang warns if there are missing braces around a subobject
>> > initializer.
>> >
>> > drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c:1447:41: warning: suggest braces
>> > around initialization of subobject [-Wmissing-braces]
>> > struct amdgpu_task_info task_info = { 0 };
>> > ^
>> > {}
>> > 1 warning generated.
>> >
>> > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c:262:41: warning: suggest braces
>> > around initialization of subobject [-Wmissing-braces]
>> > struct amdgpu_task_info task_info = { 0 };
>> > ^
>> > {}
>> > 1 warning generated.
>> >
>> > Reported-by: Nick Desaulniers
>> > Signed-off-by: Nathan Chancellor
>> > ---
>> > drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 2 +-
>> > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 2 +-
>> > 2 files changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> > b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> > index 9333109b210d..968cc1b8cdff 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> > @@ -1444,7 +1444,7 @@ static int gmc_v8_0_process_interrupt(struct
>> > amdgpu_device *adev,
>> > gmc_v8_0_set_fault_enable_default(adev, false);
>> >
>> > if (printk_ratelimit()) {
>> > - struct amdgpu_task_info task_info = { 0 };
>> > + struct amdgpu_task_info task_info = { { 0 } };
>>
>> Hi Nathan,
>> Thanks for this patch. I discussed this syntax with our language
>> lawyers. Turns out, this is not quite correct, as you're now saying
>> "initialize the first subobject to zero, but not the rest of the
>> object." -Wmissing-field-initializers would highlight this, but it's
>> not part of -Wall. It would be more correct to zero initialize the
>> full struct, including all of its subobjects with `= {};`.
>
>
> Sorry, I think I've caused some confusion here.
>
> Elements with an omitted initializer get implicitly zero-initialized. In C++,
> it's idiomatic to write `= {}` to perform aggregate zero-initialization, but
> in C, that's invalid because at least one initializer is syntactically
> required within the braces. As a result, `= {0}` is an idiomatic way to
> perform zero-initialization of an aggregate in C.
That doesn't seem to be the case:
https://godbolt.org/z/TZzfo6 shouldn't Clang warn in the case of bar()?
> Clang intends to suppress the -Wmissing-braces in that case; if the warning
> is still being produced in a recent version of Clang, that's a bug. However,
> the warning suppression was added between Clang 5 and Clang 6, so it's very
> plausible that the compiler being used here is simply older than the warning
> fix.
>
> (Long story short: the change here seems fine, but should be unnecessary as
> of Clang 6.)
The warning was identified from clang-8 ToT synced yesterday.
--
Thanks,
~Nick Desaulniers
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Add braces to initialize task_info subojects
On Wed, Sep 12, 2018 at 10:38 AM Nick Desaulniers
wrote:
> On Tue, Sep 11, 2018 at 5:26 PM Nathan Chancellor
> wrote:
> >
> > Clang warns if there are missing braces around a subobject
> > initializer.
> >
> > drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c:1447:41: warning: suggest braces
> > around initialization of subobject [-Wmissing-braces]
> > struct amdgpu_task_info task_info = { 0 };
> > ^
> > {}
> > 1 warning generated.
> >
> > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c:262:41: warning: suggest braces
> > around initialization of subobject [-Wmissing-braces]
> > struct amdgpu_task_info task_info = { 0 };
> > ^
> > {}
> > 1 warning generated.
> >
> > Reported-by: Nick Desaulniers
> > Signed-off-by: Nathan Chancellor
> > ---
> > drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 2 +-
> > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > index 9333109b210d..968cc1b8cdff 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > @@ -1444,7 +1444,7 @@ static int gmc_v8_0_process_interrupt(struct
> amdgpu_device *adev,
> > gmc_v8_0_set_fault_enable_default(adev, false);
> >
> > if (printk_ratelimit()) {
> > - struct amdgpu_task_info task_info = { 0 };
> > + struct amdgpu_task_info task_info = { { 0 } };
>
> Hi Nathan,
> Thanks for this patch. I discussed this syntax with our language
> lawyers. Turns out, this is not quite correct, as you're now saying
> "initialize the first subobject to zero, but not the rest of the
> object." -Wmissing-field-initializers would highlight this, but it's
> not part of -Wall. It would be more correct to zero initialize the
> full struct, including all of its subobjects with `= {};`.
>
Sorry, I think I've caused some confusion here.
Elements with an omitted initializer get implicitly zero-initialized. In
C++, it's idiomatic to write `= {}` to perform aggregate
zero-initialization, but in C, that's invalid because at least one
initializer is syntactically required within the braces. As a result, `=
{0}` is an idiomatic way to perform zero-initialization of an aggregate in
C. Clang intends to suppress the -Wmissing-braces in that case; if the
warning is still being produced in a recent version of Clang, that's a bug.
However, the warning suppression was added between Clang 5 and Clang 6, so
it's very plausible that the compiler being used here is simply older than
the warning fix.
(Long story short: the change here seems fine, but should be unnecessary as
of Clang 6.)
> > amdgpu_vm_get_task_info(adev, entry->pasid, _info);
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > index 72f8018fa2a8..a781a5027212 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > @@ -259,7 +259,7 @@ static int gmc_v9_0_process_interrupt(struct
> amdgpu_device *adev,
> > }
> >
> > if (printk_ratelimit()) {
> > - struct amdgpu_task_info task_info = { 0 };
> > + struct amdgpu_task_info task_info = { { 0 } };
> >
> > amdgpu_vm_get_task_info(adev, entry->pasid, _info);
> >
> > --
> > 2.18.0
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Add braces to initialize task_info subojects
On Wed, Sep 12, 2018 at 01:24:34PM -0700, Richard Smith wrote:
> On Wed, Sep 12, 2018 at 10:38 AM Nick Desaulniers
> wrote:
>
> > On Tue, Sep 11, 2018 at 5:26 PM Nathan Chancellor
> > wrote:
> > >
> > > Clang warns if there are missing braces around a subobject
> > > initializer.
> > >
> > > drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c:1447:41: warning: suggest braces
> > > around initialization of subobject [-Wmissing-braces]
> > > struct amdgpu_task_info task_info = { 0 };
> > > ^
> > > {}
> > > 1 warning generated.
> > >
> > > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c:262:41: warning: suggest braces
> > > around initialization of subobject [-Wmissing-braces]
> > > struct amdgpu_task_info task_info = { 0 };
> > > ^
> > > {}
> > > 1 warning generated.
> > >
> > > Reported-by: Nick Desaulniers
> > > Signed-off-by: Nathan Chancellor
> > > ---
> > > drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 2 +-
> > > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 2 +-
> > > 2 files changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > > index 9333109b210d..968cc1b8cdff 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > > @@ -1444,7 +1444,7 @@ static int gmc_v8_0_process_interrupt(struct
> > amdgpu_device *adev,
> > > gmc_v8_0_set_fault_enable_default(adev, false);
> > >
> > > if (printk_ratelimit()) {
> > > - struct amdgpu_task_info task_info = { 0 };
> > > + struct amdgpu_task_info task_info = { { 0 } };
> >
> > Hi Nathan,
> > Thanks for this patch. I discussed this syntax with our language
> > lawyers. Turns out, this is not quite correct, as you're now saying
> > "initialize the first subobject to zero, but not the rest of the
> > object." -Wmissing-field-initializers would highlight this, but it's
> > not part of -Wall. It would be more correct to zero initialize the
> > full struct, including all of its subobjects with `= {};`.
> >
>
> Sorry, I think I've caused some confusion here.
>
> Elements with an omitted initializer get implicitly zero-initialized. In
> C++, it's idiomatic to write `= {}` to perform aggregate
> zero-initialization, but in C, that's invalid because at least one
> initializer is syntactically required within the braces. As a result, `=
> {0}` is an idiomatic way to perform zero-initialization of an aggregate in
> C. Clang intends to suppress the -Wmissing-braces in that case; if the
> warning is still being produced in a recent version of Clang, that's a bug.
> However, the warning suppression was added between Clang 5 and Clang 6, so
> it's very plausible that the compiler being used here is simply older than
> the warning fix.
>
> (Long story short: the change here seems fine, but should be unnecessary as
> of Clang 6.)
>
Interesting...
nathan@flashbox ~/kernels/next (master >) $ clang --version | head -n1
clang version 6.0.1 (tags/RELEASE_601/final)
I guess the v2 I sent is unnecessary then. I'll leave it up to the
maintainers to decide which one they want to take.
Thanks!
Nathan
>
> > > amdgpu_vm_get_task_info(adev, entry->pasid, _info);
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > > index 72f8018fa2a8..a781a5027212 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > > @@ -259,7 +259,7 @@ static int gmc_v9_0_process_interrupt(struct
> > amdgpu_device *adev,
> > > }
> > >
> > > if (printk_ratelimit()) {
> > > - struct amdgpu_task_info task_info = { 0 };
> > > + struct amdgpu_task_info task_info = { { 0 } };
> > >
> > > amdgpu_vm_get_task_info(adev, entry->pasid, _info);
> > >
> > > --
> > > 2.18.0
> > >
> >
> >
> > --
> > Thanks,
> > ~Nick Desaulniers
> >
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Add braces to initialize task_info subojects
On Wed, Sep 12, 2018 at 02:44:34PM -0400, Alex Deucher wrote:
> On Wed, Sep 12, 2018 at 2:40 PM Nathan Chancellor
> wrote:
> >
> > On Wed, Sep 12, 2018 at 10:38:30AM -0700, Nick Desaulniers wrote:
> > > On Tue, Sep 11, 2018 at 5:26 PM Nathan Chancellor
> > > wrote:
> > > >
> > > > Clang warns if there are missing braces around a subobject
> > > > initializer.
> > > >
> > > > drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c:1447:41: warning: suggest braces
> > > > around initialization of subobject [-Wmissing-braces]
> > > > struct amdgpu_task_info task_info = { 0 };
> > > > ^
> > > > {}
> > > > 1 warning generated.
> > > >
> > > > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c:262:41: warning: suggest braces
> > > > around initialization of subobject [-Wmissing-braces]
> > > > struct amdgpu_task_info task_info = { 0 };
> > > > ^
> > > > {}
> > > > 1 warning generated.
> > > >
> > > > Reported-by: Nick Desaulniers
> > > > Signed-off-by: Nathan Chancellor
> > > > ---
> > > > drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 2 +-
> > > > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 2 +-
> > > > 2 files changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > > > b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > > > index 9333109b210d..968cc1b8cdff 100644
> > > > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > > > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > > > @@ -1444,7 +1444,7 @@ static int gmc_v8_0_process_interrupt(struct
> > > > amdgpu_device *adev,
> > > > gmc_v8_0_set_fault_enable_default(adev, false);
> > > >
> > > > if (printk_ratelimit()) {
> > > > - struct amdgpu_task_info task_info = { 0 };
> > > > + struct amdgpu_task_info task_info = { { 0 } };
> > >
> > > Hi Nathan,
> > > Thanks for this patch. I discussed this syntax with our language
> > > lawyers. Turns out, this is not quite correct, as you're now saying
> > > "initialize the first subobject to zero, but not the rest of the
> > > object." -Wmissing-field-initializers would highlight this, but it's
> > > not part of -Wall. It would be more correct to zero initialize the
> > > full struct, including all of its subobjects with `= {};`.
> > >
> >
> > Good point, I was debating on which one was correct. There are several
> > places in this driver that use the multiple brace + 0 idiom, which is
> > why I used this form. I will spin up a v2 with your suggestion, thank
> > you for the review!
>
> Feel free to fix up the others as well. The others were only changed
> due to the same warning you sent the patch for.
>
> Alex
>
Hi Alex,
Thank you for the information, I will do that in v2.
Thanks,
Nathan
> >
> > Nathan
> >
> > > >
> > > > amdgpu_vm_get_task_info(adev, entry->pasid, _info);
> > > >
> > > > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > > > b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > > > index 72f8018fa2a8..a781a5027212 100644
> > > > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > > > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > > > @@ -259,7 +259,7 @@ static int gmc_v9_0_process_interrupt(struct
> > > > amdgpu_device *adev,
> > > > }
> > > >
> > > > if (printk_ratelimit()) {
> > > > - struct amdgpu_task_info task_info = { 0 };
> > > > + struct amdgpu_task_info task_info = { { 0 } };
> > > >
> > > > amdgpu_vm_get_task_info(adev, entry->pasid, _info);
> > > >
> > > > --
> > > > 2.18.0
> > > >
> > >
> > >
> > > --
> > > Thanks,
> > > ~Nick Desaulniers
> > ___
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Add braces to initialize task_info subojects
On Wed, Sep 12, 2018 at 2:40 PM Nathan Chancellor
wrote:
>
> On Wed, Sep 12, 2018 at 10:38:30AM -0700, Nick Desaulniers wrote:
> > On Tue, Sep 11, 2018 at 5:26 PM Nathan Chancellor
> > wrote:
> > >
> > > Clang warns if there are missing braces around a subobject
> > > initializer.
> > >
> > > drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c:1447:41: warning: suggest braces
> > > around initialization of subobject [-Wmissing-braces]
> > > struct amdgpu_task_info task_info = { 0 };
> > > ^
> > > {}
> > > 1 warning generated.
> > >
> > > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c:262:41: warning: suggest braces
> > > around initialization of subobject [-Wmissing-braces]
> > > struct amdgpu_task_info task_info = { 0 };
> > > ^
> > > {}
> > > 1 warning generated.
> > >
> > > Reported-by: Nick Desaulniers
> > > Signed-off-by: Nathan Chancellor
> > > ---
> > > drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 2 +-
> > > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 2 +-
> > > 2 files changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > > b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > > index 9333109b210d..968cc1b8cdff 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > > @@ -1444,7 +1444,7 @@ static int gmc_v8_0_process_interrupt(struct
> > > amdgpu_device *adev,
> > > gmc_v8_0_set_fault_enable_default(adev, false);
> > >
> > > if (printk_ratelimit()) {
> > > - struct amdgpu_task_info task_info = { 0 };
> > > + struct amdgpu_task_info task_info = { { 0 } };
> >
> > Hi Nathan,
> > Thanks for this patch. I discussed this syntax with our language
> > lawyers. Turns out, this is not quite correct, as you're now saying
> > "initialize the first subobject to zero, but not the rest of the
> > object." -Wmissing-field-initializers would highlight this, but it's
> > not part of -Wall. It would be more correct to zero initialize the
> > full struct, including all of its subobjects with `= {};`.
> >
>
> Good point, I was debating on which one was correct. There are several
> places in this driver that use the multiple brace + 0 idiom, which is
> why I used this form. I will spin up a v2 with your suggestion, thank
> you for the review!
Feel free to fix up the others as well. The others were only changed
due to the same warning you sent the patch for.
Alex
>
> Nathan
>
> > >
> > > amdgpu_vm_get_task_info(adev, entry->pasid, _info);
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > > b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > > index 72f8018fa2a8..a781a5027212 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > > @@ -259,7 +259,7 @@ static int gmc_v9_0_process_interrupt(struct
> > > amdgpu_device *adev,
> > > }
> > >
> > > if (printk_ratelimit()) {
> > > - struct amdgpu_task_info task_info = { 0 };
> > > + struct amdgpu_task_info task_info = { { 0 } };
> > >
> > > amdgpu_vm_get_task_info(adev, entry->pasid, _info);
> > >
> > > --
> > > 2.18.0
> > >
> >
> >
> > --
> > Thanks,
> > ~Nick Desaulniers
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Add braces to initialize task_info subojects
On Wed, Sep 12, 2018 at 10:38:30AM -0700, Nick Desaulniers wrote:
> On Tue, Sep 11, 2018 at 5:26 PM Nathan Chancellor
> wrote:
> >
> > Clang warns if there are missing braces around a subobject
> > initializer.
> >
> > drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c:1447:41: warning: suggest braces
> > around initialization of subobject [-Wmissing-braces]
> > struct amdgpu_task_info task_info = { 0 };
> > ^
> > {}
> > 1 warning generated.
> >
> > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c:262:41: warning: suggest braces
> > around initialization of subobject [-Wmissing-braces]
> > struct amdgpu_task_info task_info = { 0 };
> > ^
> > {}
> > 1 warning generated.
> >
> > Reported-by: Nick Desaulniers
> > Signed-off-by: Nathan Chancellor
> > ---
> > drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 2 +-
> > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > index 9333109b210d..968cc1b8cdff 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > @@ -1444,7 +1444,7 @@ static int gmc_v8_0_process_interrupt(struct
> > amdgpu_device *adev,
> > gmc_v8_0_set_fault_enable_default(adev, false);
> >
> > if (printk_ratelimit()) {
> > - struct amdgpu_task_info task_info = { 0 };
> > + struct amdgpu_task_info task_info = { { 0 } };
>
> Hi Nathan,
> Thanks for this patch. I discussed this syntax with our language
> lawyers. Turns out, this is not quite correct, as you're now saying
> "initialize the first subobject to zero, but not the rest of the
> object." -Wmissing-field-initializers would highlight this, but it's
> not part of -Wall. It would be more correct to zero initialize the
> full struct, including all of its subobjects with `= {};`.
>
Good point, I was debating on which one was correct. There are several
places in this driver that use the multiple brace + 0 idiom, which is
why I used this form. I will spin up a v2 with your suggestion, thank
you for the review!
Nathan
> >
> > amdgpu_vm_get_task_info(adev, entry->pasid, _info);
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > index 72f8018fa2a8..a781a5027212 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > @@ -259,7 +259,7 @@ static int gmc_v9_0_process_interrupt(struct
> > amdgpu_device *adev,
> > }
> >
> > if (printk_ratelimit()) {
> > - struct amdgpu_task_info task_info = { 0 };
> > + struct amdgpu_task_info task_info = { { 0 } };
> >
> > amdgpu_vm_get_task_info(adev, entry->pasid, _info);
> >
> > --
> > 2.18.0
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Add braces to initialize task_info subojects
On Tue, Sep 11, 2018 at 5:26 PM Nathan Chancellor
wrote:
>
> Clang warns if there are missing braces around a subobject
> initializer.
>
> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c:1447:41: warning: suggest braces
> around initialization of subobject [-Wmissing-braces]
> struct amdgpu_task_info task_info = { 0 };
> ^
> {}
> 1 warning generated.
>
> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c:262:41: warning: suggest braces
> around initialization of subobject [-Wmissing-braces]
> struct amdgpu_task_info task_info = { 0 };
> ^
> {}
> 1 warning generated.
>
> Reported-by: Nick Desaulniers
> Signed-off-by: Nathan Chancellor
> ---
> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> index 9333109b210d..968cc1b8cdff 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -1444,7 +1444,7 @@ static int gmc_v8_0_process_interrupt(struct
> amdgpu_device *adev,
> gmc_v8_0_set_fault_enable_default(adev, false);
>
> if (printk_ratelimit()) {
> - struct amdgpu_task_info task_info = { 0 };
> + struct amdgpu_task_info task_info = { { 0 } };
Hi Nathan,
Thanks for this patch. I discussed this syntax with our language
lawyers. Turns out, this is not quite correct, as you're now saying
"initialize the first subobject to zero, but not the rest of the
object." -Wmissing-field-initializers would highlight this, but it's
not part of -Wall. It would be more correct to zero initialize the
full struct, including all of its subobjects with `= {};`.
>
> amdgpu_vm_get_task_info(adev, entry->pasid, _info);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 72f8018fa2a8..a781a5027212 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -259,7 +259,7 @@ static int gmc_v9_0_process_interrupt(struct
> amdgpu_device *adev,
> }
>
> if (printk_ratelimit()) {
> - struct amdgpu_task_info task_info = { 0 };
> + struct amdgpu_task_info task_info = { { 0 } };
>
> amdgpu_vm_get_task_info(adev, entry->pasid, _info);
>
> --
> 2.18.0
>
--
Thanks,
~Nick Desaulniers
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: Add braces to initialize task_info subojects
Clang warns if there are missing braces around a subobject
initializer.
drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c:1447:41: warning: suggest braces
around initialization of subobject [-Wmissing-braces]
struct amdgpu_task_info task_info = { 0 };
^
{}
1 warning generated.
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c:262:41: warning: suggest braces
around initialization of subobject [-Wmissing-braces]
struct amdgpu_task_info task_info = { 0 };
^
{}
1 warning generated.
Reported-by: Nick Desaulniers
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 2 +-
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index 9333109b210d..968cc1b8cdff 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -1444,7 +1444,7 @@ static int gmc_v8_0_process_interrupt(struct
amdgpu_device *adev,
gmc_v8_0_set_fault_enable_default(adev, false);
if (printk_ratelimit()) {
- struct amdgpu_task_info task_info = { 0 };
+ struct amdgpu_task_info task_info = { { 0 } };
amdgpu_vm_get_task_info(adev, entry->pasid, _info);
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 72f8018fa2a8..a781a5027212 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -259,7 +259,7 @@ static int gmc_v9_0_process_interrupt(struct amdgpu_device
*adev,
}
if (printk_ratelimit()) {
- struct amdgpu_task_info task_info = { 0 };
+ struct amdgpu_task_info task_info = { { 0 } };
amdgpu_vm_get_task_info(adev, entry->pasid, _info);
--
2.18.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
