Skip to contentSkip to conversation

wil6210: Avoid the use of one-element array

PATCH In review Gustavo A. R. Silva 4 participants 7 messages Jul 15, 2020 – Aug 14, 2020

Subsystems (3)
Networking Network drivers Wilocity WIL6210 wireless driver
lore
Share & tools
lore

Gustavo A. R. Silva

author
#0 lore raw

PATCH 1/1 wil6210: Avoid the use of one-element array

One-element arrays are being deprecated[1]. Replace the one-element array with a simple value type 'u8 reserved'[2], once this is just a placeholder for alignment.

[1] https://github.com/KSPP/linux/issues/79
[2] https://github.com/KSPP/linux/issues/86

Tested-by kernel test robot <[email protected]>
Link https://github.com/GustavoARSilva/linux-hardening/blob/master/cii/0-day/wil6210-20200715.md
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/net/wireless/ath/wil6210/wmi.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/wireless/ath/wil6210/wmi.h +1 -1
index 9affa4525609..beb53cac329b 100644
@@ -3086,7 +3086,7 @@ struct wmi_scheduling_scheme_event {
 	/* wmi_sched_scheme_failure_type */
 	u8 failure_type;
 	/* alignment to 32b */
-	u8 reserved[1];
+	u8 reserved;
 } __packed;
 
 /* WMI_RS_CFG_CMDID - deprecated */
Signature · 1 line
2.27.0
#1 lore raw

Hi all,

Friendly ping: who can take this? :)

Thanks
--
Gustavo

Quoted 28 lines · Gustavo A. R. Silva

One-element arrays are being deprecated[1]. Replace the one-element array with a simple value type 'u8 reserved'[2], once this is just a placeholder for alignment.

[1] https://github.com/KSPP/linux/issues/79
[2] https://github.com/KSPP/linux/issues/86

Tested-by kernel test robot <[email protected]>
Link https://github.com/GustavoARSilva/linux-hardening/blob/master/cii/0-day/wil6210-20200715.md
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/net/wireless/ath/wil6210/wmi.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/wireless/ath/wil6210/wmi.h +1 -1
index 9affa4525609..beb53cac329b 100644
@@ -3086,7 +3086,7 @@ struct wmi_scheduling_scheme_event {
 	/* wmi_sched_scheme_failure_type */
 	u8 failure_type;
 	/* alignment to 32b */
-	u8 reserved[1];
+	u8 reserved;
 } __packed;
 
 /* WMI_RS_CFG_CMDID - deprecated */
#2 lore raw
Quoted 1 line · Gustavo A. R. Silva

Friendly ping: who can take this? :)

I'll take this if it's ok. I have been just busy due to vacation period.

Signature · 1 line
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
#3 lore raw
Quoted 6 lines · Kalle Valo
Quoted 1 line · Gustavo A. R. Silva

Friendly ping: who can take this? :)

I'll take this if it's ok. I have been just busy due to vacation period.

Thanks, Kalle. :)

--
Gustavo

#4 lore raw
In reply to Gustavo A. R. Silva
Quoted 6 lines · Gustavo A. R. Silva

One-element arrays are being deprecated[1]. Replace the one-element array with a simple value type 'u8 reserved'[2], once this is just a placeholder for alignment.

[1] https://github.com/KSPP/linux/issues/79
[2] https://github.com/KSPP/linux/issues/86

Umm, no, you're misinterpreting this ... This has nothing to do with variable length and isn't used that way. As you can see from your own patch, since you're not changing any users.

johannes

#5 lore raw
Quoted 5 lines · Johannes Berg
Quoted 4 lines · Gustavo A. R. Silva

One-element arrays are being deprecated[1]. Replace the one-element array with a simple value type 'u8 reserved'[2], once this is just a placeholder for alignment.

Quoted 7 lines
Quoted 2 lines

[1] https://github.com/KSPP/linux/issues/79
[2] https://github.com/KSPP/linux/issues/86

Umm, no, you're misinterpreting this ... This has nothing to do with variable length and isn't used that way. As you can see from your own patch, since you're not changing any users.

I understand that this 'reserved' field is being used merely for alignment purposes:

drivers/net/wireless/ath/wil6210/wmi.h:

3080 /* WMI_SCHEDULING_SCHEME_EVENTID */
3081 struct wmi_scheduling_scheme_event {

3082         /* wmi_fw_status_e */
3083         u8 status;
3084         /* serial number given in command */
3085         u8 serial_num;
3086         /* wmi_sched_scheme_failure_type */
3087         u8 failure_type;
3088         /* alignment to 32b */
3089         u8 reserved[1];

3090 } __packed;

and that it's not being used as a variable-length array. The links are included to inform the people about the reason why we are removing zero-length/one-element arrays in the first place. The thing in this case is that it doesn't hurt to use a simple value type instead of the one-element array form.

Thanks
--
Gustavo

#6 lore raw
In reply to Gustavo A. R. Silva
Quoted 11 lines · Gustavo A. R. Silva

One-element arrays are being deprecated[1]. Replace the one-element array with a simple value type 'u8 reserved'[2], once this is just a placeholder for alignment.

[1] https://github.com/KSPP/linux/issues/79
[2] https://github.com/KSPP/linux/issues/86

Tested-by kernel test robot <[email protected]>
Link https://github.com/GustavoARSilva/linux-hardening/blob/master/cii/0-day/wil6210-20200715.md
Signed-off-by Gustavo A. R. Silva <[email protected]>
Signed-off-by Kalle Valo <[email protected]>

I agree with Johannes, I don't see the point of this patch as there are no clear benefits.

Patch set to Rejected.

Signature · 3 lines
https://patchwork.kernel.org/patch/11666325/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches