Re: [PATCH] drm/i915/bxt: Fix uninitialized variables in intel_check_sprite_plane

Ville Syrjälä
2015-11-18T13:30:56Z

On Wed, Nov 18, 2015 at 06:48:37PM +0530, Maiti, Nabendu Bikash wrote:
>
>
>
> On 11/18/2015 6:22 PM, Ville Syrjälä wrote:
> > On Wed, Nov 18, 2015 at 12:19:06PM +0000, Chris Wilson wrote:
> >> On Wed, Nov 18, 2015 at 05:43:52PM +0530, Nabendu Maiti wrote:
> >>> Uninitialized variables (width, Height) in intel_check_sprite_plane
> >>> leads to compilererror in O1 level. Initialize all declared variables
> >>> to fix this issue.
> >>>
> >>> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
> >> Or perhaps:
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> >> index 2b96f336589e..8d7b4eb5b5b9 100644
> >> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >> @@ -747,7 +747,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
> >> struct drm_framebuffer *fb = state->base.fb;
> >> int crtc_x, crtc_y;
> >> unsigned int crtc_w, crtc_h;
> >> - uint32_t src_x, src_y, src_w, src_h;
> >> struct drm_rect *src = &state->src;
> >> struct drm_rect *dst = &state->dst;
> >> const struct drm_rect *clip = &state->clip;
> >> @@ -813,6 +812,8 @@ intel_check_sprite_plane(struct drm_plane *plane,
> >> crtc_h = drm_rect_height(dst);
> >>
> >> if (state->visible) {
> >> + u32 src_x, src_y, src_w, src_h;
> >> +
> >> /* check again in case clipping clamped the results */
> >> hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> >> if (hscale < 0) {
> >> @@ -871,10 +872,9 @@ intel_check_sprite_plane(struct drm_plane *plane,
> >> if (crtc_w == 0)
> >> state->visible = false;
> >> }
> >> - }
> >>
> >> /* Check size restrictions when scaling */
> >> - if (state->visible && (src_w != crtc_w || src_h != crtc_h)) {
> >> + if (src_w != crtc_w || src_h != crtc_h) {
> > That would change what it does.
> yes, checked the code where inside each if condition loop we may be
> changing the state->visible var itself. Next condition check it may be
> false too.
>
> The place giving compiler error
> src->x1 = src_x << 16;
> src->x2 = (src_x + src_w) << 16;
> src->y1 = src_y << 16;
> src->y2 = (src_y + src_h) << 16;
>
> Then just one line change of initializing the variables is better?

Or maybe fix your compiler instead? I don't get any warning/errors from
this. What version of gcc are you using?

>
> >> unsigned int width_bytes;
> >>
> >> WARN_ON(!can_scale);
> >> @@ -898,7 +898,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
> >> }
> >> }
> >>
> >> - if (state->visible) {
> >> src->x1 = src_x << 16;
> >> src->x2 = (src_x + src_w) << 16;
> >> src->y1 = src_y << 16;
> >>
> >> And make both the compiler and reader happier
> >> -Chris
> >>
> >> --
> >> Chris Wilson, Intel Open Source Technology Centre
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Ville Syrjälä
Intel OTC