linux-kernel - Re: [PATCH linux-next] drm/i915/display: Remove unused variable in the for loop.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87k0iau275.fsf@intel.com>
Date:   Mon, 18 Oct 2021 12:11:58 +0300
From:   Jani Nikula <jani.nikula@...ux.intel.com>
To:     luo penghao <cgel.zte@...il.com>
Cc:     Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@...el.com>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Stephen Rothwell <sfr@...b.auug.org.au>,
        intel-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org, luo penghao <luo.penghao@....com.cn>,
        Zeal Robot <zealci@....com.cn>,
        "Deak\, Imre" <imre.deak@...el.com>
Subject: Re: [PATCH linux-next] drm/i915/display: Remove unused variable in the for loop.

On Mon, 18 Oct 2021, luo penghao <cgel.zte@...il.com> wrote:
> Variable is not used in the loop, and its assignment is redundant too.
> So it should be deleted.
>
> The clang_analyzer complains as follows:
>
> drivers/gpu/drm/i915/display/intel_fb.c:1018:3 warning:
>
> Value stored to 'cpp' is never read.
>
> Reported-by: Zeal Robot <zealci@....com.cn>
> Signed-off-by: luo penghao <luo.penghao@....com.cn>
> ---
>  drivers/gpu/drm/i915/display/intel_fb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c
> index fa1f375..b9b6a7a 100644
> --- a/drivers/gpu/drm/i915/display/intel_fb.c
> +++ b/drivers/gpu/drm/i915/display/intel_fb.c
> @@ -998,7 +998,7 @@ int intel_fill_fb_info(struct drm_i915_private *i915, struct intel_framebuffer *
>  	for (i = 0; i < num_planes; i++) {
>  		struct fb_plane_view_dims view_dims;
>  		unsigned int width, height;
> -		unsigned int cpp, size;
> +		unsigned int size;
>  		u32 offset;
>  		int x, y;
>  		int ret;
> @@ -1015,7 +1015,7 @@ int intel_fill_fb_info(struct drm_i915_private *i915, struct intel_framebuffer *
>  				return -EINVAL;
>  		}
>  
> -		cpp = fb->base.format->cpp[i];
> +		fb->base.format->cpp[i];

Thanks for the report. However, this "fix" isn't any better than having
the unused variable. It's obviously wrong.

It would be useful to dig into the history of the function, and figure
out when and why the variable became unused, and whether that caused an
actual bug or whether this was just leftovers from some refactoring.

So that's what I did. Some git blame and git log -p revealed commit
d3c5e10b6059 ("drm/i915/intel_fb: Factor out
convert_plane_offset_to_xy()") that moved the check that used the cpp
variable to a separate function, and the local variable and the line
above became unused and useless.

That's the actually helpful part. It's easy to see and verify that the
right fix is to just remove the line completely.

BR,
Jani.


>  		intel_fb_plane_dims(fb, i, &width, &height);
>  
>  		ret = convert_plane_offset_to_xy(fb, i, width, &x, &y);

-- 
Jani Nikula, Intel Open Source Graphics Center