[coreboot-gerrit] Change in coreboot[master]: commonlib/lz4_wrapper.c: do not use bitfields

[Krystian Hebel (Code Review)]

Krystian Hebel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/55040 )

Change subject: commonlib/lz4_wrapper.c: do not use bitfields
......................................................................

commonlib/lz4_wrapper.c: do not use bitfields

Order of bits in bitfields is implementation-defined. This makes them
non-portable, especially across systems using different endianness.

This change removes bitfields and uses masking and shifting instead.

Signed-off-by: Krystian Hebel 
Change-Id: Ief7d87ddb25c9baa931f27dbd54a4ca730b6ece7
---
M src/commonlib/bsd/lz4_wrapper.c
1 file changed, 33 insertions(+), 39 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/55040/1

diff --git a/src/commonlib/bsd/lz4_wrapper.c b/src/commonlib/bsd/lz4_wrapper.c
index 73185a5..73a3571 100644
--- a/src/commonlib/bsd/lz4_wrapper.c
+++ b/src/commonlib/bsd/lz4_wrapper.c
@@ -63,39 +63,32 @@

#define LZ4F_MAGICNUMBER 0x184D2204

+/* Bit field endianness is implementation-defined. Use masks instead.
+ * https://stackoverflow.com/a/6044223 */
+#define RESERVED0		0x03
+#define HAS_CONTENT_CHECKSUM	0x04
+#define HAS_CONTENT_SIZE	0x08
+#define HAS_BLOCK_CHECKSUM	0x10
+#define INDEPENDENT_BLOCKS	0x20
+#define VERSION			0xC0
+#define VERSION_SHIFT		6
+
+#define RESERVED1_2		0x8F
+#define MAX_BLOCK_SIZE		0x70
+
 struct lz4_frame_header {
    uint32_t magic;
-	union {
-		uint8_t flags;
-		struct {
-			uint8_t reserved0		: 2;
-			uint8_t has_content_checksum	: 1;
-			uint8_t has_content_size	: 1;
-			uint8_t has_block_checksum	: 1;
-			uint8_t independent_blocks	: 1;
-			uint8_t version			: 2;
-		};
-	};
-	union {
-		uint8_t block_descriptor;
-		struct {
-			uint8_t reserved1		: 4;
-			uint8_t max_block_size		: 3;
-			uint8_t reserved2		: 1;
-		};
-	};
+	uint8_t flags;
+	uint8_t block_descriptor;
    /* + uint64_t content_size iff has_content_size is set */
    /* + uint8_t header_checksum */
 } __packed;

+#define BH_SIZE			0x7FFFFFFF
+#define NOT_COMPRESSED		0x80000000
+
 struct lz4_block_header {
-	union {
-		uint32_t raw;
-		struct {
-			uint32_t size		: 31;
-			uint32_t not_compressed	: 1;
-		};
-	};
+	uint32_t raw;
    /* + size bytes of data */
    /* + uint32_t block_checksum iff has_block_checksum is set */
 } __packed;
@@ -114,16 +107,17 @@
    		return 0;	/* input overrun */

/* We assume there's always only a single, standard frame. */
-		if (le32toh(h->magic) != LZ4F_MAGICNUMBER || h->version != 1)
+		if (le32toh(h->magic) != LZ4F_MAGICNUMBER
+		    || (h->flags & VERSION) != (1 << VERSION_SHIFT))
    		return 0;	/* unknown format */
-		if (h->reserved0 || h->reserved1 || h->reserved2)
+		if ((h->flags & RESERVED0) || (h->block_descriptor & RESERVED1_2))
    		return 0;	/* reserved must be zero */
-		if (!h->independent_blocks)
+		if (!(h->flags & INDEPENDENT_BLOCKS))
    		return 0;	/* we don't support block dependency */
-		has_block_checksum = h->has_block_checksum;
+		has_block_checksum = h->flags & HAS_BLOCK_CHECKSUM;

in += sizeof(*h);
-		if (h->has_content_size)
+		if (h->flags & HAS_CONTENT_SIZE)
    		in += sizeof(uint64_t);
    	in += sizeof(uint8_t);
    }
@@ -133,28 +127,28 @@
    		break;          /* input overrun */

struct lz4_block_header b = {
-			{ .raw = le32toh(*(const uint32_t *)in) }
+			.raw = le32toh(*(const uint32_t *)in)
    	};
    	in += sizeof(struct lz4_block_header);

-		if ((size_t)(in - src) + b.size > srcn)
+		if ((size_t)(in - src) + (b.raw & BH_SIZE) > srcn)
    		break;			/* input overrun */

-		if (!b.size) {
+		if (!(b.raw & BH_SIZE)) {
    		out_size = out - dst;
    		break;			/* decompression successful */
    	}

-		if (b.not_compressed) {
-			size_t size = MIN((uintptr_t)b.size, (uintptr_t)dst
+		if (b.raw & NOT_COMPRESSED) {
+			size_t size = MIN((uintptr_t)(b.raw & BH_SIZE), (uintptr_t)dst
    			+ dstn - (uintptr_t)out);
    		memcpy(out, in, size);
-			if (size < b.size)
+			if (size < (b.raw & BH_SIZE))
    			break;		/* output overrun */
    		out += size;
    	} else {
    		/* constant folding essential, do not touch params! */
-			int ret = LZ4_decompress_generic(in, out, b.size,
+			int ret = LZ4_decompress_generic(in, out, (b.raw & BH_SIZE),
    				dst + dstn - out, endOnInputSize,
    				full, 0, noDict, out, NULL, 0);
    		if (ret < 0)
@@ -162,7 +156,7 @@
    		out += ret;
    	}

-		in += b.size;
+		in += (b.raw & BH_SIZE);
    	if (has_block_checksum)
    		in += sizeof(uint32_t);
    }