| Message ID | 20200218160727.GA17010@kaaira-HP-Pavilion-Notebook (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | staging: bcm2835-camera: call function instead of macro | expand |
On Tue, Feb 18, 2020 at 09:37:28PM +0530, Kaaira Gupta wrote: > Fix checkpatch.pl warning of 'macro argument reuse' in bcm2835-camera.h > by removing the macro and calling the function, written in macro in > bcm2835-camera.h, directly in bcm2835-camera.c > > Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in> > --- > .../bcm2835-camera/bcm2835-camera.c | 28 +++++++++++++++---- > .../bcm2835-camera/bcm2835-camera.h | 10 ------- > 2 files changed, 22 insertions(+), 16 deletions(-) > > diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c > index 1ef31a984741..19b3ba80d0e7 100644 > --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c > +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c > @@ -919,9 +919,17 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *priv, > else > f->fmt.pix.colorspace = V4L2_COLORSPACE_SMPTE170M; > f->fmt.pix.priv = 0; > - > - v4l2_dump_pix_format(1, bcm2835_v4l2_debug, &dev->v4l2_dev, &f->fmt.pix, > - __func__); > + v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev, > + "%s: w %u h %u field %u pfmt 0x%x bpl %u sz_img %u colorspace 0x%x priv %u\n", > + __func__, > + (&f->fmt.pix)->width, > + (&f->fmt.pix)->height, > + (&f->fmt.pix)->field, > + (&f->fmt.pix)->pixelformat, > + (&f->fmt.pix)->bytesperline, > + (&f->fmt.pix)->sizeimage, > + (&f->fmt.pix)->colorspace, > + (&f->fmt.pix)->priv); This is not as nice to look at as the original. Just ignore the warning. regards, dan carpenter
On Tue, Feb 18, 2020 at 09:37:11PM +0300, Dan Carpenter wrote: > On Tue, Feb 18, 2020 at 09:37:28PM +0530, Kaaira Gupta wrote: > > Fix checkpatch.pl warning of 'macro argument reuse' in bcm2835-camera.h > > by removing the macro and calling the function, written in macro in > > bcm2835-camera.h, directly in bcm2835-camera.c > > > > Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in> > > --- > > .../bcm2835-camera/bcm2835-camera.c | 28 +++++++++++++++---- > > .../bcm2835-camera/bcm2835-camera.h | 10 ------- > > 2 files changed, 22 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c > > index 1ef31a984741..19b3ba80d0e7 100644 > > --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c > > +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c > > @@ -919,9 +919,17 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *priv, > > else > > f->fmt.pix.colorspace = V4L2_COLORSPACE_SMPTE170M; > > f->fmt.pix.priv = 0; > > - > > - v4l2_dump_pix_format(1, bcm2835_v4l2_debug, &dev->v4l2_dev, &f->fmt.pix, > > - __func__); > > + v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev, > > + "%s: w %u h %u field %u pfmt 0x%x bpl %u sz_img %u colorspace 0x%x priv %u\n", > > + __func__, > > + (&f->fmt.pix)->width, > > + (&f->fmt.pix)->height, > > + (&f->fmt.pix)->field, > > + (&f->fmt.pix)->pixelformat, > > + (&f->fmt.pix)->bytesperline, > > + (&f->fmt.pix)->sizeimage, > > + (&f->fmt.pix)->colorspace, > > + (&f->fmt.pix)->priv); > > This is not as nice to look at as the original. Just ignore the > warning. > > regards, > dan carpenter > So, is this warning to be ignored from everywhere in every driver, as it doesn't look good? And if yes, then why is it there in the first place? Thanks!
On Wed, Feb 19, 2020 at 12:47:47AM +0530, Kaaira Gupta wrote: > On Tue, Feb 18, 2020 at 09:37:11PM +0300, Dan Carpenter wrote: > > On Tue, Feb 18, 2020 at 09:37:28PM +0530, Kaaira Gupta wrote: > > > Fix checkpatch.pl warning of 'macro argument reuse' in bcm2835-camera.h > > > by removing the macro and calling the function, written in macro in > > > bcm2835-camera.h, directly in bcm2835-camera.c > > > > > > Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in> > > > --- > > > .../bcm2835-camera/bcm2835-camera.c | 28 +++++++++++++++---- > > > .../bcm2835-camera/bcm2835-camera.h | 10 ------- > > > 2 files changed, 22 insertions(+), 16 deletions(-) > > > > > > diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c > > > index 1ef31a984741..19b3ba80d0e7 100644 > > > --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c > > > +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c > > > @@ -919,9 +919,17 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *priv, > > > else > > > f->fmt.pix.colorspace = V4L2_COLORSPACE_SMPTE170M; > > > f->fmt.pix.priv = 0; > > > - > > > - v4l2_dump_pix_format(1, bcm2835_v4l2_debug, &dev->v4l2_dev, &f->fmt.pix, > > > - __func__); > > > + v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev, > > > + "%s: w %u h %u field %u pfmt 0x%x bpl %u sz_img %u colorspace 0x%x priv %u\n", > > > + __func__, > > > + (&f->fmt.pix)->width, > > > + (&f->fmt.pix)->height, > > > + (&f->fmt.pix)->field, > > > + (&f->fmt.pix)->pixelformat, > > > + (&f->fmt.pix)->bytesperline, > > > + (&f->fmt.pix)->sizeimage, > > > + (&f->fmt.pix)->colorspace, > > > + (&f->fmt.pix)->priv); > > > > This is not as nice to look at as the original. Just ignore the > > warning. > > > > regards, > > dan carpenter > > > So, is this warning to be ignored from everywhere in every driver, as it > doesn't look good? And if yes, then why is it there in the first place? Obviously the reason for the warning is a good idea. Do a google search for the dangers of c macros if you don't understand. But at the same time uniformity and clean code is nice so it's a matter of setting priorities. Checkpatch is just a Perl script and not the King of The World. We can ignore it if we want to. regards, dan carpenter
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c index 1ef31a984741..19b3ba80d0e7 100644 --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c @@ -919,9 +919,17 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *priv, else f->fmt.pix.colorspace = V4L2_COLORSPACE_SMPTE170M; f->fmt.pix.priv = 0; - - v4l2_dump_pix_format(1, bcm2835_v4l2_debug, &dev->v4l2_dev, &f->fmt.pix, - __func__); + v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev, + "%s: w %u h %u field %u pfmt 0x%x bpl %u sz_img %u colorspace 0x%x priv %u\n", + __func__, + (&f->fmt.pix)->width, + (&f->fmt.pix)->height, + (&f->fmt.pix)->field, + (&f->fmt.pix)->pixelformat, + (&f->fmt.pix)->bytesperline, + (&f->fmt.pix)->sizeimage, + (&f->fmt.pix)->colorspace, + (&f->fmt.pix)->priv); return 0; } @@ -995,9 +1003,17 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv, v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev, "Now %dx%d format %08X\n", f->fmt.pix.width, f->fmt.pix.height, f->fmt.pix.pixelformat); - - v4l2_dump_pix_format(1, bcm2835_v4l2_debug, &dev->v4l2_dev, &f->fmt.pix, - __func__); + v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev, + "%s: w %u h %u field %u pfmt 0x%x bpl %u sz_img %u colorspace 0x%x priv %u\n", + __func__, + (&f->fmt.pix)->width, + (&f->fmt.pix)->height, + (&f->fmt.pix)->field, + (&f->fmt.pix)->pixelformat, + (&f->fmt.pix)->bytesperline, + (&f->fmt.pix)->sizeimage, + (&f->fmt.pix)->colorspace, + (&f->fmt.pix)->priv); return 0; } diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h index b5fce38de038..2e3e1954e3ce 100644 --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h @@ -121,16 +121,6 @@ int set_framerate_params(struct bm2835_mmal_dev *dev); /* Debug helpers */ -#define v4l2_dump_pix_format(level, debug, dev, pix_fmt, desc) \ -{ \ - v4l2_dbg(level, debug, dev, \ -"%s: w %u h %u field %u pfmt 0x%x bpl %u sz_img %u colorspace 0x%x priv %u\n", \ - desc, \ - (pix_fmt)->width, (pix_fmt)->height, (pix_fmt)->field, \ - (pix_fmt)->pixelformat, (pix_fmt)->bytesperline, \ - (pix_fmt)->sizeimage, (pix_fmt)->colorspace, (pix_fmt)->priv); \ -} - #define v4l2_dump_win_format(level, debug, dev, win_fmt, desc) \ { \ v4l2_dbg(level, debug, dev, \
Fix checkpatch.pl warning of 'macro argument reuse' in bcm2835-camera.h by removing the macro and calling the function, written in macro in bcm2835-camera.h, directly in bcm2835-camera.c Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in> --- .../bcm2835-camera/bcm2835-camera.c | 28 +++++++++++++++---- .../bcm2835-camera/bcm2835-camera.h | 10 ------- 2 files changed, 22 insertions(+), 16 deletions(-)