| Message ID | 20231213-wmi_host_mem_chunks_flexarray-v1-2-92922d92fa2c@quicinc.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 72ca7c4073ac126be1c2341644838ef5f146b36b |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wifi: ath10k: use flexible arrays | expand |
On Wed, Dec 13, 2023 at 09:06:40AM -0800, Jeff Johnson wrote: > Currently ath10k defines the following struct: > struct wmi_start_scan_tlvs { > u8 tlvs[0]; > } __packed; > > Per the guidance in [1] this should be a flexible array. However, a > direct replace to u8 tlvs[] results in the compilation error: > flexible array member in a struct with no named members > > This is because C99 6.7.2.1 (16) requires that a structure containing > a flexible array member must have more than one named member. > > So rather than defining a separate struct wmi_start_scan_tlvs which > contains the flexible tlvs[] array, just define the tlvs[] array where > struct wmi_start_scan_tlvs is being used. > > No functional changes, compile tested only. > > [1] https://docs.kernel.org/process/deprecated.html#zero-length-and-one-element-arrays > > Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com> Yeah, this too looks like the right approach: keeping the flex array close instead of externalized into a no-op struct. Reviewed-by: Kees Cook <keescook@chromium.org>
On 12/13/23 11:06, Jeff Johnson wrote: > Currently ath10k defines the following struct: > struct wmi_start_scan_tlvs { > u8 tlvs[0]; > } __packed; > > Per the guidance in [1] this should be a flexible array. However, a > direct replace to u8 tlvs[] results in the compilation error: > flexible array member in a struct with no named members > > This is because C99 6.7.2.1 (16) requires that a structure containing > a flexible array member must have more than one named member. > > So rather than defining a separate struct wmi_start_scan_tlvs which > contains the flexible tlvs[] array, just define the tlvs[] array where > struct wmi_start_scan_tlvs is being used. > > No functional changes, compile tested only. > > [1] https://docs.kernel.org/process/deprecated.html#zero-length-and-one-element-arrays > > Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com> Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org> Thanks! -- Gustavo > --- > drivers/net/wireless/ath/ath10k/wmi.c | 8 ++++---- > drivers/net/wireless/ath/ath10k/wmi.h | 11 ++--------- > 2 files changed, 6 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c > index 88befe92f95d..4d5aadbc7159 100644 > --- a/drivers/net/wireless/ath/ath10k/wmi.c > +++ b/drivers/net/wireless/ath/ath10k/wmi.c > @@ -6927,14 +6927,14 @@ void ath10k_wmi_put_start_scan_common(struct wmi_start_scan_common *cmn, > } > > static void > -ath10k_wmi_put_start_scan_tlvs(struct wmi_start_scan_tlvs *tlvs, > +ath10k_wmi_put_start_scan_tlvs(u8 *tlvs, > const struct wmi_start_scan_arg *arg) > { > struct wmi_ie_data *ie; > struct wmi_chan_list *channels; > struct wmi_ssid_list *ssids; > struct wmi_bssid_list *bssids; > - void *ptr = tlvs->tlvs; > + void *ptr = tlvs; > int i; > > if (arg->n_channels) { > @@ -7012,7 +7012,7 @@ ath10k_wmi_op_gen_start_scan(struct ath10k *ar, > cmd = (struct wmi_start_scan_cmd *)skb->data; > > ath10k_wmi_put_start_scan_common(&cmd->common, arg); > - ath10k_wmi_put_start_scan_tlvs(&cmd->tlvs, arg); > + ath10k_wmi_put_start_scan_tlvs(cmd->tlvs, arg); > > cmd->burst_duration_ms = __cpu_to_le32(0); > > @@ -7041,7 +7041,7 @@ ath10k_wmi_10x_op_gen_start_scan(struct ath10k *ar, > cmd = (struct wmi_10x_start_scan_cmd *)skb->data; > > ath10k_wmi_put_start_scan_common(&cmd->common, arg); > - ath10k_wmi_put_start_scan_tlvs(&cmd->tlvs, arg); > + ath10k_wmi_put_start_scan_tlvs(cmd->tlvs, arg); > > ath10k_dbg(ar, ATH10K_DBG_WMI, "wmi 10x start scan\n"); > return skb; > diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h > index 833ce0251a2c..52a409ff94e7 100644 > --- a/drivers/net/wireless/ath/ath10k/wmi.h > +++ b/drivers/net/wireless/ath/ath10k/wmi.h > @@ -3218,23 +3218,16 @@ struct wmi_start_scan_common { > __le32 scan_ctrl_flags; > } __packed; > > -struct wmi_start_scan_tlvs { > - /* TLV parameters. These includes channel list, ssid list, bssid list, > - * extra ies. > - */ > - u8 tlvs[0]; > -} __packed; > - > struct wmi_start_scan_cmd { > struct wmi_start_scan_common common; > __le32 burst_duration_ms; > - struct wmi_start_scan_tlvs tlvs; > + u8 tlvs[]; > } __packed; > > /* This is the definition from 10.X firmware branch */ > struct wmi_10x_start_scan_cmd { > struct wmi_start_scan_common common; > - struct wmi_start_scan_tlvs tlvs; > + u8 tlvs[]; > } __packed; > > struct wmi_ssid_arg { >
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c index 88befe92f95d..4d5aadbc7159 100644 --- a/drivers/net/wireless/ath/ath10k/wmi.c +++ b/drivers/net/wireless/ath/ath10k/wmi.c @@ -6927,14 +6927,14 @@ void ath10k_wmi_put_start_scan_common(struct wmi_start_scan_common *cmn, } static void -ath10k_wmi_put_start_scan_tlvs(struct wmi_start_scan_tlvs *tlvs, +ath10k_wmi_put_start_scan_tlvs(u8 *tlvs, const struct wmi_start_scan_arg *arg) { struct wmi_ie_data *ie; struct wmi_chan_list *channels; struct wmi_ssid_list *ssids; struct wmi_bssid_list *bssids; - void *ptr = tlvs->tlvs; + void *ptr = tlvs; int i; if (arg->n_channels) { @@ -7012,7 +7012,7 @@ ath10k_wmi_op_gen_start_scan(struct ath10k *ar, cmd = (struct wmi_start_scan_cmd *)skb->data; ath10k_wmi_put_start_scan_common(&cmd->common, arg); - ath10k_wmi_put_start_scan_tlvs(&cmd->tlvs, arg); + ath10k_wmi_put_start_scan_tlvs(cmd->tlvs, arg); cmd->burst_duration_ms = __cpu_to_le32(0); @@ -7041,7 +7041,7 @@ ath10k_wmi_10x_op_gen_start_scan(struct ath10k *ar, cmd = (struct wmi_10x_start_scan_cmd *)skb->data; ath10k_wmi_put_start_scan_common(&cmd->common, arg); - ath10k_wmi_put_start_scan_tlvs(&cmd->tlvs, arg); + ath10k_wmi_put_start_scan_tlvs(cmd->tlvs, arg); ath10k_dbg(ar, ATH10K_DBG_WMI, "wmi 10x start scan\n"); return skb; diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h index 833ce0251a2c..52a409ff94e7 100644 --- a/drivers/net/wireless/ath/ath10k/wmi.h +++ b/drivers/net/wireless/ath/ath10k/wmi.h @@ -3218,23 +3218,16 @@ struct wmi_start_scan_common { __le32 scan_ctrl_flags; } __packed; -struct wmi_start_scan_tlvs { - /* TLV parameters. These includes channel list, ssid list, bssid list, - * extra ies. - */ - u8 tlvs[0]; -} __packed; - struct wmi_start_scan_cmd { struct wmi_start_scan_common common; __le32 burst_duration_ms; - struct wmi_start_scan_tlvs tlvs; + u8 tlvs[]; } __packed; /* This is the definition from 10.X firmware branch */ struct wmi_10x_start_scan_cmd { struct wmi_start_scan_common common; - struct wmi_start_scan_tlvs tlvs; + u8 tlvs[]; } __packed; struct wmi_ssid_arg {
Currently ath10k defines the following struct: struct wmi_start_scan_tlvs { u8 tlvs[0]; } __packed; Per the guidance in [1] this should be a flexible array. However, a direct replace to u8 tlvs[] results in the compilation error: flexible array member in a struct with no named members This is because C99 6.7.2.1 (16) requires that a structure containing a flexible array member must have more than one named member. So rather than defining a separate struct wmi_start_scan_tlvs which contains the flexible tlvs[] array, just define the tlvs[] array where struct wmi_start_scan_tlvs is being used. No functional changes, compile tested only. [1] https://docs.kernel.org/process/deprecated.html#zero-length-and-one-element-arrays Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com> --- drivers/net/wireless/ath/ath10k/wmi.c | 8 ++++---- drivers/net/wireless/ath/ath10k/wmi.h | 11 ++--------- 2 files changed, 6 insertions(+), 13 deletions(-)