diff mbox series

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

Message ID 20250416115900.2491661-6-andrew.cooper3@citrix.com (mailing list archive)
State New
Headers show
Series xen: Centralise byteswap infrastructure | expand

Commit Message

Andrew Cooper April 16, 2025, 11:58 a.m. UTC 
From: Lin Liu <lin.liu@citrix.com>

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@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Anthony PERARD <anthony.perard@vates.tech>
CC: Michal Orzel <michal.orzel@amd.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Julien Grall <julien@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Bertrand Marquis <bertrand.marquis@arm.com>
CC: Shawn Anastasio <sanastasio@raptorengineering.com>
CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>
CC: Daniel P. Smith <dpsmith@apertussolutions.com>
CC: Lin Liu <lin.liu@citrix.com>

v6:
 * Fix lz4 and lzo1x too.

v5:
 * Rebase
 * Rearranged from other patches
---
 tools/libs/guest/xg_dom_decompress_lz4.c          | 10 ++++++++--
 tools/libs/guest/xg_dom_decompress_unsafe_lzo1x.c | 14 ++++----------
 tools/libs/guest/xg_dom_decompress_unsafe_xz.c    | 13 +++++++------
 xen/common/lz4/defs.h                             |  6 +++++-
 xen/common/unlzo.c                                | 12 ++++++++++--
 xen/common/xz/private.h                           | 12 +++++++++---
 6 files changed, 43 insertions(+), 24 deletions(-)

Comments

Jan Beulich April 16, 2025, 2:03 p.m. UTC | #1 
On 16.04.2025 13:58, Andrew Cooper wrote:
> From: Lin Liu <lin.liu@citrix.com>
> 
> 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@citrix.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com> # 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
diff mbox series

Patch

diff --git a/tools/libs/guest/xg_dom_decompress_lz4.c b/tools/libs/guest/xg_dom_decompress_lz4.c
index b26cce3ec532..53ef0bf328ed 100644
--- a/tools/libs/guest/xg_dom_decompress_lz4.c
+++ b/tools/libs/guest/xg_dom_decompress_lz4.c
@@ -3,6 +3,8 @@ 
 #include <inttypes.h>
 #include <stdint.h>
 
+#include INCLUDE_ENDIAN_H
+
 #define XG_NEED_UNALIGNED
 #include "xg_private.h"
 #include "xg_dom_decompress.h"
@@ -17,9 +19,13 @@  typedef uint64_t u64;
 #define likely(a) a
 #define unlikely(a) a
 
-static inline uint_fast16_t le16_to_cpup(const unsigned char *buf)
+static inline uint16_t le16_to_cpu(uint16_t v)
 {
-    return buf[0] | (buf[1] << 8);
+#if BYTE_ORDER == BIG_ENDIAN
+    return __builtin_bswap16(v);
+#else
+    return v;
+#endif
 }
 
 #include "../../xen/include/xen/lz4.h"
diff --git a/tools/libs/guest/xg_dom_decompress_unsafe_lzo1x.c b/tools/libs/guest/xg_dom_decompress_unsafe_lzo1x.c
index e58c1b95ed17..3f6b133ccf9d 100644
--- a/tools/libs/guest/xg_dom_decompress_unsafe_lzo1x.c
+++ b/tools/libs/guest/xg_dom_decompress_unsafe_lzo1x.c
@@ -16,25 +16,19 @@  typedef uint64_t u64;
 #define noinline
 #define unlikely(a) a
 
-static inline u16 be16_to_cpup(const u16 *p)
+static inline u16 be16_to_cpu(const u16 v)
 {
-	u16 v = *p;
 #if BYTE_ORDER == LITTLE_ENDIAN
-	return (((v & 0x00ffU) << 8) |
-                ((v & 0xff00U) >> 8));
+	return __builtin_bswap16(v);
 #else
 	return v;
 #endif
 }
 
-static inline u32 be32_to_cpup(const u32 *p)
+static inline u32 be32_to_cpup(const u32 v)
 {
-	u32 v = *p;
 #if BYTE_ORDER == LITTLE_ENDIAN
-	return (((v & 0x000000ffUL) << 24) |
-                ((v & 0x0000ff00UL) <<  8) |
-                ((v & 0x00ff0000UL) >>  8) |
-                ((v & 0xff000000UL) >> 24));
+	return __builtin_bswap32(v);
 #else
 	return v;
 #endif
diff --git a/tools/libs/guest/xg_dom_decompress_unsafe_xz.c b/tools/libs/guest/xg_dom_decompress_unsafe_xz.c
index 80eed912dd68..7dbd2622c3b8 100644
--- 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);
 #else
 	return v;
 #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
 }
 
 #define __force
diff --git a/xen/common/lz4/defs.h b/xen/common/lz4/defs.h
index ecfbf07f8323..e477806634c1 100644
--- 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;
+
+	memcpy(&v, p, sizeof(v));
+
+	return le16_to_cpu(v);
 }
 
 #endif
diff --git a/xen/common/unlzo.c b/xen/common/unlzo.c
index acb8dff600fc..17efb1cc8f1d 100644
--- a/xen/common/unlzo.c
+++ b/xen/common/unlzo.c
@@ -39,12 +39,20 @@ 
 
 static inline u16 get_unaligned_be16(const void *p)
 {
-	return be16_to_cpup(p);
+	u16 v;
+
+	memcpy(&v, p, sizeof(v));
+
+	return be16_to_cpu(v);
 }
 
 static inline u32 get_unaligned_be32(const void *p)
 {
-	return be32_to_cpup(p);
+	u32 v;
+
+	memcpy(&v, p, sizeof(v));
+
+	return be32_to_cpu(v);
 }
 
 #endif
diff --git a/xen/common/xz/private.h b/xen/common/xz/private.h
index 2299705378ac..a63379994fd6 100644
--- a/xen/common/xz/private.h
+++ b/xen/common/xz/private.h
@@ -18,17 +18,23 @@ 
 
 static inline u32 get_unaligned_le32(const void *p)
 {
-	return le32_to_cpup(p);
+	u32 v;
+
+	memcpy(&v, p, sizeof(v));
+
+	return le32_to_cpu(v);
 }
 
 static inline void put_unaligned_le32(u32 val, void *p)
 {
-	*(__force __le32*)p = cpu_to_le32(val);
+	u32 v = cpu_to_le32(val);
+
+	memcpy(p, &v, sizeof(v));
 }
 
 #endif
 
-#define get_le32(p) le32_to_cpup((const uint32_t *)(p))
+#define get_le32(p) le32_to_cpu(*(const uint32_t *)(p))
 
 #define false 0
 #define true 1