linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] [media] bt8xx: Remove unnecessary self-assignment
@ 2018-09-15  6:06 Nathan Chancellor
  2018-09-17 17:45 ` Nick Desaulniers
  0 siblings, 1 reply; 2+ messages in thread
From: Nathan Chancellor @ 2018-09-15  6:06 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linux-media, linux-kernel, Nick Desaulniers, Nathan Chancellor

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@google.com>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---

I'm not sure if the comment should have been removed as well. If it
does, I can easily send a v2.

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

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] [media] bt8xx: Remove unnecessary self-assignment
  2018-09-15  6:06 [PATCH] [media] bt8xx: Remove unnecessary self-assignment Nathan Chancellor
@ 2018-09-17 17:45 ` Nick Desaulniers
  0 siblings, 0 replies; 2+ messages in thread
From: Nick Desaulniers @ 2018-09-17 17:45 UTC (permalink / raw)
  To: Nathan Chancellor; +Cc: Mauro Carvalho Chehab, linux-media, LKML

On Fri, Sep 14, 2018 at 11:06 PM Nathan Chancellor
<natechancellor@gmail.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@google.com>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.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@google.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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2018-09-17 23:13 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-15  6:06 [PATCH] [media] bt8xx: Remove unnecessary self-assignment Nathan Chancellor
2018-09-17 17:45 ` Nick Desaulniers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).