One-element arrays are obsolete, and flexible
array members have taken their place. So, in
struct cca_stats_n_flags, replace the one-element
array with a flexible-array member.
This fixes warnings such as:
./drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:119:6-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
Signed-off-by: Atul Raut <[email protected]>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index de8a2e27f49c..fff32e54833d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -116,7 +116,7 @@ struct brcmf_dump_survey {
struct cca_stats_n_flags {
u32 msrmnt_time; /* Time for Measurement (msec) */
u32 msrmnt_done; /* flag set when measurement complete */
- char buf[1];
+ char buf[];
};
struct cca_msrmnt_query {
--
2.34.1
Atul Raut <[email protected]> writes:
> One-element arrays are obsolete, and flexible
> array members have taken their place. So, in
> struct cca_stats_n_flags, replace the one-element
> array with a flexible-array member.
>
> This fixes warnings such as:
> ./drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:119:6-9:
> WARNING use flexible-array member instead
> (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
>
> Signed-off-by: Atul Raut <[email protected]>
I have now three patches from you in queue:
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/
v1 submitted twice and v2 without any change log. Please read the
documentation from our wiki link below how to submit patches.
Also I would like to have a review from someone else before I consider
taking this.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Atul Raut <[email protected]> wrote:
> One-element arrays are obsolete, and flexible
> array members have taken their place. So, in
> struct cca_stats_n_flags, replace the one-element
> array with a flexible-array member.
>
> This fixes warnings such as:
> ./drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:119:6-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
>
> Signed-off-by: Atul Raut <[email protected]>
Arend, what do you think? Link to the patch after "--" line.
--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
+ Kees
On 8/3/2023 7:22 AM, Atul Raut wrote:
> One-element arrays are obsolete, and flexible
> array members have taken their place. So, in
> struct cca_stats_n_flags, replace the one-element
> array with a flexible-array member.
>
> This fixes warnings such as:
> ./drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:119:6-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
I think this correct, but maybe Kees can give definitive answer here.
> Signed-off-by: Atul Raut <[email protected]>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index de8a2e27f49c..fff32e54833d 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -116,7 +116,7 @@ struct brcmf_dump_survey {
> struct cca_stats_n_flags {
> u32 msrmnt_time; /* Time for Measurement (msec) */
> u32 msrmnt_done; /* flag set when measurement complete */
> - char buf[1];
> + char buf[];
> };
>
> struct cca_msrmnt_query {
On Thu, Jan 18, 2024 at 02:53:28PM +0100, Arend van Spriel wrote:
> + Kees
>
> On 8/3/2023 7:22 AM, Atul Raut wrote:
> > One-element arrays are obsolete, and flexible
> > array members have taken their place. So, in
> > struct cca_stats_n_flags, replace the one-element
> > array with a flexible-array member.
> >
> > This fixes warnings such as:
> > ./drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:119:6-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
>
> I think this correct, but maybe Kees can give definitive answer here.
>
> > Signed-off-by: Atul Raut <[email protected]>
> > ---
> > drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > index de8a2e27f49c..fff32e54833d 100644
> > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > @@ -116,7 +116,7 @@ struct brcmf_dump_survey {
> > struct cca_stats_n_flags {
> > u32 msrmnt_time; /* Time for Measurement (msec) */
> > u32 msrmnt_done; /* flag set when measurement complete */
> > - char buf[1];
> > + char buf[];
> > };
> > struct cca_msrmnt_query {
Normally a [1] -> [] conversion needs some details in the commit log
about why this is safe (since it runs the risk of changing sizeof(struct
cca_stats_n_flags). In this case, there's only a single user of the
struct, and nothing about its use depends on its size:
#define BRCMF_DCMD_MEDLEN 1536
...
struct cca_stats_n_flags *results;
char *buf;
...
buf = kzalloc(sizeof(char) * BRCMF_DCMD_MEDLEN, GFP_KERNEL);
...
results = (struct cca_stats_n_flags *)(buf);
...
brcmf_parse_dump_obss(results->buf, survey);
So, the allocation size is big enough for the struct, and nothing
depends on the struct size.
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
On January 18, 2024 9:19:52 PM Kees Cook <[email protected]> wrote:
> On Thu, Jan 18, 2024 at 02:53:28PM +0100, Arend van Spriel wrote:
>> + Kees
>>
>> On 8/3/2023 7:22 AM, Atul Raut wrote:
>>> One-element arrays are obsolete, and flexible
>>> array members have taken their place. So, in
>>> struct cca_stats_n_flags, replace the one-element
>>> array with a flexible-array member.
>>>
>>> This fixes warnings such as:
>>> ./drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:119:6-9:
>>> WARNING use flexible-array member instead
>>> (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
>>
>> I think this correct, but maybe Kees can give definitive answer here.
>>
>>> Signed-off-by: Atul Raut <[email protected]>
>>> ---
>>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> index de8a2e27f49c..fff32e54833d 100644
>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> @@ -116,7 +116,7 @@ struct brcmf_dump_survey {
>>> struct cca_stats_n_flags {
>>> u32 msrmnt_time; /* Time for Measurement (msec) */
>>> u32 msrmnt_done; /* flag set when measurement complete */
>>> - char buf[1];
>>> + char buf[];
>>> };
>>> struct cca_msrmnt_query {
>
> Normally a [1] -> [] conversion needs some details in the commit log
> about why this is safe (since it runs the risk of changing sizeof(struct
> cca_stats_n_flags). In this case, there's only a single user of the
> struct, and nothing about its use depends on its size:
>
> #define BRCMF_DCMD_MEDLEN 1536
> ...
> struct cca_stats_n_flags *results;
> char *buf;
> ...
> buf = kzalloc(sizeof(char) * BRCMF_DCMD_MEDLEN, GFP_KERNEL);
> ...
> results = (struct cca_stats_n_flags *)(buf);
> ...
> brcmf_parse_dump_obss(results->buf, survey);
>
> So, the allocation size is big enough for the struct, and nothing
> depends on the struct size.
>
> Reviewed-by: Kees Cook <[email protected]>
Thanks for the confirmation.
Regards,
Arend
> --
> Kees Cook
On Thu, Jan 18, 2024 at 9:20 PM Kees Cook <[email protected]> wrote:
>
> On Thu, Jan 18, 2024 at 02:53:28PM +0100, Arend van Spriel wrote:
> > + Kees
> >
> > On 8/3/2023 7:22 AM, Atul Raut wrote:
> > > One-element arrays are obsolete, and flexible
> > > array members have taken their place. So, in
> > > struct cca_stats_n_flags, replace the one-element
> > > array with a flexible-array member.
> > >
> > > This fixes warnings such as:
> > > ./drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:119:6-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> >
> > I think this correct, but maybe Kees can give definitive answer here.
> >
> > > Signed-off-by: Atul Raut <[email protected]>
> > > ---
> > > drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > > index de8a2e27f49c..fff32e54833d 100644
> > > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > > @@ -116,7 +116,7 @@ struct brcmf_dump_survey {
> > > struct cca_stats_n_flags {
> > > u32 msrmnt_time; /* Time for Measurement (msec) */
> > > u32 msrmnt_done; /* flag set when measurement complete */
> > > - char buf[1];
> > > + char buf[];
> > > };
> > > struct cca_msrmnt_query {
>
> Normally a [1] -> [] conversion needs some details in the commit log
> about why this is safe (since it runs the risk of changing sizeof(struct
> cca_stats_n_flags). In this case, there's only a single user of the
> struct, and nothing about its use depends on its size:
>
> #define BRCMF_DCMD_MEDLEN 1536
> ...
> struct cca_stats_n_flags *results;
> char *buf;
> ...
> buf = kzalloc(sizeof(char) * BRCMF_DCMD_MEDLEN, GFP_KERNEL);
> ...
> results = (struct cca_stats_n_flags *)(buf);
> ...
> brcmf_parse_dump_obss(results->buf, survey);
>
> So, the allocation size is big enough for the struct, and nothing
> depends on the struct size.
>
> Reviewed-by: Kees Cook <[email protected]>
>
As the linux-kernel-mentees mailing list is in CC, I assume that Atul
is a linux-kernel mentee. In that case, it is certainly reasonable and
at least of educational value for Atul to take Kees' explanation and
add this to the patch's commit message in his own words. Then, with
that, submit a PATCH v3.
Lukas
Thank You Kees,
Hello Lukas, Arend & Kalle,
Sorry for not getting back to you sooner, am out of county &
hence unable to push the patch right away.
Will share once back, apologize for inconvenience caused.
-Atul
On 1/19/24 05:00, Lukas Bulwahn wrote:
> On Thu, Jan 18, 2024 at 9:20 PM Kees Cook <[email protected]> wrote:
>> On Thu, Jan 18, 2024 at 02:53:28PM +0100, Arend van Spriel wrote:
>>> + Kees
>>>
>>> On 8/3/2023 7:22 AM, Atul Raut wrote:
>>>> One-element arrays are obsolete, and flexible
>>>> array members have taken their place. So, in
>>>> struct cca_stats_n_flags, replace the one-element
>>>> array with a flexible-array member.
>>>>
>>>> This fixes warnings such as:
>>>> ./drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:119:6-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
>>> I think this correct, but maybe Kees can give definitive answer here.
>>>
>>>> Signed-off-by: Atul Raut <[email protected]>
>>>> ---
>>>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>>> index de8a2e27f49c..fff32e54833d 100644
>>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>>> @@ -116,7 +116,7 @@ struct brcmf_dump_survey {
>>>> struct cca_stats_n_flags {
>>>> u32 msrmnt_time; /* Time for Measurement (msec) */
>>>> u32 msrmnt_done; /* flag set when measurement complete */
>>>> - char buf[1];
>>>> + char buf[];
>>>> };
>>>> struct cca_msrmnt_query {
>> Normally a [1] -> [] conversion needs some details in the commit log
>> about why this is safe (since it runs the risk of changing sizeof(struct
>> cca_stats_n_flags). In this case, there's only a single user of the
>> struct, and nothing about its use depends on its size:
>>
>> #define BRCMF_DCMD_MEDLEN 1536
>> ...
>> struct cca_stats_n_flags *results;
>> char *buf;
>> ...
>> buf = kzalloc(sizeof(char) * BRCMF_DCMD_MEDLEN, GFP_KERNEL);
>> ...
>> results = (struct cca_stats_n_flags *)(buf);
>> ...
>> brcmf_parse_dump_obss(results->buf, survey);
>>
>> So, the allocation size is big enough for the struct, and nothing
>> depends on the struct size.
>>
>> Reviewed-by: Kees Cook <[email protected]>
>>
> As the linux-kernel-mentees mailing list is in CC, I assume that Atul
> is a linux-kernel mentee. In that case, it is certainly reasonable and
> at least of educational value for Atul to take Kees' explanation and
> add this to the patch's commit message in his own words. Then, with
> that, submit a PATCH v3.
>
>
> Lukas