linux-kernel - Re: [PATCH] [media] bt8xx: Remove unnecessary self-assignment

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] [day] [month] [year] [list]

Message-ID: <CAKwvOd=Mx7T4tMAC0v46b=9BrkOS8MNxprkFWEQP5Bz6rZvvxg@mail.gmail.com>
Date:   Mon, 17 Sep 2018 10:45:13 -0700
From:   Nick Desaulniers <ndesaulniers@...gle.com>
To:     Nathan Chancellor <natechancellor@...il.com>
Cc:     Mauro Carvalho Chehab <mchehab@...nel.org>,
        linux-media@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] [media] bt8xx: Remove unnecessary self-assignment

On Fri, Sep 14, 2018 at 11:06 PM Nathan Chancellor
<natechancellor@...il.com> wrote:
>
> Clang warns when a variable is assigned to itself.
>
> drivers/media/pci/bt8xx/bttv-driver.c:2043:13: warning: explicitly
> assigning value of variable of type '__s32' (aka 'int') to itself
> [-Wself-assign]
>         min_height = min_height;
>         ~~~~~~~~~~ ^ ~~~~~~~~~~
> 1 warning generated.
>
> There doesn't appear to be any good reason for this and this statement
> was added in commit e5bd0260e7d3 ("V4L/DVB (5077): Bttv cropping
> support") back in 2007. Just remove it.
>
> Reported-by: Nick Desaulniers <ndesaulniers@...gle.com>
> Signed-off-by: Nathan Chancellor <natechancellor@...il.com>
> ---
>
> I'm not sure if the comment should have been removed as well. If it
> does, I can easily send a v2.

I think the comments are actually reversed; the comment about max
scale should be for max_height and min scale should be for min_height.

Self assignment usually is used to silence unused variable warnings,
which doesn't look to be the case here.

Not sure about the comment; I think it's good to keep it, just unsure
of the exact statement it applies to.  Maybe Mauro has thoughts on
that?

Either way, you can add my reviewed by tag to this or a v2 that
moves/removes the comment, though I think this patch may just be fine.
Reviewed-by: Nick Desaulniers <ndesaulniers@...gle.com>

Thanks for the patch.

>
> Thanks!
>
>  drivers/media/pci/bt8xx/bttv-driver.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c
> index 413bf287547c..b2cfcbb0008e 100644
> --- a/drivers/media/pci/bt8xx/bttv-driver.c
> +++ b/drivers/media/pci/bt8xx/bttv-driver.c
> @@ -2040,7 +2040,6 @@ limit_scaled_size_lock       (struct bttv_fh *               fh,
>         max_width = max_width & width_mask;
>
>         /* Max. scale factor is 16:1 for frames, 8:1 for fields. */
> -       min_height = min_height;
>         /* Min. scale factor is 1:1. */
>         max_height >>= !V4L2_FIELD_HAS_BOTH(field);
>
> --
> 2.18.0
>


-- 
Thanks,
~Nick Desaulniers