Re: [PATCH v2][next] scsi: sd: Avoid -Wflex-array-member-not-at-end warning

[Christoph Hellwig]

On Sat, May 03, 2025 at 12:08:23PM -0700, Bart Van Assche wrote:
> On 5/1/25 11:50 PM, Christoph Hellwig wrote:
> > (I still wish we'd kill the stupid struct and this test and just used
> > the simpler and cleaner bitshifting and masking)
> 
> Bit-shifting and masking results in faster code. For slow path code I
> prefer bitfields because this results in much more compact source code.

Despite claims to the contrary bit tests and masking are often a lot
simpler, and you don't need the duplicate struct layouts and tests
verifying that you get them right as it is almost impossible to write
code that only works on LE or BE.  E.g. the patch below would simplify
the stream status quite a bit.  It also fixes the assumption that the
bitfield endianess is the same as the byte endianess, which is not
true in general (although I think it is for all Linux supported
architectures).

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index f0eec4708ddd..41ed22f1ad0b 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -5528,7 +5528,8 @@ static int resp_get_stream_status(struct scsi_cmnd *scp,
 	     offset += 8, stream_id++) {
 		struct scsi_stream_status *stream_status = (void *)arr + offset;
 
-		stream_status->perm = stream_id < PERMANENT_STREAM_COUNT;
+		if (stream_id < PERMANENT_STREAM_COUNT)
+			stream_status->flags |= SCSI_STREAM_STATUS_PERM;
 		put_unaligned_be16(stream_id,
 				   &stream_status->stream_identifier);
 		stream_status->rel_lifetime = stream_id + 1;
diff --git a/drivers/scsi/scsi_proto_test.c b/drivers/scsi/scsi_proto_test.c
index c093389edabb..d61302e7715a 100644
--- a/drivers/scsi/scsi_proto_test.c
+++ b/drivers/scsi/scsi_proto_test.c
@@ -22,15 +22,6 @@ static void test_scsi_proto(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, d.desc.params[0] + 0, 0xe4);
 	KUNIT_EXPECT_EQ(test, d.desc.params[1] + 0, 0xe3);
 
-	static const union {
-		struct scsi_stream_status s;
-		u8 arr[sizeof(struct scsi_stream_status)];
-	} ss = { .arr = { 0x80, 0, 0x12, 0x34, 0x3f } };
-	KUNIT_EXPECT_EQ(test, ss.s.perm + 0, 1);
-	KUNIT_EXPECT_EQ(test, get_unaligned_be16(&ss.s.stream_identifier),
-			0x1234);
-	KUNIT_EXPECT_EQ(test, ss.s.rel_lifetime + 0, 0x3f);
-
 	static const union {
 		struct scsi_stream_status_header h;
 		u8 arr[sizeof(struct scsi_stream_status_header)];
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 3f6e87705b62..deb8af152f13 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3215,7 +3215,7 @@ static bool sd_is_perm_stream(struct scsi_disk *sdkp, unsigned int stream_id)
 		return false;
 	if (get_unaligned_be32(&buf.h.len) < sizeof(struct scsi_stream_status))
 		return false;
-	return buf.s.perm;
+	return buf.s.flags & SCSI_STREAM_STATUS_PERM;
 }
 
 static void sd_read_io_hints(struct scsi_disk *sdkp, unsigned char *buffer)
diff --git a/include/scsi/scsi_proto.h b/include/scsi/scsi_proto.h
index f64385cde5b9..88c1d950be54 100644
--- a/include/scsi/scsi_proto.h
+++ b/include/scsi/scsi_proto.h
@@ -319,26 +319,10 @@ static_assert(sizeof(struct scsi_io_group_descriptor) == 16);
 
 /* SCSI stream status descriptor */
 struct scsi_stream_status {
-#if defined(__BIG_ENDIAN)
-	u8 perm: 1;
-	u8 reserved1: 7;
-#elif defined(__LITTLE_ENDIAN)
-	u8 reserved1: 7;
-	u8 perm: 1;
-#else
-#error
-#endif
-	u8 reserved2;
+	u8 flags;
+#define SCSI_STREAM_STATUS_PERM		(1u << 7)
 	__be16 stream_identifier;
-#if defined(__BIG_ENDIAN)
-	u8 reserved3: 2;
-	u8 rel_lifetime: 6;
-#elif defined(__LITTLE_ENDIAN)
-	u8 rel_lifetime: 6;
-	u8 reserved3: 2;
-#else
-#error
-#endif
+	u8 rel_lifetime;
 	u8 reserved4[3];
 };