* [Intel-gfx] [PATCH] drm/i915/selftests: Fix uninitialized variable
@ 2020-03-03 14:23 Aditya Swarup
2020-03-03 15:25 ` Jani Nikula
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Aditya Swarup @ 2020-03-03 14:23 UTC (permalink / raw
To: intel-gfx
Static code analysis tool identified struct lrc_timestamp data as being
uninitialized and then data.ce[] is being checked for NULL/negative
value in the error path. Initializing data variable fixes the issue.
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
---
drivers/gpu/drm/i915/gt/selftest_lrc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index ccf9debacd90..9b75b3c77a5b 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -4725,7 +4725,7 @@ static int live_lrc_timestamp(void *arg)
{
struct intel_gt *gt = arg;
enum intel_engine_id id;
- struct lrc_timestamp data;
+ struct lrc_timestamp data = { 0 };
const u32 poison[] = {
0,
S32_MAX,
--
2.25.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/selftests: Fix uninitialized variable
2020-03-03 14:23 [Intel-gfx] [PATCH] drm/i915/selftests: Fix uninitialized variable Aditya Swarup
@ 2020-03-03 15:25 ` Jani Nikula
2020-03-03 20:30 ` Matt Roper
2020-03-03 18:45 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915/selftests: Fix uninitialized variable (rev2) Patchwork
2020-03-03 20:25 ` [Intel-gfx] [PATCH] drm/i915/selftests: Fix uninitialized variable Matt Roper
2 siblings, 1 reply; 7+ messages in thread
From: Jani Nikula @ 2020-03-03 15:25 UTC (permalink / raw
To: Aditya Swarup, intel-gfx
On Tue, 03 Mar 2020, Aditya Swarup <aditya.swarup@intel.com> wrote:
> Static code analysis tool identified struct lrc_timestamp data as being
> uninitialized and then data.ce[] is being checked for NULL/negative
> value in the error path. Initializing data variable fixes the issue.
>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
> ---
> drivers/gpu/drm/i915/gt/selftest_lrc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> index ccf9debacd90..9b75b3c77a5b 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> @@ -4725,7 +4725,7 @@ static int live_lrc_timestamp(void *arg)
> {
> struct intel_gt *gt = arg;
> enum intel_engine_id id;
> - struct lrc_timestamp data;
> + struct lrc_timestamp data = { 0 };
{} is preferred over {0}.
BR,
Jani.
> const u32 poison[] = {
> 0,
> S32_MAX,
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915/selftests: Fix uninitialized variable (rev2)
2020-03-03 14:23 [Intel-gfx] [PATCH] drm/i915/selftests: Fix uninitialized variable Aditya Swarup
2020-03-03 15:25 ` Jani Nikula
@ 2020-03-03 18:45 ` Patchwork
2020-03-03 20:25 ` [Intel-gfx] [PATCH] drm/i915/selftests: Fix uninitialized variable Matt Roper
2 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2020-03-03 18:45 UTC (permalink / raw
To: Gustavo A. R. Silva; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/selftests: Fix uninitialized variable (rev2)
URL : https://patchwork.freedesktop.org/series/42194/
State : failure
== Summary ==
Applying: drm/i915/selftests: Fix uninitialized variable
Using index info to reconstruct a base tree...
M drivers/gpu/drm/i915/gt/selftest_lrc.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/gt/selftest_lrc.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/gt/selftest_lrc.c
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0001 drm/i915/selftests: Fix uninitialized variable
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/selftests: Fix uninitialized variable
2020-03-03 14:23 [Intel-gfx] [PATCH] drm/i915/selftests: Fix uninitialized variable Aditya Swarup
2020-03-03 15:25 ` Jani Nikula
2020-03-03 18:45 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915/selftests: Fix uninitialized variable (rev2) Patchwork
@ 2020-03-03 20:25 ` Matt Roper
2 siblings, 0 replies; 7+ messages in thread
From: Matt Roper @ 2020-03-03 20:25 UTC (permalink / raw
To: Aditya Swarup; +Cc: intel-gfx
On Tue, Mar 03, 2020 at 06:23:47AM -0800, Aditya Swarup wrote:
> Static code analysis tool identified struct lrc_timestamp data as being
> uninitialized and then data.ce[] is being checked for NULL/negative
> value in the error path. Initializing data variable fixes the issue.
>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/gt/selftest_lrc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> index ccf9debacd90..9b75b3c77a5b 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> @@ -4725,7 +4725,7 @@ static int live_lrc_timestamp(void *arg)
> {
> struct intel_gt *gt = arg;
> enum intel_engine_id id;
> - struct lrc_timestamp data;
> + struct lrc_timestamp data = { 0 };
> const u32 poison[] = {
> 0,
> S32_MAX,
> --
> 2.25.0
>
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/selftests: Fix uninitialized variable
2020-03-03 15:25 ` Jani Nikula
@ 2020-03-03 20:30 ` Matt Roper
2020-03-04 10:07 ` Jani Nikula
0 siblings, 1 reply; 7+ messages in thread
From: Matt Roper @ 2020-03-03 20:30 UTC (permalink / raw
To: Jani Nikula; +Cc: intel-gfx
On Tue, Mar 03, 2020 at 05:25:21PM +0200, Jani Nikula wrote:
> On Tue, 03 Mar 2020, Aditya Swarup <aditya.swarup@intel.com> wrote:
> > Static code analysis tool identified struct lrc_timestamp data as being
> > uninitialized and then data.ce[] is being checked for NULL/negative
> > value in the error path. Initializing data variable fixes the issue.
> >
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
> > ---
> > drivers/gpu/drm/i915/gt/selftest_lrc.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> > index ccf9debacd90..9b75b3c77a5b 100644
> > --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
> > +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> > @@ -4725,7 +4725,7 @@ static int live_lrc_timestamp(void *arg)
> > {
> > struct intel_gt *gt = arg;
> > enum intel_engine_id id;
> > - struct lrc_timestamp data;
> > + struct lrc_timestamp data = { 0 };
>
> {} is preferred over {0}.
Is there a reference for this (e.g., in the kernel coding style)? I
thought this came up a couple years ago and the consensus was the other
way, although I could be misremembering. Unless it's changed in a
recent standard, I think {} is only legal in C++, so using it in C code
is a gcc-ism?
Matt
>
> BR,
> Jani.
>
> > const u32 poison[] = {
> > 0,
> > S32_MAX,
>
> --
> Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/selftests: Fix uninitialized variable
2020-03-03 20:30 ` Matt Roper
@ 2020-03-04 10:07 ` Jani Nikula
2020-03-04 20:50 ` Jani Nikula
0 siblings, 1 reply; 7+ messages in thread
From: Jani Nikula @ 2020-03-04 10:07 UTC (permalink / raw
To: Matt Roper; +Cc: intel-gfx
On Tue, 03 Mar 2020, Matt Roper <matthew.d.roper@intel.com> wrote:
> On Tue, Mar 03, 2020 at 05:25:21PM +0200, Jani Nikula wrote:
>> On Tue, 03 Mar 2020, Aditya Swarup <aditya.swarup@intel.com> wrote:
>> > - struct lrc_timestamp data;
>> > + struct lrc_timestamp data = { 0 };
>>
>> {} is preferred over {0}.
>
> Is there a reference for this (e.g., in the kernel coding style)? I
> thought this came up a couple years ago and the consensus was the other
> way, although I could be misremembering. Unless it's changed in a
> recent standard, I think {} is only legal in C++, so using it in C code
> is a gcc-ism?
Both are widely used in the kernel. I think we've mostly converged to {}
in i915. Yes, it's a gcc-ism in C code, but the kernel is gcc, not
standard C.
I can't find a reference right now, but ISTR there are some warnings
issued in some cases with the {0} initializer, depending on the struct
and perhaps on the compiler.
Anyway, we're 71 to 9 in favor of {} in i915, so please go with that.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/selftests: Fix uninitialized variable
2020-03-04 10:07 ` Jani Nikula
@ 2020-03-04 20:50 ` Jani Nikula
0 siblings, 0 replies; 7+ messages in thread
From: Jani Nikula @ 2020-03-04 20:50 UTC (permalink / raw
To: Matt Roper; +Cc: intel-gfx
On Wed, 04 Mar 2020, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Tue, 03 Mar 2020, Matt Roper <matthew.d.roper@intel.com> wrote:
>> On Tue, Mar 03, 2020 at 05:25:21PM +0200, Jani Nikula wrote:
>>> On Tue, 03 Mar 2020, Aditya Swarup <aditya.swarup@intel.com> wrote:
>>> > - struct lrc_timestamp data;
>>> > + struct lrc_timestamp data = { 0 };
>>>
>>> {} is preferred over {0}.
>>
>> Is there a reference for this (e.g., in the kernel coding style)? I
>> thought this came up a couple years ago and the consensus was the other
>> way, although I could be misremembering. Unless it's changed in a
>> recent standard, I think {} is only legal in C++, so using it in C code
>> is a gcc-ism?
>
> Both are widely used in the kernel. I think we've mostly converged to {}
> in i915. Yes, it's a gcc-ism in C code, but the kernel is gcc, not
> standard C.
>
> I can't find a reference right now, but ISTR there are some warnings
> issued in some cases with the {0} initializer, depending on the struct
> and perhaps on the compiler.
Here's one [1].
BR,
Jani.
[1] http://patchwork.freedesktop.org/patch/msgid/20200304183654.GA9011@paulmck-ThinkPad-P72
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-03-04 20:50 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-03 14:23 [Intel-gfx] [PATCH] drm/i915/selftests: Fix uninitialized variable Aditya Swarup
2020-03-03 15:25 ` Jani Nikula
2020-03-03 20:30 ` Matt Roper
2020-03-04 10:07 ` Jani Nikula
2020-03-04 20:50 ` Jani Nikula
2020-03-03 18:45 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915/selftests: Fix uninitialized variable (rev2) Patchwork
2020-03-03 20:25 ` [Intel-gfx] [PATCH] drm/i915/selftests: Fix uninitialized variable Matt Roper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.