drm/i915: fix a read size argument
Subsystems (3)
PATCH 1/1 drm/i915: fix a read size argument
We want to read 3 bytes here, but because the parenthesis are in the wrong place we instead read:
sizeof(intel_dp->edp_dpcd) == sizeof(intel_dp->edp_dpcd)
which is one byte.
drivers/gpu/drm/i915/intel_dp.c +2 -2
@@ -3551,8 +3551,8 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)/* Read the eDP Display control capabilities registers */ if ((intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP ] &DP_DPCD_DISPLAY_CONTROL_CAPABLE ) &&drm_dp_dpcd_read (&intel_dp->aux,DP_EDP_DPCD_REV , - intel_dp->edp_dpcd,sizeof (intel_dp->edp_dpcd) == -sizeof (intel_dp->edp_dpcd))) + intel_dp->edp_dpcd,sizeof (intel_dp->edp_dpcd)) == +sizeof (intel_dp->edp_dpcd))DRM_DEBUG_KMS ("EDP DPCD : %*ph\n" , (int )sizeof (intel_dp->edp_dpcd), intel_dp->edp_dpcd);
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 9 lines · Dan Carpenter
We want to read 3 bytes here, but because the parenthesis are in the wrong place we instead read:
sizeof(intel_dp->edp_dpcd) == sizeof(intel_dp->edp_dpcd)which is one byte.
Fixes fe5a66f91c88 ("drm/i915: Read PSR caps/intermediate freqs/etc. only once on eDP")Signed-off-by Dan Carpenter <[email protected]>
Oops, does smatch catch this? I don't recall seeing the complaint in recent runs?
-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
Quoted 4 lines · Chris Wilson
Quoted 3 lines · Dan Carpenter
We want to read 3 bytes here, but because the parenthesis are in the wrong place we instead read:
Quoted 8 lines
Quoted 6 lines
sizeof(intel_dp->edp_dpcd) == sizeof(intel_dp->edp_dpcd)which is one byte.
Fixes fe5a66f91c88 ("drm/i915: Read PSR caps/intermediate freqs/etc. only once on eDP")Signed-off-by Dan Carpenter <[email protected]>Oops, does smatch catch this?
Yeah. It does.
regards,
dan carpenter
Quoted 9 lines · Dan Carpenter
We want to read 3 bytes here, but because the parenthesis are in the wrong place we instead read:
sizeof(intel_dp->edp_dpcd) == sizeof(intel_dp->edp_dpcd)which is one byte.
Fixes fe5a66f91c88 ("drm/i915: Read PSR caps/intermediate freqs/etc. only once on eDP")Signed-off-by Dan Carpenter <[email protected]>
Good catch! What tool did you use to find it, or did you find it by inspection?
(btw, there's a missing --- here, between the commit msg and the diff)
Quoted 6 lines
drivers/gpu/drm/i915/intel_dp.c +0
-0
drivers/gpu/drm/i915/intel_dp.c intel_edp_init_dpcd(struct intel_dp *intel_dp) · line 3557
Quoted 9 lines
drivers/gpu/drm/i915/intel_dp.c +2
-2@@ -3551,8 +3551,8 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)/* Read the eDP Display control capabilities registers */ if ((intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP ] &DP_DPCD_DISPLAY_CONTROL_CAPABLE ) &&drm_dp_dpcd_read (&intel_dp->aux,DP_EDP_DPCD_REV , - intel_dp->edp_dpcd,sizeof (intel_dp->edp_dpcd) == -sizeof (intel_dp->edp_dpcd))) + intel_dp->edp_dpcd,sizeof (intel_dp->edp_dpcd)) == +sizeof (intel_dp->edp_dpcd))DRM_DEBUG_KMS ("EDP DPCD : %*ph\n" , (int )sizeof (intel_dp->edp_dpcd), intel_dp->edp_dpcd);
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Am 13.10.2016 10:55, schrieb Dan Carpenter:
Quoted 15 lines
We want to read 3 bytes here, but because the parenthesis are in the wrong place we instead read:
sizeof(intel_dp->edp_dpcd) == sizeof(intel_dp->edp_dpcd)which is one byte.
Fixes fe5a66f91c88 ("drm/i915: Read PSR caps/intermediate freqs/etc. only once on eDP")Signed-off-by Dan Carpenter <[email protected]>drivers/gpu/drm/i915/intel_dp.c +0
-0
drivers/gpu/drm/i915/intel_dp.c intel_edp_init_dpcd(struct intel_dp *intel_dp) · line 3555
Quoted 7 lines
drivers/gpu/drm/i915/intel_dp.c +2
-2@@ -3551,8 +3551,8 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)/* Read the eDP Display control capabilities registers */ if ((intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP ] &DP_DPCD_DISPLAY_CONTROL_CAPABLE ) &&drm_dp_dpcd_read (&intel_dp->aux,DP_EDP_DPCD_REV , - intel_dp->edp_dpcd,sizeof (intel_dp->edp_dpcd) == -sizeof (intel_dp->edp_dpcd))) + intel_dp->edp_dpcd,sizeof (intel_dp->edp_dpcd)) == +sizeof (intel_dp->edp_dpcd))
perhaps we can morph that into something more readable ? I would suggest:
if (intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP] & DP_DPCD_DISPLAY_CONTROL_CAPABLE)
{
size_t size=sizeof(intel_dp->edp_dpcd); /* == EDP_DISPLAY_CTL_CAP_SIZE */
int ret;
ret=drm_dp_dpcd_read(&intel_dp->aux, DP_EDP_DPCD_REV,intel_dp->edp_dpcd,size);
if (ret != size )
......}
this way it improves readability and reduces line length.
Quoted 8 lines
drivers/gpu/drm/i915/intel_dp.c +0
-1DRM_DEBUG_KMS ("EDP DPCD : %*ph\n" , (int )sizeof (intel_dp->edp_dpcd), intel_dp->edp_dpcd); --To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Jani Nikula
maintainerQuoted 10 lines · walter harms
Am 13.10.2016 10:55, schrieb Dan Carpenter:
Quoted 9 lines
We want to read 3 bytes here, but because the parenthesis are in the wrong place we instead read:
sizeof(intel_dp->edp_dpcd) == sizeof(intel_dp->edp_dpcd)which is one byte.
Fixes fe5a66f91c88 ("drm/i915: Read PSR caps/intermediate freqs/etc. only once on eDP")Signed-off-by Dan Carpenter <[email protected]>
Dan, good catch, thank you. Luckily we only really use the first byte currently...
Quoted 28 lines
drivers/gpu/drm/i915/intel_dp.c intel_edp_init_dpcd(struct intel_dp *intel_dp) · line 3555
Quoted 12 lines
drivers/gpu/drm/i915/intel_dp.c +2
-2@@ -3551,8 +3551,8 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)/* Read the eDP Display control capabilities registers */ if ((intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP ] &DP_DPCD_DISPLAY_CONTROL_CAPABLE ) &&drm_dp_dpcd_read (&intel_dp->aux,DP_EDP_DPCD_REV , - intel_dp->edp_dpcd,sizeof (intel_dp->edp_dpcd) == -sizeof (intel_dp->edp_dpcd))) + intel_dp->edp_dpcd,sizeof (intel_dp->edp_dpcd)) == +sizeof (intel_dp->edp_dpcd))perhaps we can morph that into something more readable ? I would suggest:
if (intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP] &
DP_DPCD_DISPLAY_CONTROL_CAPABLE){ size_t size=sizeof(intel_dp->edp_dpcd); /* == EDP_DISPLAY_CTL_CAP_SIZE */ int ret; ret=drm_dp_dpcd_read(&intel_dp->aux, DP_EDP_DPCD_REV,intel_dp->edp_dpcd,size); if (ret != size ) ......
Quoted 3 lines
}
this way it improves readability and reduces line length.
Not convinced, let's just take the simple brace fix now.
BR,
Jani.
Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel