All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] staging: rtl8192e, rtl8192u: use correct notation to define pointer
@ 2020-10-26 12:14 ` Muhammad Usama Anjum
  0 siblings, 0 replies; 10+ messages in thread
From: Muhammad Usama Anjum @ 2020-10-26 12:14 UTC (permalink / raw)
  To: gregkh; +Cc: linux-kernel, devel, musamaanjum

Use pointer notation instead of using array notation as info_element is
a pointer not array.

Warnings from sparse:
drivers/staging/rtl8192u/ieee80211/ieee80211.h:1013:51: warning: array of flexible structures
drivers/staging/rtl8192u/ieee80211/ieee80211.h:985:51: warning: array of flexible structures
drivers/staging/rtl8192u/ieee80211/ieee80211.h:963:51: warning: array of flexible structures
drivers/staging/rtl8192u/ieee80211/ieee80211.h:996:51: warning: array of flexible structures
drivers/staging/rtl8192u/ieee80211/ieee80211.h:974:51: warning: array of flexible structures

drivers/staging/rtl8192e/rtllib.h:832:48: warning: array of flexible structures
drivers/staging/rtl8192e/rtllib.h:851:48: warning: array of flexible structures
drivers/staging/rtl8192e/rtllib.h:805:48: warning: array of flexible structures
drivers/staging/rtl8192e/rtllib.h:843:48: warning: array of flexible structures
drivers/staging/rtl8192e/rtllib.h:821:48: warning: array of flexible structures

Signed-off-by: Muhammad Usama Anjum <musamaanjum@gmail.com>
---
 drivers/staging/rtl8192e/rtllib.h              | 10 +++++-----
 drivers/staging/rtl8192u/ieee80211/ieee80211.h | 12 ++++++------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h
index b84f00b8d18b..1dab9c3d08a8 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 {
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211.h b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
index 39f4ddd86796..37b240dd0b1f 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211.h
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
@@ -960,7 +960,7 @@ struct ieee80211_authentication {
 	__le16 transaction;
 	__le16 status;
 	/*challenge*/
-	struct ieee80211_info_element info_element[];
+	struct ieee80211_info_element *info_element;
 } __packed;
 
 struct ieee80211_disassoc {
@@ -971,7 +971,7 @@ struct ieee80211_disassoc {
 struct ieee80211_probe_request {
 	struct rtl_80211_hdr_3addr header;
 	/* SSID, supported rates */
-	struct ieee80211_info_element info_element[];
+	struct ieee80211_info_element *info_element;
 } __packed;
 
 struct ieee80211_probe_response {
@@ -982,7 +982,7 @@ struct ieee80211_probe_response {
 	/* SSID, supported rates, FH params, DS params,
 	 * CF params, IBSS params, TIM (if beacon), RSN
 	 */
-	struct ieee80211_info_element info_element[];
+	struct ieee80211_info_element *info_element;
 } __packed;
 
 /* Alias beacon for probe_response */
@@ -993,7 +993,7 @@ struct ieee80211_assoc_request_frame {
 	__le16 capability;
 	__le16 listen_interval;
 	/* SSID, supported rates, RSN */
-	struct ieee80211_info_element info_element[];
+	struct ieee80211_info_element *info_element;
 } __packed;
 
 struct ieee80211_reassoc_request_frame {
@@ -1002,7 +1002,7 @@ struct ieee80211_reassoc_request_frame {
 	__le16 listen_interval;
 	u8 current_ap[ETH_ALEN];
 	/* SSID, supported rates, RSN */
-	struct ieee80211_info_element info_element[];
+	struct ieee80211_info_element *info_element;
 } __packed;
 
 struct ieee80211_assoc_response_frame {
@@ -1010,7 +1010,7 @@ struct ieee80211_assoc_response_frame {
 	__le16 capability;
 	__le16 status;
 	__le16 aid;
-	struct ieee80211_info_element info_element[]; /* supported rates */
+	struct ieee80211_info_element *info_element; /* supported rates */
 } __packed;
 
 struct ieee80211_txb {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH] staging: rtl8192e, rtl8192u: use correct notation to define pointer
@ 2020-10-26 12:14 ` Muhammad Usama Anjum
  0 siblings, 0 replies; 10+ messages in thread
From: Muhammad Usama Anjum @ 2020-10-26 12:14 UTC (permalink / raw)
  To: gregkh; +Cc: devel, linux-kernel, musamaanjum

Use pointer notation instead of using array notation as info_element is
a pointer not array.

Warnings from sparse:
drivers/staging/rtl8192u/ieee80211/ieee80211.h:1013:51: warning: array of flexible structures
drivers/staging/rtl8192u/ieee80211/ieee80211.h:985:51: warning: array of flexible structures
drivers/staging/rtl8192u/ieee80211/ieee80211.h:963:51: warning: array of flexible structures
drivers/staging/rtl8192u/ieee80211/ieee80211.h:996:51: warning: array of flexible structures
drivers/staging/rtl8192u/ieee80211/ieee80211.h:974:51: warning: array of flexible structures

drivers/staging/rtl8192e/rtllib.h:832:48: warning: array of flexible structures
drivers/staging/rtl8192e/rtllib.h:851:48: warning: array of flexible structures
drivers/staging/rtl8192e/rtllib.h:805:48: warning: array of flexible structures
drivers/staging/rtl8192e/rtllib.h:843:48: warning: array of flexible structures
drivers/staging/rtl8192e/rtllib.h:821:48: warning: array of flexible structures

Signed-off-by: Muhammad Usama Anjum <musamaanjum@gmail.com>
---
 drivers/staging/rtl8192e/rtllib.h              | 10 +++++-----
 drivers/staging/rtl8192u/ieee80211/ieee80211.h | 12 ++++++------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h
index b84f00b8d18b..1dab9c3d08a8 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 {
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211.h b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
index 39f4ddd86796..37b240dd0b1f 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211.h
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
@@ -960,7 +960,7 @@ struct ieee80211_authentication {
 	__le16 transaction;
 	__le16 status;
 	/*challenge*/
-	struct ieee80211_info_element info_element[];
+	struct ieee80211_info_element *info_element;
 } __packed;
 
 struct ieee80211_disassoc {
@@ -971,7 +971,7 @@ struct ieee80211_disassoc {
 struct ieee80211_probe_request {
 	struct rtl_80211_hdr_3addr header;
 	/* SSID, supported rates */
-	struct ieee80211_info_element info_element[];
+	struct ieee80211_info_element *info_element;
 } __packed;
 
 struct ieee80211_probe_response {
@@ -982,7 +982,7 @@ struct ieee80211_probe_response {
 	/* SSID, supported rates, FH params, DS params,
 	 * CF params, IBSS params, TIM (if beacon), RSN
 	 */
-	struct ieee80211_info_element info_element[];
+	struct ieee80211_info_element *info_element;
 } __packed;
 
 /* Alias beacon for probe_response */
@@ -993,7 +993,7 @@ struct ieee80211_assoc_request_frame {
 	__le16 capability;
 	__le16 listen_interval;
 	/* SSID, supported rates, RSN */
-	struct ieee80211_info_element info_element[];
+	struct ieee80211_info_element *info_element;
 } __packed;
 
 struct ieee80211_reassoc_request_frame {
@@ -1002,7 +1002,7 @@ struct ieee80211_reassoc_request_frame {
 	__le16 listen_interval;
 	u8 current_ap[ETH_ALEN];
 	/* SSID, supported rates, RSN */
-	struct ieee80211_info_element info_element[];
+	struct ieee80211_info_element *info_element;
 } __packed;
 
 struct ieee80211_assoc_response_frame {
@@ -1010,7 +1010,7 @@ struct ieee80211_assoc_response_frame {
 	__le16 capability;
 	__le16 status;
 	__le16 aid;
-	struct ieee80211_info_element info_element[]; /* supported rates */
+	struct ieee80211_info_element *info_element; /* supported rates */
 } __packed;
 
 struct ieee80211_txb {
-- 
2.25.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [PATCH] staging: rtl8192e, rtl8192u: use correct notation to define pointer
  2020-10-26 12:14 ` Muhammad Usama Anjum
@ 2020-10-27 11:23   ` Greg KH
  -1 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2020-10-27 11:23 UTC (permalink / raw)
  To: Muhammad Usama Anjum; +Cc: devel, linux-kernel

On Mon, Oct 26, 2020 at 05:14:35PM +0500, Muhammad Usama Anjum wrote:
> Use pointer notation instead of using array notation as info_element is
> a pointer not array.
> 
> Warnings from sparse:
> drivers/staging/rtl8192u/ieee80211/ieee80211.h:1013:51: warning: array of flexible structures
> drivers/staging/rtl8192u/ieee80211/ieee80211.h:985:51: warning: array of flexible structures
> drivers/staging/rtl8192u/ieee80211/ieee80211.h:963:51: warning: array of flexible structures
> drivers/staging/rtl8192u/ieee80211/ieee80211.h:996:51: warning: array of flexible structures
> drivers/staging/rtl8192u/ieee80211/ieee80211.h:974:51: warning: array of flexible structures
> 
> drivers/staging/rtl8192e/rtllib.h:832:48: warning: array of flexible structures
> drivers/staging/rtl8192e/rtllib.h:851:48: warning: array of flexible structures
> drivers/staging/rtl8192e/rtllib.h:805:48: warning: array of flexible structures
> drivers/staging/rtl8192e/rtllib.h:843:48: warning: array of flexible structures
> drivers/staging/rtl8192e/rtllib.h:821:48: warning: array of flexible structures
> 
> Signed-off-by: Muhammad Usama Anjum <musamaanjum@gmail.com>
> ---
>  drivers/staging/rtl8192e/rtllib.h              | 10 +++++-----
>  drivers/staging/rtl8192u/ieee80211/ieee80211.h | 12 ++++++------
>  2 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h
> index b84f00b8d18b..1dab9c3d08a8 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;

Are you sure these changes are correct?  This isn't just a list of
structures after this at the end of the structure?

Please look at commit 5979afa2c4d1 ("staging: Replace zero-length array
with flexible-array member") which made most of these flexible arrays.

This is not a pointer, it really is an array, I think sparse is really
wrong here, be careful.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH] staging: rtl8192e, rtl8192u: use correct notation to define pointer
@ 2020-10-27 11:23   ` Greg KH
  0 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2020-10-27 11:23 UTC (permalink / raw)
  To: Muhammad Usama Anjum; +Cc: devel, linux-kernel

On Mon, Oct 26, 2020 at 05:14:35PM +0500, Muhammad Usama Anjum wrote:
> Use pointer notation instead of using array notation as info_element is
> a pointer not array.
> 
> Warnings from sparse:
> drivers/staging/rtl8192u/ieee80211/ieee80211.h:1013:51: warning: array of flexible structures
> drivers/staging/rtl8192u/ieee80211/ieee80211.h:985:51: warning: array of flexible structures
> drivers/staging/rtl8192u/ieee80211/ieee80211.h:963:51: warning: array of flexible structures
> drivers/staging/rtl8192u/ieee80211/ieee80211.h:996:51: warning: array of flexible structures
> drivers/staging/rtl8192u/ieee80211/ieee80211.h:974:51: warning: array of flexible structures
> 
> drivers/staging/rtl8192e/rtllib.h:832:48: warning: array of flexible structures
> drivers/staging/rtl8192e/rtllib.h:851:48: warning: array of flexible structures
> drivers/staging/rtl8192e/rtllib.h:805:48: warning: array of flexible structures
> drivers/staging/rtl8192e/rtllib.h:843:48: warning: array of flexible structures
> drivers/staging/rtl8192e/rtllib.h:821:48: warning: array of flexible structures
> 
> Signed-off-by: Muhammad Usama Anjum <musamaanjum@gmail.com>
> ---
>  drivers/staging/rtl8192e/rtllib.h              | 10 +++++-----
>  drivers/staging/rtl8192u/ieee80211/ieee80211.h | 12 ++++++------
>  2 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h
> index b84f00b8d18b..1dab9c3d08a8 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;

Are you sure these changes are correct?  This isn't just a list of
structures after this at the end of the structure?

Please look at commit 5979afa2c4d1 ("staging: Replace zero-length array
with flexible-array member") which made most of these flexible arrays.

This is not a pointer, it really is an array, I think sparse is really
wrong here, be careful.

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH] staging: rtl8192e, rtl8192u: use correct notation to define pointer
  2020-10-27 11:23   ` Greg KH
@ 2020-10-27 13:50     ` Dan Carpenter
  -1 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2020-10-27 13:50 UTC (permalink / raw)
  To: Greg KH; +Cc: Muhammad Usama Anjum, devel, linux-kernel

On Tue, Oct 27, 2020 at 12:23:03PM +0100, Greg KH wrote:
> On Mon, Oct 26, 2020 at 05:14:35PM +0500, Muhammad Usama Anjum wrote:
> > Use pointer notation instead of using array notation as info_element is
> > a pointer not array.
> > 
> > Warnings from sparse:
> > drivers/staging/rtl8192u/ieee80211/ieee80211.h:1013:51: warning: array of flexible structures
> > drivers/staging/rtl8192u/ieee80211/ieee80211.h:985:51: warning: array of flexible structures
> > drivers/staging/rtl8192u/ieee80211/ieee80211.h:963:51: warning: array of flexible structures
> > drivers/staging/rtl8192u/ieee80211/ieee80211.h:996:51: warning: array of flexible structures
> > drivers/staging/rtl8192u/ieee80211/ieee80211.h:974:51: warning: array of flexible structures
> > 
> > drivers/staging/rtl8192e/rtllib.h:832:48: warning: array of flexible structures
> > drivers/staging/rtl8192e/rtllib.h:851:48: warning: array of flexible structures
> > drivers/staging/rtl8192e/rtllib.h:805:48: warning: array of flexible structures
> > drivers/staging/rtl8192e/rtllib.h:843:48: warning: array of flexible structures
> > drivers/staging/rtl8192e/rtllib.h:821:48: warning: array of flexible structures
> > 
> > Signed-off-by: Muhammad Usama Anjum <musamaanjum@gmail.com>
> > ---
> >  drivers/staging/rtl8192e/rtllib.h              | 10 +++++-----
> >  drivers/staging/rtl8192u/ieee80211/ieee80211.h | 12 ++++++------
> >  2 files changed, 11 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h
> > index b84f00b8d18b..1dab9c3d08a8 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;
> 
> Are you sure these changes are correct?  This isn't just a list of
> structures after this at the end of the structure?

Definitely the patch will break things at runtime.  I was surprised that
it compiles, but it does.

> 
> Please look at commit 5979afa2c4d1 ("staging: Replace zero-length array
> with flexible-array member") which made most of these flexible arrays.
> 
> This is not a pointer, it really is an array, I think sparse is really
> wrong here, be careful.

It's an interesting warning message.  Sparse is correct that the code
looks strange.  If there were ever two or more elements in the array
then the code would break.  But since the code only uses a max of one
element then it's fine.

I guess the question is does this warning ever catch bugs in real life?
It seems like that the kind of bug which would be caught in testing so
static analysis is not going to be useful.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH] staging: rtl8192e, rtl8192u: use correct notation to define pointer
@ 2020-10-27 13:50     ` Dan Carpenter
  0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2020-10-27 13:50 UTC (permalink / raw)
  To: Greg KH; +Cc: devel, Muhammad Usama Anjum, linux-kernel

On Tue, Oct 27, 2020 at 12:23:03PM +0100, Greg KH wrote:
> On Mon, Oct 26, 2020 at 05:14:35PM +0500, Muhammad Usama Anjum wrote:
> > Use pointer notation instead of using array notation as info_element is
> > a pointer not array.
> > 
> > Warnings from sparse:
> > drivers/staging/rtl8192u/ieee80211/ieee80211.h:1013:51: warning: array of flexible structures
> > drivers/staging/rtl8192u/ieee80211/ieee80211.h:985:51: warning: array of flexible structures
> > drivers/staging/rtl8192u/ieee80211/ieee80211.h:963:51: warning: array of flexible structures
> > drivers/staging/rtl8192u/ieee80211/ieee80211.h:996:51: warning: array of flexible structures
> > drivers/staging/rtl8192u/ieee80211/ieee80211.h:974:51: warning: array of flexible structures
> > 
> > drivers/staging/rtl8192e/rtllib.h:832:48: warning: array of flexible structures
> > drivers/staging/rtl8192e/rtllib.h:851:48: warning: array of flexible structures
> > drivers/staging/rtl8192e/rtllib.h:805:48: warning: array of flexible structures
> > drivers/staging/rtl8192e/rtllib.h:843:48: warning: array of flexible structures
> > drivers/staging/rtl8192e/rtllib.h:821:48: warning: array of flexible structures
> > 
> > Signed-off-by: Muhammad Usama Anjum <musamaanjum@gmail.com>
> > ---
> >  drivers/staging/rtl8192e/rtllib.h              | 10 +++++-----
> >  drivers/staging/rtl8192u/ieee80211/ieee80211.h | 12 ++++++------
> >  2 files changed, 11 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h
> > index b84f00b8d18b..1dab9c3d08a8 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;
> 
> Are you sure these changes are correct?  This isn't just a list of
> structures after this at the end of the structure?

Definitely the patch will break things at runtime.  I was surprised that
it compiles, but it does.

> 
> Please look at commit 5979afa2c4d1 ("staging: Replace zero-length array
> with flexible-array member") which made most of these flexible arrays.
> 
> This is not a pointer, it really is an array, I think sparse is really
> wrong here, be careful.

It's an interesting warning message.  Sparse is correct that the code
looks strange.  If there were ever two or more elements in the array
then the code would break.  But since the code only uses a max of one
element then it's fine.

I guess the question is does this warning ever catch bugs in real life?
It seems like that the kind of bug which would be caught in testing so
static analysis is not going to be useful.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH] staging: rtl8192e, rtl8192u: use correct notation to define pointer
  2020-10-27 11:23   ` Greg KH
@ 2020-10-28  4:42     ` Luc Van Oostenryck
  -1 siblings, 0 replies; 10+ messages in thread
From: Luc Van Oostenryck @ 2020-10-28  4:42 UTC (permalink / raw)
  To: Greg KH; +Cc: Muhammad Usama Anjum, devel, linux-kernel

On Tue, Oct 27, 2020 at 12:23:03PM +0100, Greg KH wrote:
> 
> Are you sure these changes are correct?  This isn't just a list of
> structures after this at the end of the structure?
> 
> Please look at commit 5979afa2c4d1 ("staging: Replace zero-length array
> with flexible-array member") which made most of these flexible arrays.
> 
> This is not a pointer, it really is an array, I think sparse is really
> wrong here, be careful.

Sparse's warning is not about changing the definition of this member
as if it was the argument of a function. It's about how can you use
an array of structure when this structure has a flexible member.
It's a recent warning, added purposely to catch this.
See https://lore.kernel.org/r/CAHk-=wgJZ05ap8VQdzWDWJVWVtZiOYTc6cnNB8gNeQzEnfm-tw@mail.gmail.com

-- Luc

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH] staging: rtl8192e, rtl8192u: use correct notation to define pointer
@ 2020-10-28  4:42     ` Luc Van Oostenryck
  0 siblings, 0 replies; 10+ messages in thread
From: Luc Van Oostenryck @ 2020-10-28  4:42 UTC (permalink / raw)
  To: Greg KH; +Cc: devel, Muhammad Usama Anjum, linux-kernel

On Tue, Oct 27, 2020 at 12:23:03PM +0100, Greg KH wrote:
> 
> Are you sure these changes are correct?  This isn't just a list of
> structures after this at the end of the structure?
> 
> Please look at commit 5979afa2c4d1 ("staging: Replace zero-length array
> with flexible-array member") which made most of these flexible arrays.
> 
> This is not a pointer, it really is an array, I think sparse is really
> wrong here, be careful.

Sparse's warning is not about changing the definition of this member
as if it was the argument of a function. It's about how can you use
an array of structure when this structure has a flexible member.
It's a recent warning, added purposely to catch this.
See https://lore.kernel.org/r/CAHk-=wgJZ05ap8VQdzWDWJVWVtZiOYTc6cnNB8gNeQzEnfm-tw@mail.gmail.com

-- Luc
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH] staging: rtl8192e, rtl8192u: use correct notation to define pointer
  2020-10-28  4:42     ` Luc Van Oostenryck
@ 2021-02-04 14:51       ` Muhammad Usama Anjum
  -1 siblings, 0 replies; 10+ messages in thread
From: Muhammad Usama Anjum @ 2021-02-04 14:51 UTC (permalink / raw)
  To: Luc Van Oostenryck, Greg KH; +Cc: devel, linux-kernel

> Sparse's warning is not about changing the definition of this member
> as if it was the argument of a function. It's about how can you use
> an array of structure when this structure has a flexible member.

We have the following structures in drivers/staging/rtl8192e. (I've
simplified them for showing here.)

struct rtllib_hdr_3addr {
	int a;	
	int payload[];
};

struct rtllib_info_element {
	int len;
	int data[];
};

struct rtllib_probe_request {
	struct rtllib_hdr_3addr header;
	struct rtllib_info_element info_element[];
};

static void func(struct rtllib_probe_request *ptr) {
	ptr->header.a = 1;
}

Running sparse gives: 
sparse -Wflexible-array-array flexible_array.c
flexible_array.c:13:48: warning: array of flexible structures

There are several such structures in rtl8192e and rtl8192u. I've been
trying to fix one of them. But it seems like more knowledge is
required to refactor the driver.

Thanks,
Usama




^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH] staging: rtl8192e, rtl8192u: use correct notation to define pointer
@ 2021-02-04 14:51       ` Muhammad Usama Anjum
  0 siblings, 0 replies; 10+ messages in thread
From: Muhammad Usama Anjum @ 2021-02-04 14:51 UTC (permalink / raw)
  To: Luc Van Oostenryck, Greg KH; +Cc: devel, linux-kernel

> Sparse's warning is not about changing the definition of this member
> as if it was the argument of a function. It's about how can you use
> an array of structure when this structure has a flexible member.

We have the following structures in drivers/staging/rtl8192e. (I've
simplified them for showing here.)

struct rtllib_hdr_3addr {
	int a;	
	int payload[];
};

struct rtllib_info_element {
	int len;
	int data[];
};

struct rtllib_probe_request {
	struct rtllib_hdr_3addr header;
	struct rtllib_info_element info_element[];
};

static void func(struct rtllib_probe_request *ptr) {
	ptr->header.a = 1;
}

Running sparse gives: 
sparse -Wflexible-array-array flexible_array.c
flexible_array.c:13:48: warning: array of flexible structures

There are several such structures in rtl8192e and rtl8192u. I've been
trying to fix one of them. But it seems like more knowledge is
required to refactor the driver.

Thanks,
Usama



_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-02-04 19:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-26 12:14 [PATCH] staging: rtl8192e, rtl8192u: use correct notation to define pointer Muhammad Usama Anjum
2020-10-26 12:14 ` Muhammad Usama Anjum
2020-10-27 11:23 ` Greg KH
2020-10-27 11:23   ` Greg KH
2020-10-27 13:50   ` Dan Carpenter
2020-10-27 13:50     ` Dan Carpenter
2020-10-28  4:42   ` Luc Van Oostenryck
2020-10-28  4:42     ` Luc Van Oostenryck
2021-02-04 14:51     ` Muhammad Usama Anjum
2021-02-04 14:51       ` Muhammad Usama Anjum

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.