netdev - Re: [PATCH ethtool-next 2/8] sff-common: Move OFFSET_TO_U16

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: <20211202215507.fymmrckusn5y4rqh@lion.mk-sys.cz>
Date:   Thu, 2 Dec 2021 22:55:07 +0100
From:   Michal Kubecek <mkubecek@...e.cz>
To:     Ido Schimmel <idosch@...sch.org>
Cc:     netdev@...r.kernel.org, popadrian1996@...il.com, andrew@...n.ch,
        mlxsw@...dia.com, moshe@...dia.com,
        Ido Schimmel <idosch@...dia.com>
Subject: Re: [PATCH ethtool-next 2/8] sff-common: Move OFFSET_TO_U16_PTR() to
 common header file

On Tue, Nov 23, 2021 at 07:40:56PM +0200, Ido Schimmel wrote:
> From: Ido Schimmel <idosch@...dia.com>
> 
> The define is also useful for CMIS, so move it from SFF-8636 to the
> common header file.
> 
> Signed-off-by: Ido Schimmel <idosch@...dia.com>
> ---
>  qsfp.c       | 1 -
>  sff-common.h | 4 ++--
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/qsfp.c b/qsfp.c
> index 58c4c4775e9b..b3c9e1516af9 100644
> --- a/qsfp.c
> +++ b/qsfp.c
> @@ -700,7 +700,6 @@ sff8636_show_wavelength_or_copper_compliance(const struct sff8636_memory_map *ma
>   * Second byte are 1/256th of degree, which are added to the dec part.
>   */
>  #define SFF8636_OFFSET_TO_TEMP(offset) ((__s16)OFFSET_TO_U16(offset))
> -#define OFFSET_TO_U16_PTR(ptr, offset) (ptr[offset] << 8 | ptr[(offset) + 1])
>  
>  static void sff8636_dom_parse(const struct sff8636_memory_map *map,
>  			      struct sff_diags *sd)
> diff --git a/sff-common.h b/sff-common.h
> index aab306e0b74f..9e323008ba19 100644
> --- a/sff-common.h
> +++ b/sff-common.h
> @@ -126,8 +126,8 @@
>  #define  SFF8024_ENCODING_PAM4			0x08
>  
>  /* Most common case: 16-bit unsigned integer in a certain unit */
> -#define OFFSET_TO_U16(offset) \
> -		(id[offset] << 8 | id[(offset) + 1])
> +#define OFFSET_TO_U16_PTR(ptr, offset) (ptr[offset] << 8 | ptr[(offset) + 1])
> +#define OFFSET_TO_U16(offset) OFFSET_TO_U16_PTR(id, offset)

I'm aware that you are just moving an old piece of code around so this
is rather meant as a tip for a future cleanup: OFFSET_TO_U16_PTR() does
not really need to be a macro so an inline function would be cleaner
(and type safe). OFFSET_TO_U16() is only used in 4 places, AFAICS, so
it is IMHO questionable if it is worth the worse readability.

Michal

>  
>  # define PRINT_xX_PWR(string, var)                             \
>  		printf("\t%-41s : %.4f mW / %.2f dBm\n", (string),         \
> -- 
> 2.31.1
> 

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)