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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Nov 18, 2015 at 10:33:55PM +0530, Maiti, Nabendu Bikash wrote:
> 
> 
> On 11/18/2015 7:00 PM, Ville Syrjälä wrote:
> > 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?
>   I still get the warning and error if -Werror is enabled, And on 
> Makefile O1 optimization is enabled.

And why exactly are you building with -O1?

> (note on Android build by default 
> Werror is enabled and gcc-- GCC: (GNU) 4.9.2
> )
> 
> on generic build, gcc --gcc (Ubuntu 4.9.3-5ubuntu1~14.04) 4.9.3
> 
> following are the error..
>    CC      drivers/staging/android/timed_gpio.o
> /home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c: 
> In function 'intel_check_sprite_plane':
> /home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c:1241:20: 
> error: 'src_h' may be used uninitialized in this function 
> [-Werror=maybe-uninitialized]
>     src->y2 = (src_y + src_h) << 16;
>                      ^
> /home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c:1239:20: 
> error: 'src_w' may be used uninitialized in this function 
> [-Werror=maybe-uninitialized]
>     src->x2 = (src_x + src_w) << 16;
>                      ^
> /home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c:1241:20: 
> error: 'src_y' may be used uninitialized in this function 
> [-Werror=maybe-uninitialized]
>     src->y2 = (src_y + src_h) << 16;
>                      ^
> /home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c:1238:19: 
> error: 'src_x' may be used uninitialized in this function 
> [-Werror=maybe-uninitialized]
>     src->x1 = src_x << 16;
> >
> >>>>                           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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx