[PATCH] media: use swap() to make code cleaner

[PATCH] media: use swap() to make code cleaner

[davidcomponentone]

From: Yang Guang <yang.guang5@zte.com.cn>

Use the macro 'swap()' defined in 'include/linux/minmax.h' to avoid
opencoding it.

Signed-off-by: Yang Guang <yang.guang5@zte.com.cn>
---
 drivers/media/pci/saa7134/saa7134-video.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
index 374c8e1087de..6f4132058c35 100644
--- a/drivers/media/pci/saa7134/saa7134-video.c
+++ b/drivers/media/pci/saa7134/saa7134-video.c
@@ -823,7 +823,7 @@ static int buffer_activate(struct saa7134_dev *dev,
 {
 	struct saa7134_dmaqueue *dmaq = buf->vb2.vb2_buf.vb2_queue->drv_priv;
 	unsigned long base,control,bpl;
-	unsigned long bpl_uv,lines_uv,base2,base3,tmp; /* planar */
+	unsigned long bpl_uv, lines_uv, base2, base3; /* planar */
 
 	video_dbg("buffer_activate buf=%p\n", buf);
 	buf->top_seen = 0;
@@ -869,9 +869,7 @@ static int buffer_activate(struct saa7134_dev *dev,
 		base2    = base + bpl * dev->height;
 		base3    = base2 + bpl_uv * lines_uv;
 		if (dev->fmt->uvswap) {
-			tmp = base2;
-			base2 = base3;
-			base3 = tmp;
+			swap(base2, base3);
 		}
 		video_dbg("uv: bpl=%ld lines=%ld base2/3=%ld/%ld\n",
 			bpl_uv,lines_uv,base2,base3);
-- 
2.30.2

Re: [PATCH] media: use swap() to make code cleaner

[Kieran Bingham]

Hi David,

Is this patch something you discovered somewhere and have posted on
behalf of Yang Guang?

Have you made any modifications to it that would require your sign off?
or is it simply a repost of a patch you came across?

Quoting davidcomponentone@gmail.com (2021-11-03 08:33:37)
> From: Yang Guang <yang.guang5@zte.com.cn>
> 
> Use the macro 'swap()' defined in 'include/linux/minmax.h' to avoid
> opencoding it.
> 
> Signed-off-by: Yang Guang <yang.guang5@zte.com.cn>

The change itself looks fine to me though.

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

> ---
>  drivers/media/pci/saa7134/saa7134-video.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
> index 374c8e1087de..6f4132058c35 100644
> --- a/drivers/media/pci/saa7134/saa7134-video.c
> +++ b/drivers/media/pci/saa7134/saa7134-video.c
> @@ -823,7 +823,7 @@ static int buffer_activate(struct saa7134_dev *dev,
>  {
>         struct saa7134_dmaqueue *dmaq = buf->vb2.vb2_buf.vb2_queue->drv_priv;
>         unsigned long base,control,bpl;
> -       unsigned long bpl_uv,lines_uv,base2,base3,tmp; /* planar */
> +       unsigned long bpl_uv, lines_uv, base2, base3; /* planar */
>  
>         video_dbg("buffer_activate buf=%p\n", buf);
>         buf->top_seen = 0;
> @@ -869,9 +869,7 @@ static int buffer_activate(struct saa7134_dev *dev,
>                 base2    = base + bpl * dev->height;
>                 base3    = base2 + bpl_uv * lines_uv;
>                 if (dev->fmt->uvswap) {
> -                       tmp = base2;
> -                       base2 = base3;
> -                       base3 = tmp;
> +                       swap(base2, base3);
>                 }
>                 video_dbg("uv: bpl=%ld lines=%ld base2/3=%ld/%ld\n",
>                         bpl_uv,lines_uv,base2,base3);
> -- 
> 2.30.2
>

Re: [PATCH] media: use swap() to make code cleaner

[David Yang]

Hi Kieran,

Both of  yang.guang5@zte.com.cn and davidcomponentone@gmail.com are
mine.
Because the yang.guang5@zte.com.cn can't send email out. So I use the
gmail to send.

Thanks.

On Wed, Nov 03, 2021 at 03:14:14PM +0000, Kieran Bingham wrote:
> Hi David,
> 
> Is this patch something you discovered somewhere and have posted on
> behalf of Yang Guang?
> 
> Have you made any modifications to it that would require your sign off?
> or is it simply a repost of a patch you came across?
> 
> Quoting davidcomponentone@gmail.com (2021-11-03 08:33:37)
> > From: Yang Guang <yang.guang5@zte.com.cn>
> > 
> > Use the macro 'swap()' defined in 'include/linux/minmax.h' to avoid
> > opencoding it.
> > 
> > Signed-off-by: Yang Guang <yang.guang5@zte.com.cn>
> 
> The change itself looks fine to me though.
> 
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> > ---
> >  drivers/media/pci/saa7134/saa7134-video.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
> > index 374c8e1087de..6f4132058c35 100644
> > --- a/drivers/media/pci/saa7134/saa7134-video.c
> > +++ b/drivers/media/pci/saa7134/saa7134-video.c
> > @@ -823,7 +823,7 @@ static int buffer_activate(struct saa7134_dev *dev,
> >  {
> >         struct saa7134_dmaqueue *dmaq = buf->vb2.vb2_buf.vb2_queue->drv_priv;
> >         unsigned long base,control,bpl;
> > -       unsigned long bpl_uv,lines_uv,base2,base3,tmp; /* planar */
> > +       unsigned long bpl_uv, lines_uv, base2, base3; /* planar */
> >  
> >         video_dbg("buffer_activate buf=%p\n", buf);
> >         buf->top_seen = 0;
> > @@ -869,9 +869,7 @@ static int buffer_activate(struct saa7134_dev *dev,
> >                 base2    = base + bpl * dev->height;
> >                 base3    = base2 + bpl_uv * lines_uv;
> >                 if (dev->fmt->uvswap) {
> > -                       tmp = base2;
> > -                       base2 = base3;
> > -                       base3 = tmp;
> > +                       swap(base2, base3);
> >                 }
> >                 video_dbg("uv: bpl=%ld lines=%ld base2/3=%ld/%ld\n",
> >                         bpl_uv,lines_uv,base2,base3);
> > -- 
> > 2.30.2
> >

Re: [PATCH] media: use swap() to make code cleaner

[Andy Shevchenko]

On Wed, Nov 3, 2021 at 10:34 AM <davidcomponentone@gmail.com> wrote:
> Use the macro 'swap()' defined in 'include/linux/minmax.h' to avoid
> opencoding it.

Same comments as per all your valuable contributions: just think more
about the code that you are dealing with!

>                 if (dev->fmt->uvswap) {
> -                       tmp = base2;
> -                       base2 = base3;
> -                       base3 = tmp;
> +                       swap(base2, base3);
>                 }

Have you run checkpatch? What did it say?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] media: use swap() to make code cleaner

[Hans Verkuil]

On 04/11/2021 11:43, Andy Shevchenko wrote:
> On Wed, Nov 3, 2021 at 10:34 AM <davidcomponentone@gmail.com> wrote:
>> Use the macro 'swap()' defined in 'include/linux/minmax.h' to avoid
>> opencoding it.
> 
> Same comments as per all your valuable contributions: just think more
> about the code that you are dealing with!
> 
>>                 if (dev->fmt->uvswap) {
>> -                       tmp = base2;
>> -                       base2 = base3;
>> -                       base3 = tmp;
>> +                       swap(base2, base3);
>>                 }
> 
> Have you run checkpatch? What did it say?
> 

checkpatch says all is fine :-)

But yes, the {} can now be dropped. If I apply the patch, then run checkpatch,
it will indeed complain about the {}.

Regards,

	Hans