linux-kernel - Re: [PATCH v3 06/10] staging: r8188eu: Add space between function & macro parameters

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <8e7c457c-a58a-9c2a-ba0-d520c5e0f53e@inria.fr>
Date:   Thu, 20 Oct 2022 12:21:35 +0200 (CEST)
From:   Julia Lawall <julia.lawall@...ia.fr>
To:     Deepak R Varma <drv@...lo.com>
cc:     outreachy@...ts.linux.dev, Larry.Finger@...inger.net,
        phil@...lpotter.co.uk, paskripkin@...il.com,
        gregkh@...uxfoundation.org, linux-staging@...ts.linux.dev,
        linux-kernel@...r.kernel.org, kumarpraveen@...ux.microsoft.com,
        saurabh.truth@...il.com
Subject: Re: [PATCH v3 06/10] staging: r8188eu: Add space between function
 & macro parameters

> > > -#define PlatformEFIOWrite1Byte(_a,_b,_c)		\
> > > -	rtw_write8(_a,_b,_c)
> > > -#define PlatformEFIOWrite2Byte(_a,_b,_c)		\
> > > -	rtw_write16(_a,_b,_c)
> > > -#define PlatformEFIOWrite4Byte(_a,_b,_c)		\
> > > -	rtw_write32(_a,_b,_c)
> > > -
> > > -#define PlatformEFIORead1Byte(_a,_b)		\
> > > -		rtw_read8(_a,_b)
> > > -#define PlatformEFIORead2Byte(_a,_b)		\
> > > -		rtw_read16(_a,_b)
> > > -#define PlatformEFIORead4Byte(_a,_b)		\
> > > -		rtw_read32(_a,_b)
> > > +#define PlatformEFIOWrite1Byte(_a, _b, _c)		\
> > > +	rtw_write8(_a, _b, _c)
> > > +#define PlatformEFIOWrite2Byte(_a, _b, _c)		\
> > > +	rtw_write16(_a, _b, _c)
> > > +#define PlatformEFIOWrite4Byte(_a, _b, _c)		\
> > > +	rtw_write32(_a, _b, _c)
> > > +
> > > +#define PlatformEFIORead1Byte(_a, _b)		\
> > > +		rtw_read8(_a, _b)
> > > +#define PlatformEFIORead2Byte(_a, _b)		\
> > > +		rtw_read16(_a, _b)
> > > +#define PlatformEFIORead4Byte(_a, _b)		\
> > > +		rtw_read32(_a, _b)
> >
> > Could these be inline functions?
>
> I am actually not seeing these macros being used anywhere. These macros were
> added recently [commit ID: 7884fc0a1473c2721f496f1d1ddc9d2c91aefa53] in 2021. I
> am unsure if they are intended to be used in the future or can removed entirely.
>
> Making these inline functions can be done, however, will we need to measure
> performance impact? I will need help and time for this evaluation.

Inline functions shouldn't have any performance impact.  For these simple
things the compiler should inline them.

The reason why a macro may be needed is if it is not possible to find a
single type for all of the possible argument values, or if some argument
values are assigned to by the macro definition, and not just read.

I would have suggested to look at all of the uses to see if there are any
concerns like this, but if there aren't any uses, that won't be possible.
There seems to be no special knowledge in these macros that is worth
preserving, so I think that they can just be dropped.

julia