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

[Greg KH <gregkh@linuxfoundation.org>]

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