[PATCH] staging: greybus: Add brackets to avoid precedence issues

[PATCH] staging: greybus: Add brackets to avoid precedence issues

[Nishka Dasgupta]

Add parentheses around arguments to avoid precedence issues. Issue found
by Checkpatch.

Signed-off-by: Nishka Dasgupta <nishka.dasgupta@yahoo.com>
---
 drivers/staging/greybus/camera.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/greybus/camera.c b/drivers/staging/greybus/camera.c
index 615c8e7fd51e..7b653a4fd63f 100644
--- a/drivers/staging/greybus/camera.c
+++ b/drivers/staging/greybus/camera.c
@@ -180,9 +180,9 @@ static const struct gb_camera_fmt_info *gb_camera_get_format_info(u16 gb_fmt)
 
 #define GB_CAMERA_MAX_SETTINGS_SIZE	8192
 
-#define gcam_dbg(gcam, format...)	dev_dbg(&gcam->bundle->dev, format)
-#define gcam_info(gcam, format...)	dev_info(&gcam->bundle->dev, format)
-#define gcam_err(gcam, format...)	dev_err(&gcam->bundle->dev, format)
+#define gcam_dbg((gcam), format...)	dev_dbg(&gcam->bundle->dev, format)
+#define gcam_info((gcam), format...)	dev_info(&gcam->bundle->dev, format)
+#define gcam_err((gcam), format...)	dev_err(&gcam->bundle->dev, format)
 
 static int gb_camera_operation_sync_flags(struct gb_connection *connection,
 					  int type, unsigned int flags,
-- 
2.19.1

Re: [PATCH] staging: greybus: Add brackets to avoid precedence issues

[Greg KH]

On Mon, Mar 04, 2019 at 11:09:47PM +0530, Nishka Dasgupta wrote:
> Add parentheses around arguments to avoid precedence issues. Issue found
> by Checkpatch.
> 
> Signed-off-by: Nishka Dasgupta <nishka.dasgupta@yahoo.com>
> ---
>  drivers/staging/greybus/camera.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/greybus/camera.c b/drivers/staging/greybus/camera.c
> index 615c8e7fd51e..7b653a4fd63f 100644
> --- a/drivers/staging/greybus/camera.c
> +++ b/drivers/staging/greybus/camera.c
> @@ -180,9 +180,9 @@ static const struct gb_camera_fmt_info *gb_camera_get_format_info(u16 gb_fmt)
>  
>  #define GB_CAMERA_MAX_SETTINGS_SIZE	8192
>  
> -#define gcam_dbg(gcam, format...)	dev_dbg(&gcam->bundle->dev, format)
> -#define gcam_info(gcam, format...)	dev_info(&gcam->bundle->dev, format)
> -#define gcam_err(gcam, format...)	dev_err(&gcam->bundle->dev, format)
> +#define gcam_dbg((gcam), format...)	dev_dbg(&gcam->bundle->dev, format)
> +#define gcam_info((gcam), format...)	dev_info(&gcam->bundle->dev, format)
> +#define gcam_err((gcam), format...)	dev_err(&gcam->bundle->dev, format)

Please explain how adding an extra () helps anything here.

I think checkpatch is very confused.  Always use your brain when running
it, as it is just a perl script and sometimes gets things wrong (like
here.)

thanks,

greg k-h

Re: [Outreachy kernel] Re: [PATCH] staging: greybus: Add brackets to avoid precedence issues

[Nishka Dasgupta]

On Tue, Mar 5, 2019 at 12:32 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Mon, Mar 04, 2019 at 11:09:47PM +0530, Nishka Dasgupta wrote:
> > Add parentheses around arguments to avoid precedence issues. Issue found
> > by Checkpatch.
> >
> > Signed-off-by: Nishka Dasgupta <nishka.dasgupta@yahoo.com>
> > ---
> >  drivers/staging/greybus/camera.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/staging/greybus/camera.c b/drivers/staging/greybus/camera.c
> > index 615c8e7fd51e..7b653a4fd63f 100644
> > --- a/drivers/staging/greybus/camera.c
> > +++ b/drivers/staging/greybus/camera.c
> > @@ -180,9 +180,9 @@ static const struct gb_camera_fmt_info *gb_camera_get_format_info(u16 gb_fmt)
> >
> >  #define GB_CAMERA_MAX_SETTINGS_SIZE  8192
> >
> > -#define gcam_dbg(gcam, format...)    dev_dbg(&gcam->bundle->dev, format)
> > -#define gcam_info(gcam, format...)   dev_info(&gcam->bundle->dev, format)
> > -#define gcam_err(gcam, format...)    dev_err(&gcam->bundle->dev, format)
> > +#define gcam_dbg((gcam), format...)  dev_dbg(&gcam->bundle->dev, format)
> > +#define gcam_info((gcam), format...) dev_info(&gcam->bundle->dev, format)
> > +#define gcam_err((gcam), format...)  dev_err(&gcam->bundle->dev, format)
>
> Please explain how adding an extra () helps anything here.
>
> I think checkpatch is very confused.  Always use your brain when running
> it, as it is just a perl script and sometimes gets things wrong (like
> here.)
I thought adding brackets just demarcates the parameter(s)? If that is
not necessary or useful, then I won't submit this patch in future.

> thanks,
>
> greg k-h
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20190304190228.GD13830%40kroah.com.
> For more options, visit https://groups.google.com/d/optout.
- Nishka

Re: [Outreachy kernel] Re: [PATCH] staging: greybus: Add brackets to avoid precedence issues

[Julia Lawall]

On Tue, 5 Mar 2019, Nishka Dasgupta wrote:

> On Tue, Mar 5, 2019 at 12:32 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Mon, Mar 04, 2019 at 11:09:47PM +0530, Nishka Dasgupta wrote:
> > > Add parentheses around arguments to avoid precedence issues. Issue found
> > > by Checkpatch.
> > >
> > > Signed-off-by: Nishka Dasgupta <nishka.dasgupta@yahoo.com>
> > > ---
> > >  drivers/staging/greybus/camera.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/staging/greybus/camera.c b/drivers/staging/greybus/camera.c
> > > index 615c8e7fd51e..7b653a4fd63f 100644
> > > --- a/drivers/staging/greybus/camera.c
> > > +++ b/drivers/staging/greybus/camera.c
> > > @@ -180,9 +180,9 @@ static const struct gb_camera_fmt_info *gb_camera_get_format_info(u16 gb_fmt)
> > >
> > >  #define GB_CAMERA_MAX_SETTINGS_SIZE  8192
> > >
> > > -#define gcam_dbg(gcam, format...)    dev_dbg(&gcam->bundle->dev, format)
> > > -#define gcam_info(gcam, format...)   dev_info(&gcam->bundle->dev, format)
> > > -#define gcam_err(gcam, format...)    dev_err(&gcam->bundle->dev, format)
> > > +#define gcam_dbg((gcam), format...)  dev_dbg(&gcam->bundle->dev, format)
> > > +#define gcam_info((gcam), format...) dev_info(&gcam->bundle->dev, format)
> > > +#define gcam_err((gcam), format...)  dev_err(&gcam->bundle->dev, format)
> >
> > Please explain how adding an extra () helps anything here.
> >
> > I think checkpatch is very confused.  Always use your brain when running
> > it, as it is just a perl script and sometimes gets things wrong (like
> > here.)
> I thought adding brackets just demarcates the parameter(s)? If that is
> not necessary or useful, then I won't submit this patch in future.

I'm not sure what this does.  A macro parameter list shouldn't have
parentheses in it.  I wonder if cpp sees the parentheses and decides it is
not a paramater list, but rather part of the macro body?  Are these macros
actually used?  If not you can just remove them.

julia


>
> > thanks,
> >
> > greg k-h
> >
> > --
> > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20190304190228.GD13830%40kroah.com.
> > For more options, visit https://groups.google.com/d/optout.
> - Nishka
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CAC4%2B%2BKAwPEjSSTbgnRYXCU%3Dz_WNJAs3rbAtL0XboZ-OCmjCmrg%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] Re: [PATCH] staging: greybus: Add brackets to avoid precedence issues

[Nishka Dasgupta]

On 05/03/19 2:42 AM, Julia Lawall wrote:
> 
> 
> On Tue, 5 Mar 2019, Nishka Dasgupta wrote:
> 
>> On Tue, Mar 5, 2019 at 12:32 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>>>
>>> On Mon, Mar 04, 2019 at 11:09:47PM +0530, Nishka Dasgupta wrote:
>>>> Add parentheses around arguments to avoid precedence issues. Issue found
>>>> by Checkpatch.
>>>>
>>>> Signed-off-by: Nishka Dasgupta <nishka.dasgupta@yahoo.com>
>>>> ---
>>>>   drivers/staging/greybus/camera.c | 6 +++---
>>>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/staging/greybus/camera.c b/drivers/staging/greybus/camera.c
>>>> index 615c8e7fd51e..7b653a4fd63f 100644
>>>> --- a/drivers/staging/greybus/camera.c
>>>> +++ b/drivers/staging/greybus/camera.c
>>>> @@ -180,9 +180,9 @@ static const struct gb_camera_fmt_info *gb_camera_get_format_info(u16 gb_fmt)
>>>>
>>>>   #define GB_CAMERA_MAX_SETTINGS_SIZE  8192
>>>>
>>>> -#define gcam_dbg(gcam, format...)    dev_dbg(&gcam->bundle->dev, format)
>>>> -#define gcam_info(gcam, format...)   dev_info(&gcam->bundle->dev, format)
>>>> -#define gcam_err(gcam, format...)    dev_err(&gcam->bundle->dev, format)
>>>> +#define gcam_dbg((gcam), format...)  dev_dbg(&gcam->bundle->dev, format)
>>>> +#define gcam_info((gcam), format...) dev_info(&gcam->bundle->dev, format)
>>>> +#define gcam_err((gcam), format...)  dev_err(&gcam->bundle->dev, format)
>>>
>>> Please explain how adding an extra () helps anything here.
>>>
>>> I think checkpatch is very confused.  Always use your brain when running
>>> it, as it is just a perl script and sometimes gets things wrong (like
>>> here.)
>> I thought adding brackets just demarcates the parameter(s)? If that is
>> not necessary or useful, then I won't submit this patch in future.
> 
> I'm not sure what this does.  A macro parameter list shouldn't have
> parentheses in it.  I wonder if cpp sees the parentheses and decides it is
> not a paramater list, but rather part of the macro body?  Are these macros
> actually used?  If not you can just remove them.

Ah, okay, I'll look into whether these macros are actually used.
- Nishka

> julia
> 
> 
>>
>>> thanks,
>>>
>>> greg k-h
>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
>>> To post to this group, send email to outreachy-kernel@googlegroups.com.
>>> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20190304190228.GD13830%40kroah.com.
>>> For more options, visit https://groups.google.com/d/optout.
>> - Nishka
>>
>> --
>> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
>> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CAC4%2B%2BKAwPEjSSTbgnRYXCU%3Dz_WNJAs3rbAtL0XboZ-OCmjCmrg%40mail.gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>