drm/i915: uninitialized value on error path

PATCH In review Dan Carpenter 5 participants 12 messages Apr 13, 2017 – Apr 18, 2017

Subsystems (3)

DRM drivers Intel i915 Kernel janitors

lore

Share & tools

lore

Dan Carpenter

author

2017-04-13 19:53 UTC #0 lore raw

PATCH 1/1 drm/i915: uninitialized value on error path

"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 <[email protected]>

drivers/gpu/drm/i915/intel_display.c +1 -0

index 48a546210d8b..d0e9578952d5 100644
@@ -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;
 	}

Chris Wilson

2017-04-13 20:29 UTC #1 lore raw

Quoted 10 lines · 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 <[email protected]>
drivers/gpu/drm/i915/intel_display.c +0 -0
index 48a546210d8b..d0e9578952d5 100644

drivers/gpu/drm/i915/intel_display.c int intel_get_load_detect_pipe(struct drm_connector *connector, · line 9568

Quoted 6 lines

drivers/gpu/drm/i915/intel_display.c +1 -0

@@ -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

drivers/gpu/drm/i915/intel_display.c +1 -0

index 8ffeda9d349b..4d0b3d80ed24 100644
@@ -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

Signature · 5 lines

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

Dan Carpenter

author

2017-04-13 21:13 UTC #2 lore raw

True. I'll resend.

regards,
dan carpenter

Dan Carpenter

author

2017-04-14 19:54 UTC #3 lore raw

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

In reply to Chris Wilson

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 <[email protected]>

drivers/gpu/drm/i915/intel_display.c +2 -0

index 48a546210d8b..2c66de875d3d 100644
@@ -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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Chris Wilson

Reviewed-by on v2 1/1

2017-04-15 09:32 UTC #4 lore raw

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

Quoted 8 lines · 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 <[email protected]>

Reviewed-by Chris Wilson <[email protected]>

Pushed, thanks very much for the patch.
-Chris

Signature · 1 line

Chris Wilson, Intel Open Source Technology Centre

Patchwork

2017-04-13 20:44 UTC #5 lore raw

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

In reply to Dan Carpenter

== 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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Patchwork

2017-04-14 20:13 UTC #6 lore raw

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

In reply to Dan Carpenter

== 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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Dan Carpenter

author

2017-04-17 10:53 UTC #7 lore raw

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

Quoted 5 lines · Patchwork

== 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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Jani Nikula

maintainer

2017-04-18 07:58 UTC #8 lore raw

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

Quoted 10 lines · Dan Carpenter

Quoted 5 lines · Patchwork
== 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.

Signature · 5 lines

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

Dan Carpenter

author

2017-04-18 09:15 UTC #9 lore raw

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

Quoted 8 lines · Jani Nikula

Quoted 7 lines · Dan Carpenter
Quoted 5 lines · Patchwork
== Series Details ==
Series: drm/i915: uninitialized value on error path (rev3)
URL : https://patchwork.freedesktop.org/series/23038/
State : success

Quoted 7 lines

Quoted 3 lines
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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Jani Nikula

maintainer

2017-04-18 09:39 UTC #10 lore raw

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

Quoted 20 lines · Dan Carpenter

Quoted 8 lines · Jani Nikula
Quoted 7 lines · Dan Carpenter
Quoted 5 lines · Patchwork
== Series Details ==
Series: drm/i915: uninitialized value on error path (rev3)
URL : https://patchwork.freedesktop.org/series/23038/
State : success
Quoted 7 lines
Quoted 3 lines
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:

Quoted 6 lines

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.

Signature · 5 lines

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

Martin Peres

2017-04-18 10:35 UTC #11 lore raw

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

In reply to Dan Carpenter

Quoted 17 lines · Dan Carpenter

Quoted 8 lines · Jani Nikula
Quoted 7 lines · Dan Carpenter
Quoted 5 lines · Patchwork
== Series Details ==
Series: drm/i915: uninitialized value on error path (rev3)
URL : https://patchwork.freedesktop.org/series/23038/
State : success
Quoted 7 lines
Quoted 3 lines
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. ;)

Quoted 6 lines

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".

Quoted 3 lines

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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx