[PATCH] drm/i915: uninitialized value on error path

[PATCH] drm/i915: uninitialized value on error path

[Dan Carpenter]

"ret" isn't initialized on this error path.  It doesn't really cause
any problems unless it's randomly set to -EDEADLK which is not likely.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 48a546210d8b..d0e9578952d5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9563,6 +9563,7 @@ int intel_get_load_detect_pipe(struct drm_connector *connector,
 	 */
 	if (!crtc) {
 		DRM_DEBUG_KMS("no pipe available for load-detect\n");
+		ret = -ENODEV;
 		goto fail;
 	}
 

[PATCH] drm/i915: uninitialized value on error path

[Dan Carpenter]

"ret" isn't initialized on this error path.  It doesn't really cause
any problems unless it's randomly set to -EDEADLK which is not likely.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 48a546210d8b..d0e9578952d5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9563,6 +9563,7 @@ int intel_get_load_detect_pipe(struct drm_connector *connector,
 	 */
 	if (!crtc) {
 		DRM_DEBUG_KMS("no pipe available for load-detect\n");
+		ret = -ENODEV;
 		goto fail;
 	}
 

Re: [PATCH] drm/i915: uninitialized value on error path

[Chris Wilson]

On Thu, Apr 13, 2017 at 10:53:11PM +0300, Dan Carpenter wrote:
> "ret" isn't initialized on this error path.  It doesn't really cause
> any problems unless it's randomly set to -EDEADLK which is not likely.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 48a546210d8b..d0e9578952d5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9563,6 +9563,7 @@ int intel_get_load_detect_pipe(struct drm_connector *connector,
>  	 */
>  	if (!crtc) {
>  		DRM_DEBUG_KMS("no pipe available for load-detect\n");
> +		ret = -ENODEV;
>  		goto fail;
>  	}

Looks like we need

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8ffeda9d349b..4d0b3d80ed24 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9618,6 +9618,7 @@ int intel_get_load_detect_pipe(struct drm_connector *connector,
                DRM_DEBUG_KMS("reusing fbdev for load-detection framebuffer\n");
        if (IS_ERR(fb)) {
                DRM_DEBUG_KMS("failed to allocate framebuffer for load-detection\n");
+               ret = PTR_ERR(fb);
                goto fail;
        }
 
as well.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: uninitialized value on error path

[Chris Wilson]

On Thu, Apr 13, 2017 at 10:53:11PM +0300, Dan Carpenter wrote:
> "ret" isn't initialized on this error path.  It doesn't really cause
> any problems unless it's randomly set to -EDEADLK which is not likely.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 48a546210d8b..d0e9578952d5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9563,6 +9563,7 @@ int intel_get_load_detect_pipe(struct drm_connector *connector,
>  	 */
>  	if (!crtc) {
>  		DRM_DEBUG_KMS("no pipe available for load-detect\n");
> +		ret = -ENODEV;
>  		goto fail;
>  	}

Looks like we need

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8ffeda9d349b..4d0b3d80ed24 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9618,6 +9618,7 @@ int intel_get_load_detect_pipe(struct drm_connector *connector,
                DRM_DEBUG_KMS("reusing fbdev for load-detection framebuffer\n");
        if (IS_ERR(fb)) {
                DRM_DEBUG_KMS("failed to allocate framebuffer for load-detection\n");
+               ret = PTR_ERR(fb);
                goto fail;
        }
 
as well.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: uninitialized value on error path

[Patchwork]

== Series Details ==

Series: drm/i915: uninitialized value on error path
URL   : https://patchwork.freedesktop.org/series/23038/
State : success

== Summary ==

Series 23038v1 drm/i915: uninitialized value on error path
https://patchwork.freedesktop.org/api/1.0/series/23038/revisions/1/mbox/

Test gem_exec_fence:
        Subgroup await-hang-default:
                pass       -> INCOMPLETE (fi-hsw-4770) fdo#99726

fdo#99726 https://bugs.freedesktop.org/show_bug.cgi?id=99726

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:428s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:428s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:569s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:507s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time:548s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:483s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:478s
fi-hsw-4770      total:48   pass:41   dwarn:0   dfail:0   fail:0   skip:6  
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:411s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:427s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:491s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:478s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:457s
fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time:571s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:459s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:571s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:461s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:485s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:429s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:541s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:405s

0a54b7fee9ecda9257d34e5c3ac8de6516a60a90 drm-tip: 2017y-04m-13d-19h-12m-59s UTC integration manifest
c6c1e55 drm/i915: uninitialized value on error path

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4505/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: uninitialized value on error path

[Dan Carpenter]

True.  I'll resend.

regards,
dan carpenter

Re: [PATCH] drm/i915: uninitialized value on error path

[Dan Carpenter]

True.  I'll resend.

regards,
dan carpenter

[PATCH v2] drm/i915: set "ret" correctly on error paths

[Dan Carpenter]

If "crtc" is NULL, then my static checker complains that "ret" isn't
initialized on that path.  It doesn't really cause a problem unless
"ret" is somehow set to -EDEADLK which is not likely.

Chris Wilson also noticed another error path where "ret" isn't set
correctly.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 48a546210d8b..2c66de875d3d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9563,6 +9563,7 @@ int intel_get_load_detect_pipe(struct drm_connector *connector,
 	 */
 	if (!crtc) {
 		DRM_DEBUG_KMS("no pipe available for load-detect\n");
+		ret = -ENODEV;
 		goto fail;
 	}
 
@@ -9619,6 +9620,7 @@ int intel_get_load_detect_pipe(struct drm_connector *connector,
 		DRM_DEBUG_KMS("reusing fbdev for load-detection framebuffer\n");
 	if (IS_ERR(fb)) {
 		DRM_DEBUG_KMS("failed to allocate framebuffer for load-detection\n");
+		ret = PTR_ERR(fb);
 		goto fail;
 	}
 

[PATCH v2] drm/i915: set "ret" correctly on error paths

[Dan Carpenter]

If "crtc" is NULL, then my static checker complains that "ret" isn't
initialized on that path.  It doesn't really cause a problem unless
"ret" is somehow set to -EDEADLK which is not likely.

Chris Wilson also noticed another error path where "ret" isn't set
correctly.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 48a546210d8b..2c66de875d3d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9563,6 +9563,7 @@ int intel_get_load_detect_pipe(struct drm_connector *connector,
 	 */
 	if (!crtc) {
 		DRM_DEBUG_KMS("no pipe available for load-detect\n");
+		ret = -ENODEV;
 		goto fail;
 	}
 
@@ -9619,6 +9620,7 @@ int intel_get_load_detect_pipe(struct drm_connector *connector,
 		DRM_DEBUG_KMS("reusing fbdev for load-detection framebuffer\n");
 	if (IS_ERR(fb)) {
 		DRM_DEBUG_KMS("failed to allocate framebuffer for load-detection\n");
+		ret = PTR_ERR(fb);
 		goto fail;
 	}
 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

✓ Fi.CI.BAT: success for drm/i915: uninitialized value on error path (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: uninitialized value on error path (rev3)
URL   : https://patchwork.freedesktop.org/series/23038/
State : success

== Summary ==

Series 23038v3 drm/i915: uninitialized value on error path
https://patchwork.freedesktop.org/api/1.0/series/23038/revisions/3/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                pass       -> DMESG-WARN (fi-byt-j1900) fdo#100652

fdo#100652 https://bugs.freedesktop.org/show_bug.cgi?id=100652

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:427s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:432s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:571s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:516s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time:549s
fi-byt-j1900     total:278  pass:253  dwarn:1   dfail:0   fail:0   skip:24  time:483s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:479s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:415s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:404s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:431s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:496s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:487s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:458s
fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time:562s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:462s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:572s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:463s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:500s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:433s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:535s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:405s

4079da1bee731e2bf185411abd9ecda25f247890 drm-tip: 2017y-04m-13d-21h-13m-19s UTC integration manifest
8f25cd1 drm/i915: set "ret" correctly on error paths

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4507/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915: set "ret" correctly on error paths

[Chris Wilson]

On Fri, Apr 14, 2017 at 10:54:25PM +0300, Dan Carpenter wrote:
> If "crtc" is NULL, then my static checker complains that "ret" isn't
> initialized on that path.  It doesn't really cause a problem unless
> "ret" is somehow set to -EDEADLK which is not likely.
> 
> Chris Wilson also noticed another error path where "ret" isn't set
> correctly.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Pushed, thanks very much for the patch.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH v2] drm/i915: set "ret" correctly on error paths

[Chris Wilson]

On Fri, Apr 14, 2017 at 10:54:25PM +0300, Dan Carpenter wrote:
> If "crtc" is NULL, then my static checker complains that "ret" isn't
> initialized on that path.  It doesn't really cause a problem unless
> "ret" is somehow set to -EDEADLK which is not likely.
> 
> Chris Wilson also noticed another error path where "ret" isn't set
> correctly.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Pushed, thanks very much for the patch.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: ✓ Fi.CI.BAT: success for drm/i915: uninitialized value on error path (rev3)

[Dan Carpenter]

On Fri, Apr 14, 2017 at 08:13:43PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: uninitialized value on error path (rev3)
> URL   : https://patchwork.freedesktop.org/series/23038/
> State : success

These patchwork emails are sort of useless.  I wouldn't have sent the
patch if it couldn't compile which is basically all this is testing.
It's not exercising the failure path.

regards,
dan carpenter

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.BAT: success for drm/i915: uninitialized value on error path (rev3)

[Jani Nikula]

On Mon, 17 Apr 2017, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> On Fri, Apr 14, 2017 at 08:13:43PM -0000, Patchwork wrote:
>> == Series Details ==
>> 
>> Series: drm/i915: uninitialized value on error path (rev3)
>> URL   : https://patchwork.freedesktop.org/series/23038/
>> State : success
>
> These patchwork emails are sort of useless.  I wouldn't have sent the
> patch if it couldn't compile which is basically all this is testing.
> It's not exercising the failure path.

Yeah, the CI is not smart enough to know that just by looking at the
patch, so it'll test them all. ;)

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.BAT: success for drm/i915: uninitialized value on error path (rev3)

[Dan Carpenter]

On Tue, Apr 18, 2017 at 10:58:44AM +0300, Jani Nikula wrote:
> On Mon, 17 Apr 2017, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > On Fri, Apr 14, 2017 at 08:13:43PM -0000, Patchwork wrote:
> >> == Series Details ==
> >> 
> >> Series: drm/i915: uninitialized value on error path (rev3)
> >> URL   : https://patchwork.freedesktop.org/series/23038/
> >> State : success
> >
> > These patchwork emails are sort of useless.  I wouldn't have sent the
> > patch if it couldn't compile which is basically all this is testing.
> > It's not exercising the failure path.
> 
> Yeah, the CI is not smart enough to know that just by looking at the
> patch, so it'll test them all. ;)
> 

But could it just send responses if something fails?  Or it could be a
much shorter email:

    CI test successful.

    Use X-mailer-blahblah to filter successful tests.

We don't need to expose all the other internal information unless
something fails.

regards,
dan carpenter
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.BAT: success for drm/i915: uninitialized value on error path (rev3)

[Jani Nikula]

On Tue, 18 Apr 2017, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> On Tue, Apr 18, 2017 at 10:58:44AM +0300, Jani Nikula wrote:
>> On Mon, 17 Apr 2017, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>> > On Fri, Apr 14, 2017 at 08:13:43PM -0000, Patchwork wrote:
>> >> == Series Details ==
>> >> 
>> >> Series: drm/i915: uninitialized value on error path (rev3)
>> >> URL   : https://patchwork.freedesktop.org/series/23038/
>> >> State : success
>> >
>> > These patchwork emails are sort of useless.  I wouldn't have sent the
>> > patch if it couldn't compile which is basically all this is testing.
>> > It's not exercising the failure path.
>> 
>> Yeah, the CI is not smart enough to know that just by looking at the
>> patch, so it'll test them all. ;)
>> 
>
> But could it just send responses if something fails?  Or it could be a
> much shorter email:
>
>     CI test successful.
>
>     Use X-mailer-blahblah to filter successful tests.
>
> We don't need to expose all the other internal information unless
> something fails.

Thanks, I passed on the feedback.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.BAT: success for drm/i915: uninitialized value on error path (rev3)

[Martin Peres]

On 18/04/17 12:14, Dan Carpenter wrote:
> On Tue, Apr 18, 2017 at 10:58:44AM +0300, Jani Nikula wrote:
>> On Mon, 17 Apr 2017, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>>> On Fri, Apr 14, 2017 at 08:13:43PM -0000, Patchwork wrote:
>>>> == Series Details ==
>>>>
>>>> Series: drm/i915: uninitialized value on error path (rev3)
>>>> URL   : https://patchwork.freedesktop.org/series/23038/
>>>> State : success
>>>
>>> These patchwork emails are sort of useless.  I wouldn't have sent the
>>> patch if it couldn't compile which is basically all this is testing.
>>> It's not exercising the failure path.
>>
>> Yeah, the CI is not smart enough to know that just by looking at the
>> patch, so it'll test them all. ;)
>>
> 
> But could it just send responses if something fails?  Or it could be a
> much shorter email:
> 
>      CI test successful.
> 
>      Use X-mailer-blahblah to filter successful tests.

I guess that could be an interesting thing to add. But what prevents you
from dropping these emails if you don't want them? Their subject always
starts with "Fi.CI.BAT: success".

> 
> We don't need to expose all the other internal information unless
> something fails.

The purpose of this email is to say the patch series actually got
tested, making it easier for the person pushing the series that it
passed the basic requirements. The alternative would be to ask
people to check on patchwork every time, which is cumbersome.

One reason also to "expose the internals" is that machines come
and go as we often, we only have one machine of old platforms. So, a
patch checking a fix for gen4 with no actual gen4 HW testing it would be
pointless.

</ my two cents>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx