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

From: "Maiti, Nabendu Bikash" <nabendu.bikash.maiti@intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>,
	"Chris Wilson" <chris@chris-wilson.co.uk>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915/bxt: Fix uninitialized variables in intel_check_sprite_plane
Date: Wed, 18 Nov 2015 18:48:37 +0530	[thread overview]
Message-ID: <564C7AAD.20705@intel.com> (raw)
In-Reply-To: <20151118125202.GG4437@intel.com>

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?

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

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