2021-04-27 17:51:09

by Jitendra

[permalink] [raw]
Subject: [PATCH] staging: rtl8192e: fix array of flexible structures

This patch fixes sparse warning "array of flexible structures"
for rtllib.h.

eg. drivers/staging/rtl8192e/rtllib.h:832:48: warning: array of
flexible structures

Signed-off-by: Jitendra Khasdev <[email protected]>
---
drivers/staging/rtl8192e/rtllib.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h
index 4cabaf2..c7cb318 100644
--- a/drivers/staging/rtl8192e/rtllib.h
+++ b/drivers/staging/rtl8192e/rtllib.h
@@ -802,7 +802,7 @@ struct rtllib_authentication {
__le16 transaction;
__le16 status;
/*challenge*/
- struct rtllib_info_element info_element[];
+ struct rtllib_info_element *info_element;
} __packed;

struct rtllib_disauth {
@@ -818,7 +818,7 @@ struct rtllib_disassoc {
struct rtllib_probe_request {
struct rtllib_hdr_3addr header;
/* SSID, supported rates */
- struct rtllib_info_element info_element[];
+ struct rtllib_info_element *info_element;
} __packed;

struct rtllib_probe_response {
@@ -829,7 +829,7 @@ struct rtllib_probe_response {
/* SSID, supported rates, FH params, DS params,
* CF params, IBSS params, TIM (if beacon), RSN
*/
- struct rtllib_info_element info_element[];
+ struct rtllib_info_element *info_element;
} __packed;

/* Alias beacon for probe_response */
@@ -840,7 +840,7 @@ struct rtllib_assoc_request_frame {
__le16 capability;
__le16 listen_interval;
/* SSID, supported rates, RSN */
- struct rtllib_info_element info_element[];
+ struct rtllib_info_element *info_element;
} __packed;

struct rtllib_assoc_response_frame {
@@ -848,7 +848,7 @@ struct rtllib_assoc_response_frame {
__le16 capability;
__le16 status;
__le16 aid;
- struct rtllib_info_element info_element[]; /* supported rates */
+ struct rtllib_info_element *info_element; /* supported rates */
} __packed;

struct rtllib_txb {
--
1.8.3.1


2021-04-27 18:14:16

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] staging: rtl8192e: fix array of flexible structures

On Tue, Apr 27, 2021 at 11:19:45PM +0530, Jitendra Khasdev wrote:
> This patch fixes sparse warning "array of flexible structures"
> for rtllib.h.
>
> eg. drivers/staging/rtl8192e/rtllib.h:832:48: warning: array of
> flexible structures
>
> Signed-off-by: Jitendra Khasdev <[email protected]>
> ---
> drivers/staging/rtl8192e/rtllib.h | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h
> index 4cabaf2..c7cb318 100644
> --- a/drivers/staging/rtl8192e/rtllib.h
> +++ b/drivers/staging/rtl8192e/rtllib.h
> @@ -802,7 +802,7 @@ struct rtllib_authentication {
> __le16 transaction;
> __le16 status;
> /*challenge*/
> - struct rtllib_info_element info_element[];
> + struct rtllib_info_element *info_element;

You just changed the definition of this structure, and the other
structures here. Are you sure this is working properly?

thanks,

greg k-h

2021-04-27 18:59:43

by Jitendra

[permalink] [raw]
Subject: Re: [PATCH] staging: rtl8192e: fix array of flexible structures

On Tue, Apr 27, 2021 at 08:10:20PM +0200, Greg KH wrote:
>On Tue, Apr 27, 2021 at 11:19:45PM +0530, Jitendra Khasdev wrote:
>> This patch fixes sparse warning "array of flexible structures"
>> for rtllib.h.
>>
>> eg. drivers/staging/rtl8192e/rtllib.h:832:48: warning: array of
>> flexible structures
>>
>> Signed-off-by: Jitendra Khasdev <[email protected]>
>> ---
>> drivers/staging/rtl8192e/rtllib.h | 10 +++++-----
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h
>> index 4cabaf2..c7cb318 100644
>> --- a/drivers/staging/rtl8192e/rtllib.h
>> +++ b/drivers/staging/rtl8192e/rtllib.h
>> @@ -802,7 +802,7 @@ struct rtllib_authentication {
>> __le16 transaction;
>> __le16 status;
>> /*challenge*/
>> - struct rtllib_info_element info_element[];
>> + struct rtllib_info_element *info_element;
>
>You just changed the definition of this structure, and the other
>structures here. Are you sure this is working properly?
>

I have compiled the driver and install it on my vm, but I don't this specific
hardware, so couldn't test it.

I fixed in context of sparse.

---
Jitendra

2021-04-28 06:03:50

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] staging: rtl8192e: fix array of flexible structures

On Wed, Apr 28, 2021 at 12:28:44AM +0530, Jitendra wrote:
> On Tue, Apr 27, 2021 at 08:10:20PM +0200, Greg KH wrote:
> > On Tue, Apr 27, 2021 at 11:19:45PM +0530, Jitendra Khasdev wrote:
> > > This patch fixes sparse warning "array of flexible structures"
> > > for rtllib.h.
> > >
> > > eg. drivers/staging/rtl8192e/rtllib.h:832:48: warning: array of
> > > flexible structures
> > >
> > > Signed-off-by: Jitendra Khasdev <[email protected]>
> > > ---
> > > drivers/staging/rtl8192e/rtllib.h | 10 +++++-----
> > > 1 file changed, 5 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h
> > > index 4cabaf2..c7cb318 100644
> > > --- a/drivers/staging/rtl8192e/rtllib.h
> > > +++ b/drivers/staging/rtl8192e/rtllib.h
> > > @@ -802,7 +802,7 @@ struct rtllib_authentication {
> > > __le16 transaction;
> > > __le16 status;
> > > /*challenge*/
> > > - struct rtllib_info_element info_element[];
> > > + struct rtllib_info_element *info_element;
> >
> > You just changed the definition of this structure, and the other
> > structures here. Are you sure this is working properly?
> >
>
> I have compiled the driver and install it on my vm, but I don't this specific
> hardware, so couldn't test it.
>
> I fixed in context of sparse.

Please verify that this change is correct by looking at how the
structures are being created (i.e. is this being treated as a flexible
array or a pointer?)

I think we have been through this before and that sparse is not right,
but I can't remember...

thanks,

greg k-h

2021-04-28 07:26:03

by Jitendra

[permalink] [raw]
Subject: Re: [PATCH] staging: rtl8192e: fix array of flexible structures

On Wed, Apr 28, 2021 at 08:01:21AM +0200, Greg KH wrote:
>On Wed, Apr 28, 2021 at 12:28:44AM +0530, Jitendra wrote:
>> On Tue, Apr 27, 2021 at 08:10:20PM +0200, Greg KH wrote:
>> > On Tue, Apr 27, 2021 at 11:19:45PM +0530, Jitendra Khasdev wrote:
>> > > This patch fixes sparse warning "array of flexible structures"
>> > > for rtllib.h.
>> > >
>> > > eg. drivers/staging/rtl8192e/rtllib.h:832:48: warning: array of
>> > > flexible structures
>> > >
>> > > Signed-off-by: Jitendra Khasdev <[email protected]>
>> > > ---
>> > > drivers/staging/rtl8192e/rtllib.h | 10 +++++-----
>> > > 1 file changed, 5 insertions(+), 5 deletions(-)
>> > >
>> > > diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h
>> > > index 4cabaf2..c7cb318 100644
>> > > --- a/drivers/staging/rtl8192e/rtllib.h
>> > > +++ b/drivers/staging/rtl8192e/rtllib.h
>> > > @@ -802,7 +802,7 @@ struct rtllib_authentication {
>> > > __le16 transaction;
>> > > __le16 status;
>> > > /*challenge*/
>> > > - struct rtllib_info_element info_element[];
>> > > + struct rtllib_info_element *info_element;
>> >
>> > You just changed the definition of this structure, and the other
>> > structures here. Are you sure this is working properly?
>> >
>>
>> I have compiled the driver and install it on my vm, but I don't this specific
>> hardware, so couldn't test it.
>>
>> I fixed in context of sparse.
>
>Please verify that this change is correct by looking at how the
>structures are being created (i.e. is this being treated as a flexible
>array or a pointer?)
>
>I think we have been through this before and that sparse is not right,
>but I can't remember...
>
Yes, it is getting used as flexible array in code. hence, simply we can drop
this patch.

Also, looks to me, there is no more sparse warnings to fix in staging.

---
Jitendra

2021-04-29 14:23:03

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] staging: rtl8192e: fix array of flexible structures

On Tue, Apr 27, 2021 at 11:19:45PM +0530, Jitendra Khasdev wrote:
> This patch fixes sparse warning "array of flexible structures"
> for rtllib.h.
>
> eg. drivers/staging/rtl8192e/rtllib.h:832:48: warning: array of
> flexible structures
>
> Signed-off-by: Jitendra Khasdev <[email protected]>
> ---
> drivers/staging/rtl8192e/rtllib.h | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h
> index 4cabaf2..c7cb318 100644
> --- a/drivers/staging/rtl8192e/rtllib.h
> +++ b/drivers/staging/rtl8192e/rtllib.h
> @@ -802,7 +802,7 @@ struct rtllib_authentication {
> __le16 transaction;
> __le16 status;
> /*challenge*/
> - struct rtllib_info_element info_element[];
> + struct rtllib_info_element *info_element;
> } __packed;

This patch is wrong.

The original code is basically fine. Normally it doesn't make sense to
have an array of flex arrays, but in this case it "flexes" between 0 and
1. If it were had two elements then the match the math wouldn't work
at all.

We should probably get rid of it and just add some giant comments and
defines to do the math.

But changing it to a pointer isn't right.

regards,
dan carpenter