Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1506726 > unrolled thread

Re: [PATCH 1/1] drm/i915/dsi: silence a warning about uninitialized return value

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 1/1] drm/i915/dsi: silence a warning about uninitialized  return value Nicolas Iooss <nicolas.iooss_linux@m4x.org> - 2016-10-23 19:00 +0200
    Re: [Intel-gfx] [PATCH 1/1] drm/i915/dsi: silence a warning about  uninitialized return value Chris Wilson <chris@chris-wilson.co.uk> - 2016-10-23 19:30 +0200

#1506726 — Re: [PATCH 1/1] drm/i915/dsi: silence a warning about uninitialized return value

Hello,

I sent the patch below a few weeks ago. I got some comments (cf. [1])
which looked good, but the patch has not been merged in linux-next yet.
Do I need to fix something (like rewrite the commit message) in order to
get it merged?

Thanks,
Nicolas

[1] https://patchwork.freedesktop.org/patch/108941/

On 04/09/16 20:58, Nicolas Iooss wrote:
> When building the kernel with clang and some warning flags, the compiler
> reports that the return value of dcs_get_backlight() may be
> uninitialized:
> 
>     drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c:53:2: error: variable
>     'data' is used uninitialized whenever 'for' loop exits because its
>     condition is false [-Werror,-Wsometimes-uninitialized]
>             for_each_dsi_port(port, intel_dsi->dcs_backlight_ports) {
>             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     drivers/gpu/drm/i915/intel_dsi.h:126:49: note: expanded from macro
>     'for_each_dsi_port'
>     #define for_each_dsi_port(__port, __ports_mask)
>                                 for_each_port_masked(__port, __ports_mask)
>                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     drivers/gpu/drm/i915/i915_drv.h:322:26: note: expanded from macro
>     'for_each_port_masked'
>         for ((__port) = PORT_A; (__port) < I915_MAX_PORTS; (__port)++)  \
>                                 ^~~~~~~~~~~~~~~~~~~~~~~~~
>     drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c:60:9: note:
>     uninitialized use occurs here
>             return data;
>                    ^~~~
> 
> As intel_dsi->dcs_backlight_ports seems to be always initialized to a
> non-null value, the content of the for loop is always executed and there
> is no bug in the current code. Nevertheless the compiler has no way of
> knowing that assumption, so initialize variable 'data' to silence the
> warning here.
> 
> Signed-off-by: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
> ---
>  drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c b/drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c
> index ac7c6020c443..eec45856f910 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c
> @@ -46,7 +46,7 @@ static u32 dcs_get_backlight(struct intel_connector *connector)
>  	struct intel_encoder *encoder = connector->encoder;
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>  	struct mipi_dsi_device *dsi_device;
> -	u8 data;
> +	u8 data = 0;
>  	enum port port;
>  
>  	/* FIXME: Need to take care of 16 bit brightness level */
> 

[toc] | [next] | [standalone]

#1506730 — Re: [Intel-gfx] [PATCH 1/1] drm/i915/dsi: silence a warning about uninitialized return value

On Sun, Oct 23, 2016 at 06:55:58PM +0200, Nicolas Iooss wrote:
> Hello,
> 
> I sent the patch below a few weeks ago. I got some comments (cf. [1])
> which looked good, but the patch has not been merged in linux-next yet.
> Do I need to fix something (like rewrite the commit message) in order to
> get it merged?

It basically boils down to that you said it doesn't fix a bug, but shuts
up a non-default compiler using custom warning flags.

But there is a bug in that code, as mipi_dsi_dcs_read() may return an
error value, which may just be ENOMEM, but if it did we would return the
garbage. Not only should we fix the error handling here, but you also
need to fix the error handling in drivers/video/backlight/backlight.c as
despite many callees returning an error, it assumes that
bd->ops->get_brightness() never returns an error...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web