linux-kernel - Re: [PATCH] staging: rtl8723bs: core: Replace macros RotR1 through Mk16 with static inline functions

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: <Y045f/UzV8Dx5tpp@kroah.com>
Date:   Tue, 18 Oct 2022 07:28:31 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Brent Pappas <bpappas@...pasbrent.com>
Cc:     linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org,
        Brent Pappas <pappasbrent@...il.com>
Subject: Re: [PATCH] staging: rtl8723bs: core: Replace macros RotR1 through
 Mk16 with static inline functions

On Mon, Oct 17, 2022 at 01:16:54PM -0400, Brent Pappas wrote:
> From: Brent Pappas <pappasbrent@...il.com>
> 
> Replace macros "RotR1", "Lo8", "Hi8", "Lo16", "Hi16", and "Mk16" with
> static inline functions to comply with Linux coding style standards.
> 
> Signed-off-by: Brent Pappas <bpappas@...pasbrent.com>
> ---
>  drivers/staging/rtl8723bs/core/rtw_security.c | 35 +++++++++++++++----
>  1 file changed, 29 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c
> index ac731415f733..519e141fb82c 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_security.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_security.c
> @@ -253,12 +253,35 @@ void rtw_seccalctkipmic(u8 *key, u8 *header, u8 *data, u32 data_len, u8 *mic_cod
>  }
>  
>  /* macros for extraction/creation of unsigned char/unsigned short values  */
> -#define RotR1(v16)   ((((v16) >> 1) & 0x7FFF) ^ (((v16) & 1) << 15))
> -#define   Lo8(v16)   ((u8)((v16)       & 0x00FF))
> -#define   Hi8(v16)   ((u8)(((v16) >> 8) & 0x00FF))
> -#define  Lo16(v32)   ((u16)((v32)       & 0xFFFF))
> -#define  Hi16(v32)   ((u16)(((v32) >> 16) & 0xFFFF))
> -#define  Mk16(hi, lo) ((lo) ^ (((u16)(hi)) << 8))
> +static inline u16 RotR1(u16 v16)
> +{
> +	return ((((v16) >> 1) & 0x7FFF) ^ (((v16) & 1) << 15));
> +}
> +
> +static inline u8 Lo8(u16 v16)
> +{
> +	return ((u8)((v16)       & 0x00FF));

Odd use of spaces, doesn't checkpatch complain about this?

But the larger question is, don't we already have functions for this in
the core kernel?  Why not just use them instead of hand-rolling custom
functions instead?

thanks,

greg k-h