Re: [PATCH v6 05/15] xen/decompressors: Remove use of *_to_cpup() helpers

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.04.2025 13:58, Andrew Cooper wrote:
> From: Lin Liu <lin.liu@xxxxxxxxxx>
> 
> These wrappers simply hide a deference, which adds to the cognitive complexity
> of reading the code.  As such, they're not going to be included in the new
> byteswap infrastructure.
> 
> No functional change.
> 
> Signed-off-by: Lin Liu <lin.liu@xxxxxxxxxx>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx> # hypervisor
with a request below.

> --- a/tools/libs/guest/xg_dom_decompress_unsafe_xz.c
> +++ b/tools/libs/guest/xg_dom_decompress_unsafe_xz.c
> @@ -19,18 +19,19 @@ typedef uint32_t __le32;
>  static inline u32 cpu_to_le32(const u32 v)
>  {
>  #if BYTE_ORDER == BIG_ENDIAN
> -     return (((v & 0x000000ffUL) << 24) |
> -             ((v & 0x0000ff00UL) <<  8) |
> -             ((v & 0x00ff0000UL) >>  8) |
> -             ((v & 0xff000000UL) >> 24));
> +        return __builtin_bswap32(v);

Supposedly a hard tab is to be used for indentation here, if original code
and ...

>  #else
>       return v;

... this is to be trusted.

>  #endif
>  }
>  
> -static inline u32 le32_to_cpup(const u32 *p)
> +static inline u32 le32_to_cpu(const u32 p)
>  {
> -     return cpu_to_le32(*p);
> +#if BYTE_ORDER == BIG_ENDIAN
> +        return __builtin_bswap32(v);
> +#else
> +     return v;
> +#endif
>  }

Same here.

> --- a/xen/common/lz4/defs.h
> +++ b/xen/common/lz4/defs.h
> @@ -18,7 +18,11 @@
>  
>  static inline u16 get_unaligned_le16(const void *p)
>  {
> -     return le16_to_cpup(p);
> +     u16 v;

Here and below - I realize there's u16 in context (u32 elsewhere), yet it would
still be nice if no new instances appeared and you used uint16_t here (and
uint32_t in the other cases).

Jan