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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20190416112029.GB9823@kroah.com>
Date:   Tue, 16 Apr 2019 13:20:29 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Andre <andredainez@...il.com>
Cc:     lkcamp@...ts.libreplanetbr.org, realwakka@...il.com,
        straube.linux@...il.com, hle@....eu.com, rico.schrage@...il.com,
        sophie.matter@....de, Valentin.Vidic@...net.hr, simon@...anor.nu,
        devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: gdm724x: Add parenthesis to Macro arguments

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@...il.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