[PATCH] staging: gdm724x: Add parenthesis to Macro arguments

[PATCH] staging: gdm724x: Add parenthesis to Macro arguments

[Andre Dainez]

Fix checkpatch errors:

CHECK: Macro argument 'len' may be better as '(len)' to avoid precedence issues
CHECK: Macro argument 'nlh' may be better as '(nlh)' to avoid precedence issues

Signed-off-by: Andre Dainez <andredainez@gmail.com>
---
 drivers/staging/gdm724x/netlink_k.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/gdm724x/netlink_k.c b/drivers/staging/gdm724x/netlink_k.c
index 92440c3..36d88f4 100644
--- a/drivers/staging/gdm724x/netlink_k.c
+++ b/drivers/staging/gdm724x/netlink_k.c
@@ -19,8 +19,8 @@ static DEFINE_MUTEX(netlink_mutex);
 #define ND_NLMSG_SPACE(len)	(NLMSG_SPACE(len) + ND_IFINDEX_LEN)
 #define ND_NLMSG_DATA(nlh)	((void *)((char *)NLMSG_DATA(nlh) + \
 						  ND_IFINDEX_LEN))
-#define ND_NLMSG_S_LEN(len)	(len + ND_IFINDEX_LEN)
-#define ND_NLMSG_R_LEN(nlh)	(nlh->nlmsg_len - ND_IFINDEX_LEN)
+#define ND_NLMSG_S_LEN(len)	((len) + ND_IFINDEX_LEN)
+#define ND_NLMSG_R_LEN(nlh)	((nlh)->nlmsg_len - ND_IFINDEX_LEN)
 #define ND_NLMSG_IFIDX(nlh)	NLMSG_DATA(nlh)
 #define ND_MAX_MSG_LEN		(1024 * 32)
 
-- 
2.7.4

Re: [PATCH] staging: gdm724x: Add parenthesis to Macro arguments

[Greg KH]

On Tue, Apr 02, 2019 at 10:04:05PM -0300, Andre Dainez wrote:
> Fix checkpatch errors:
> 
> CHECK: Macro argument 'len' may be better as '(len)' to avoid precedence issues
> CHECK: Macro argument 'nlh' may be better as '(nlh)' to avoid precedence issues
> 
> Signed-off-by: Andre Dainez <andredainez@gmail.com>
> ---
>  drivers/staging/gdm724x/netlink_k.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/gdm724x/netlink_k.c b/drivers/staging/gdm724x/netlink_k.c
> index 92440c3..36d88f4 100644
> --- a/drivers/staging/gdm724x/netlink_k.c
> +++ b/drivers/staging/gdm724x/netlink_k.c
> @@ -19,8 +19,8 @@ static DEFINE_MUTEX(netlink_mutex);
>  #define ND_NLMSG_SPACE(len)	(NLMSG_SPACE(len) + ND_IFINDEX_LEN)
>  #define ND_NLMSG_DATA(nlh)	((void *)((char *)NLMSG_DATA(nlh) + \
>  						  ND_IFINDEX_LEN))
> -#define ND_NLMSG_S_LEN(len)	(len + ND_IFINDEX_LEN)
> -#define ND_NLMSG_R_LEN(nlh)	(nlh->nlmsg_len - ND_IFINDEX_LEN)
> +#define ND_NLMSG_S_LEN(len)	((len) + ND_IFINDEX_LEN)

This makes sense, but:

> +#define ND_NLMSG_R_LEN(nlh)	((nlh)->nlmsg_len - ND_IFINDEX_LEN)

That does not, correct?

Re: [PATCH] staging: gdm724x: Add parenthesis to Macro arguments

[Andre]

Hi Greg, thanks for replying.

On 03/04/2019 01:26, Greg KH wrote:
> On Tue, Apr 02, 2019 at 10:04:05PM -0300, Andre Dainez wrote:
>> Fix checkpatch errors:
>>
>> CHECK: Macro argument 'len' may be better as '(len)' to avoid precedence issues
>> CHECK: Macro argument 'nlh' may be better as '(nlh)' to avoid precedence issues
>>
>> Signed-off-by: Andre Dainez <andredainez@gmail.com>
>> ---
>>  drivers/staging/gdm724x/netlink_k.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/gdm724x/netlink_k.c b/drivers/staging/gdm724x/netlink_k.c
>> index 92440c3..36d88f4 100644
>> --- a/drivers/staging/gdm724x/netlink_k.c
>> +++ b/drivers/staging/gdm724x/netlink_k.c
>> @@ -19,8 +19,8 @@ static DEFINE_MUTEX(netlink_mutex);
>>  #define ND_NLMSG_SPACE(len)	(NLMSG_SPACE(len) + ND_IFINDEX_LEN)
>>  #define ND_NLMSG_DATA(nlh)	((void *)((char *)NLMSG_DATA(nlh) + \
>>  						  ND_IFINDEX_LEN))
>> -#define ND_NLMSG_S_LEN(len)	(len + ND_IFINDEX_LEN)
>> -#define ND_NLMSG_R_LEN(nlh)	(nlh->nlmsg_len - ND_IFINDEX_LEN)
>> +#define ND_NLMSG_S_LEN(len)	((len) + ND_IFINDEX_LEN)
> 
> This makes sense, but:
> 
>> +#define ND_NLMSG_R_LEN(nlh)	((nlh)->nlmsg_len - ND_IFINDEX_LEN)
> 
> That does not, correct?
> 
Could you please clarify why this doesn't make sense?
If, for some reason I calculate by hand the pointer address and call this macro like: 
ND_NLMSG_R_LEN(nlh + sizeof(*nlh)), 
then it would expand like nlh + sizeof(*nlh)->nlmsg_len - ND_IFINDEX_LEN
which looks wrong in my pov, no?

Please, let me know if I misunderstood anything.
Thanks

Re: [PATCH] staging: gdm724x: Add parenthesis to Macro arguments

[Greg KH]

On Mon, Apr 08, 2019 at 02:37:58PM -0300, Andre wrote:
> Hi Greg, thanks for replying.
> 
> On 03/04/2019 01:26, Greg KH wrote:
> > On Tue, Apr 02, 2019 at 10:04:05PM -0300, Andre Dainez wrote:
> >> Fix checkpatch errors:
> >>
> >> CHECK: Macro argument 'len' may be better as '(len)' to avoid precedence issues
> >> CHECK: Macro argument 'nlh' may be better as '(nlh)' to avoid precedence issues
> >>
> >> Signed-off-by: Andre Dainez <andredainez@gmail.com>
> >> ---
> >>  drivers/staging/gdm724x/netlink_k.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/staging/gdm724x/netlink_k.c b/drivers/staging/gdm724x/netlink_k.c
> >> index 92440c3..36d88f4 100644
> >> --- a/drivers/staging/gdm724x/netlink_k.c
> >> +++ b/drivers/staging/gdm724x/netlink_k.c
> >> @@ -19,8 +19,8 @@ static DEFINE_MUTEX(netlink_mutex);
> >>  #define ND_NLMSG_SPACE(len)	(NLMSG_SPACE(len) + ND_IFINDEX_LEN)
> >>  #define ND_NLMSG_DATA(nlh)	((void *)((char *)NLMSG_DATA(nlh) + \
> >>  						  ND_IFINDEX_LEN))
> >> -#define ND_NLMSG_S_LEN(len)	(len + ND_IFINDEX_LEN)
> >> -#define ND_NLMSG_R_LEN(nlh)	(nlh->nlmsg_len - ND_IFINDEX_LEN)
> >> +#define ND_NLMSG_S_LEN(len)	((len) + ND_IFINDEX_LEN)
> > 
> > This makes sense, but:
> > 
> >> +#define ND_NLMSG_R_LEN(nlh)	((nlh)->nlmsg_len - ND_IFINDEX_LEN)
> > 
> > That does not, correct?
> > 
> Could you please clarify why this doesn't make sense?
> If, for some reason I calculate by hand the pointer address and call this macro like: 
> ND_NLMSG_R_LEN(nlh + sizeof(*nlh)), 
> then it would expand like nlh + sizeof(*nlh)->nlmsg_len - ND_IFINDEX_LEN
> which looks wrong in my pov, no?

Why would anyone ever do such a thing?  :)

That's the issue here, this is not needed as if someone were to want to
do something crazy like you are suggesting, it will properly blow up, so
no need to change anything here.

thanks,

greg k-h