scsi: sd: Avoid -Wflex-array-member-not-at-end warning
PATCH v2 1/1 scsi: sd: Avoid -Wflex-array-member-not-at-end warning
Apparently, the flexible-array member in struct scsi_stream_status_header is not actually needed. Remove it along with the related KUnit test.
So, with these changes, fix the following warning:
drivers/scsi/sd.c:3195:50: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
---
Changes in v2:
- Revome flexible-array member. (Christoph)
v1:
- Link: https://lore.kernel.org/linux-hardening/aAwos0mLxneG9R_t@kspp/
drivers/scsi/scsi_proto_test.c | 8 -------- drivers/scsi/sd.c | 2 +- include/scsi/scsi_proto.h | 1 - 3 files changed, 1 insertion(+), 10 deletions(-)
drivers/scsi/scsi_proto_test.c +0 -8
@@ -30,14 +30,6 @@ static void test_scsi_proto(struct kunit *test)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 )]; - } sh = { .arr = {1 ,2 ,3 ,4 ,0 ,0 ,5 ,6 } }; -KUNIT_EXPECT_EQ (test,get_unaligned_be32 (&sh.h.len),0x1020304 ); -KUNIT_EXPECT_EQ (test,get_unaligned_be16 (&sh.h.number_of_open_streams), -0x506 ); }static struct kunit_case scsi_proto_test_cases[] = {
drivers/scsi/sd.c +1 -1
@@ -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.h.stream_status[0 ].perm; +return buf.s.perm; }static void sd_read_io_hints (struct scsi_disk *sdkp,unsigned char *buffer)
include/scsi/scsi_proto.h +0 -1
@@ -349,7 +349,6 @@ struct scsi_stream_status_header {__be32 len;/* length in bytes of stream_status[] array. */ u16 reserved;__be16 number_of_open_streams; - DECLARE_FLEX_ARRAY(struct scsi_stream_status, stream_status); };static_assert (sizeof (struct scsi_stream_status_header ) ==8 );
Signature · 1 line
2.43.0
I actually send a patch for this yesterday, also keeping the proto test that actually still works despite my initial overreaching removal. (I still wish we'd kill the stupid struct and this test and just used the simpler and cleaner bitshifting and masking)
Quoted 2 lines · Christoph Hellwig
(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.
Bart.
Quoted 6 lines · Bart Van Assche
Quoted 2 lines · Christoph Hellwig
(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).
drivers/scsi/scsi_debug.c +2 -1
@@ -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 ;
drivers/scsi/scsi_proto_test.c +0 -9
@@ -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)];
drivers/scsi/sd.c +1 -1
@@ -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)
include/scsi/scsi_proto.h +3 -19
@@ -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 ]; };