linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper
@ 2022-11-17 10:18 Deepak R Varma
  2022-11-17 12:54 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 12+ messages in thread
From: Deepak R Varma @ 2022-11-17 10:18 UTC (permalink / raw)
  To: Greg Kroah-Hartman, linux-staging, linux-kernel; +Cc: gustavoars

The code currently uses C90 standard extension based zero length arrays.
The zero length array member also happens to be the only member of the
structs. Such zero length array declarations are deprecated and the
new C99 standard extension of flexible array declarations are to be
used instead.

The DECLARE_FLEX_ARRAY() helper allows for a flexible array member as
the only member in a structure. Refer to these links [1], [2] for
details.

[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://lkml.kernel.org/r/YxKY6O2hmdwNh8r8@work

Issue identified using Coccinelle.

Signed-off-by: Deepak R Varma <drv@mailo.com>
---

Notes:
   1. Proposed change is compile tested only.
   2. Solution feedback from gustavoars@kernel.org


 drivers/staging/wlan-ng/hfa384x.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/wlan-ng/hfa384x.h b/drivers/staging/wlan-ng/hfa384x.h
index 0611e37df6ac..3a1edcb43e07 100644
--- a/drivers/staging/wlan-ng/hfa384x.h
+++ b/drivers/staging/wlan-ng/hfa384x.h
@@ -960,15 +960,15 @@ struct hfa384x_pdr_nicid {
 } __packed;

 struct hfa384x_pdr_refdac_measurements {
-	u16 value[0];
+	DECLARE_FLEX_ARRAY(u16, value);
 } __packed;

 struct hfa384x_pdr_vgdac_measurements {
-	u16 value[0];
+	DECLARE_FLEX_ARRAY(u16, value);
 } __packed;

 struct hfa384x_pdr_level_comp_measurements {
-	u16 value[0];
+	DECLARE_FLEX_ARRAY(u16, value);
 } __packed;

 struct hfa384x_pdr_mac_address {
--
2.34.1




^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper
  2022-11-17 10:18 [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper Deepak R Varma
@ 2022-11-17 12:54 ` Greg Kroah-Hartman
  2022-11-17 13:20   ` Deepak R Varma
  0 siblings, 1 reply; 12+ messages in thread
From: Greg Kroah-Hartman @ 2022-11-17 12:54 UTC (permalink / raw)
  To: Deepak R Varma; +Cc: linux-staging, linux-kernel, gustavoars

On Thu, Nov 17, 2022 at 03:48:45PM +0530, Deepak R Varma wrote:
> The code currently uses C90 standard extension based zero length arrays.
> The zero length array member also happens to be the only member of the
> structs. Such zero length array declarations are deprecated and the
> new C99 standard extension of flexible array declarations are to be
> used instead.
> 
> The DECLARE_FLEX_ARRAY() helper allows for a flexible array member as
> the only member in a structure. Refer to these links [1], [2] for
> details.
> 
> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> [2] https://lkml.kernel.org/r/YxKY6O2hmdwNh8r8@work
> 
> Issue identified using Coccinelle.
> 
> Signed-off-by: Deepak R Varma <drv@mailo.com>
> ---
> 
> Notes:
>    1. Proposed change is compile tested only.
>    2. Solution feedback from gustavoars@kernel.org
> 
> 
>  drivers/staging/wlan-ng/hfa384x.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/wlan-ng/hfa384x.h b/drivers/staging/wlan-ng/hfa384x.h
> index 0611e37df6ac..3a1edcb43e07 100644
> --- a/drivers/staging/wlan-ng/hfa384x.h
> +++ b/drivers/staging/wlan-ng/hfa384x.h
> @@ -960,15 +960,15 @@ struct hfa384x_pdr_nicid {
>  } __packed;
> 
>  struct hfa384x_pdr_refdac_measurements {
> -	u16 value[0];
> +	DECLARE_FLEX_ARRAY(u16, value);
>  } __packed;

Why?  This structure is never used anywhere, right?  So why is this
needed to be changed and not just removed entirely?  Same for the other
structures in this patch.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper
  2022-11-17 12:54 ` Greg Kroah-Hartman
@ 2022-11-17 13:20   ` Deepak R Varma
  2022-11-17 18:03     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 12+ messages in thread
From: Deepak R Varma @ 2022-11-17 13:20 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-staging, linux-kernel, gustavoars

On Thu, Nov 17, 2022 at 01:54:49PM +0100, Greg Kroah-Hartman wrote:
> On Thu, Nov 17, 2022 at 03:48:45PM +0530, Deepak R Varma wrote:
> > The code currently uses C90 standard extension based zero length arrays.
> > The zero length array member also happens to be the only member of the
> > structs. Such zero length array declarations are deprecated and the
> > new C99 standard extension of flexible array declarations are to be
> > used instead.
> >
> > The DECLARE_FLEX_ARRAY() helper allows for a flexible array member as
> > the only member in a structure. Refer to these links [1], [2] for
> > details.
> >
> > [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> > [2] https://lkml.kernel.org/r/YxKY6O2hmdwNh8r8@work
> >
> > Issue identified using Coccinelle.
> >
> > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > ---
> >
> > Notes:
> >    1. Proposed change is compile tested only.
> >    2. Solution feedback from gustavoars@kernel.org
> >
> >
> >  drivers/staging/wlan-ng/hfa384x.h | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/staging/wlan-ng/hfa384x.h b/drivers/staging/wlan-ng/hfa384x.h
> > index 0611e37df6ac..3a1edcb43e07 100644
> > --- a/drivers/staging/wlan-ng/hfa384x.h
> > +++ b/drivers/staging/wlan-ng/hfa384x.h
> > @@ -960,15 +960,15 @@ struct hfa384x_pdr_nicid {
> >  } __packed;
> >
> >  struct hfa384x_pdr_refdac_measurements {
> > -	u16 value[0];
> > +	DECLARE_FLEX_ARRAY(u16, value);
> >  } __packed;
>
> Why?  This structure is never used anywhere, right?  So why is this
> needed to be changed and not just removed entirely?  Same for the other
> structures in this patch.

Hello Greg,
I am unable to confirm that these structures are truly not needed in the absence
if a real device based testing. I could only validate that using the compile
build and driver loading.

This change that I am proposing in the interim would enable the compiler to
protect the structure from addition of a new member below the zero length array.

If there is a way to confirm that the structures are indeed not needed, I can
revise the patch and send the cleanup accordingly. Please suggest.

Thank you,
./drv


>
> thanks,
>
> greg k-h



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper
  2022-11-17 13:20   ` Deepak R Varma
@ 2022-11-17 18:03     ` Greg Kroah-Hartman
  2022-11-19 14:38       ` Deepak R Varma
  0 siblings, 1 reply; 12+ messages in thread
From: Greg Kroah-Hartman @ 2022-11-17 18:03 UTC (permalink / raw)
  To: Deepak R Varma; +Cc: linux-staging, linux-kernel, gustavoars

On Thu, Nov 17, 2022 at 06:50:55PM +0530, Deepak R Varma wrote:
> On Thu, Nov 17, 2022 at 01:54:49PM +0100, Greg Kroah-Hartman wrote:
> > On Thu, Nov 17, 2022 at 03:48:45PM +0530, Deepak R Varma wrote:
> > > The code currently uses C90 standard extension based zero length arrays.
> > > The zero length array member also happens to be the only member of the
> > > structs. Such zero length array declarations are deprecated and the
> > > new C99 standard extension of flexible array declarations are to be
> > > used instead.
> > >
> > > The DECLARE_FLEX_ARRAY() helper allows for a flexible array member as
> > > the only member in a structure. Refer to these links [1], [2] for
> > > details.
> > >
> > > [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> > > [2] https://lkml.kernel.org/r/YxKY6O2hmdwNh8r8@work
> > >
> > > Issue identified using Coccinelle.
> > >
> > > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > > ---
> > >
> > > Notes:
> > >    1. Proposed change is compile tested only.
> > >    2. Solution feedback from gustavoars@kernel.org
> > >
> > >
> > >  drivers/staging/wlan-ng/hfa384x.h | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/staging/wlan-ng/hfa384x.h b/drivers/staging/wlan-ng/hfa384x.h
> > > index 0611e37df6ac..3a1edcb43e07 100644
> > > --- a/drivers/staging/wlan-ng/hfa384x.h
> > > +++ b/drivers/staging/wlan-ng/hfa384x.h
> > > @@ -960,15 +960,15 @@ struct hfa384x_pdr_nicid {
> > >  } __packed;
> > >
> > >  struct hfa384x_pdr_refdac_measurements {
> > > -	u16 value[0];
> > > +	DECLARE_FLEX_ARRAY(u16, value);
> > >  } __packed;
> >
> > Why?  This structure is never used anywhere, right?  So why is this
> > needed to be changed and not just removed entirely?  Same for the other
> > structures in this patch.
> 
> Hello Greg,
> I am unable to confirm that these structures are truly not needed in the absence
> if a real device based testing. I could only validate that using the compile
> build and driver loading.

Think this through, if no one is actually using this structure, and it
is of 0 size, then do you think it is being used?

> This change that I am proposing in the interim would enable the compiler to
> protect the structure from addition of a new member below the zero length array.

Why would you want to add a new member below this?  That's not what is
happening here at all.

Please think this through a bit more.

good luck!

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper
  2022-11-17 18:03     ` Greg Kroah-Hartman
@ 2022-11-19 14:38       ` Deepak R Varma
  2022-11-28  7:45         ` Deepak R Varma
  0 siblings, 1 reply; 12+ messages in thread
From: Deepak R Varma @ 2022-11-19 14:38 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-staging, linux-kernel, gustavoars

On Thu, Nov 17, 2022 at 07:03:21PM +0100, Greg Kroah-Hartman wrote:
> On Thu, Nov 17, 2022 at 06:50:55PM +0530, Deepak R Varma wrote:
> > On Thu, Nov 17, 2022 at 01:54:49PM +0100, Greg Kroah-Hartman wrote:
> > > On Thu, Nov 17, 2022 at 03:48:45PM +0530, Deepak R Varma wrote:
> > > >
> > > >  struct hfa384x_pdr_refdac_measurements {
> > > > -	u16 value[0];
> > > > +	DECLARE_FLEX_ARRAY(u16, value);
> > > >  } __packed;
> > >
> > > Why?  This structure is never used anywhere, right?  So why is this
> > > needed to be changed and not just removed entirely?  Same for the other
> > > structures in this patch.
> >
> > Hello Greg,
> > I am unable to confirm that these structures are truly not needed in the absence
> > if a real device based testing. I could only validate that using the compile
> > build and driver loading.
>
> Think this through, if no one is actually using this structure, and it
> is of 0 size, then do you think it is being used?

Hello Greg,
I did not find any memory allocation for these zero length array structures.
Also, the union or its enclosing structure do not appear to access the members.
Hence I am leaning towards concluding that these zero length array structures do
not appear to be necessary.

There are a few other structures that are part of the same union, however, they
too do not appear to be used for accessing the memory assigned to the union [or
its enclosing structure]. I think most of the members of these unions can be
replaced by one max size structure of this union [e.g. struct
hfa384x_pdr_mkk_measurements].

Could you please comment if I am reading the code right?

For your quick reference, the zero length structure declaration are online 963
whereas the union is on line number 1080 of the file drivers/staging/wlan-ng/hfa384x.h


Thank you,
./drv


>
> > This change that I am proposing in the interim would enable the compiler to
> > protect the structure from addition of a new member below the zero length array.
>
> Why would you want to add a new member below this?  That's not what is
> happening here at all.

I came across this one old commit where such an accident happened. This is from
a recent LWN article:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e48f129c2f20

I understand the C99 now protects from such an attempt at the compile time
itself.

Thank you,
./drv


>
> Please think this through a bit more.
>
> good luck!
>
> greg k-h
>



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper
  2022-11-19 14:38       ` Deepak R Varma
@ 2022-11-28  7:45         ` Deepak R Varma
  2022-11-28  7:50           ` Dan Carpenter
  2022-11-28  7:53           ` Greg Kroah-Hartman
  0 siblings, 2 replies; 12+ messages in thread
From: Deepak R Varma @ 2022-11-28  7:45 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-staging, linux-kernel, gustavoars

On Sat, Nov 19, 2022 at 08:08:15PM +0530, Deepak R Varma wrote:
> On Thu, Nov 17, 2022 at 07:03:21PM +0100, Greg Kroah-Hartman wrote:
> > On Thu, Nov 17, 2022 at 06:50:55PM +0530, Deepak R Varma wrote:
> > > On Thu, Nov 17, 2022 at 01:54:49PM +0100, Greg Kroah-Hartman wrote:
> > > > On Thu, Nov 17, 2022 at 03:48:45PM +0530, Deepak R Varma wrote:
> > > > >
> > > > >  struct hfa384x_pdr_refdac_measurements {
> > > > > -	u16 value[0];
> > > > > +	DECLARE_FLEX_ARRAY(u16, value);
> > > > >  } __packed;
> > > >
> > > > Why?  This structure is never used anywhere, right?  So why is this
> > > > needed to be changed and not just removed entirely?  Same for the other
> > > > structures in this patch.
> > >
> > > Hello Greg,
> > > I am unable to confirm that these structures are truly not needed in the absence
> > > if a real device based testing. I could only validate that using the compile
> > > build and driver loading.
> >
> > Think this through, if no one is actually using this structure, and it
> > is of 0 size, then do you think it is being used?
>
> Hello Greg,
> I did not find any memory allocation for these zero length array structures.
> Also, the union or its enclosing structure do not appear to access the members.
> Hence I am leaning towards concluding that these zero length array structures do
> not appear to be necessary.
>
> There are a few other structures that are part of the same union, however, they
> too do not appear to be used for accessing the memory assigned to the union [or
> its enclosing structure]. I think most of the members of these unions can be
> replaced by one max size structure of this union [e.g. struct
> hfa384x_pdr_mkk_measurements].
>
> Could you please comment if I am reading the code right?
>
> For your quick reference, the zero length structure declaration are online 963
> whereas the union is on line number 1080 of the file drivers/staging/wlan-ng/hfa384x.h

Hello Greg,
can you please suggest how should I approach this clean-up/correction?

Thank you,
./drv

>
>
> Thank you,
> ./drv
>
>
> >
> > > This change that I am proposing in the interim would enable the compiler to
> > > protect the structure from addition of a new member below the zero length array.
> >
> > Why would you want to add a new member below this?  That's not what is
> > happening here at all.
>
> I came across this one old commit where such an accident happened. This is from
> a recent LWN article:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e48f129c2f20
>
> I understand the C99 now protects from such an attempt at the compile time
> itself.
>
> Thank you,
> ./drv
>
>
> >
> > Please think this through a bit more.
> >
> > good luck!
> >
> > greg k-h
> >



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper
  2022-11-28  7:45         ` Deepak R Varma
@ 2022-11-28  7:50           ` Dan Carpenter
  2022-11-28  8:21             ` Deepak R Varma
  2022-11-28  7:53           ` Greg Kroah-Hartman
  1 sibling, 1 reply; 12+ messages in thread
From: Dan Carpenter @ 2022-11-28  7:50 UTC (permalink / raw)
  To: Deepak R Varma
  Cc: Greg Kroah-Hartman, linux-staging, linux-kernel, gustavoars

On Mon, Nov 28, 2022 at 01:15:43PM +0530, Deepak R Varma wrote:
> On Sat, Nov 19, 2022 at 08:08:15PM +0530, Deepak R Varma wrote:
> > On Thu, Nov 17, 2022 at 07:03:21PM +0100, Greg Kroah-Hartman wrote:
> > > On Thu, Nov 17, 2022 at 06:50:55PM +0530, Deepak R Varma wrote:
> > > > On Thu, Nov 17, 2022 at 01:54:49PM +0100, Greg Kroah-Hartman wrote:
> > > > > On Thu, Nov 17, 2022 at 03:48:45PM +0530, Deepak R Varma wrote:
> > > > > >
> > > > > >  struct hfa384x_pdr_refdac_measurements {
> > > > > > -	u16 value[0];
> > > > > > +	DECLARE_FLEX_ARRAY(u16, value);
> > > > > >  } __packed;
> > > > >
> > > > > Why?  This structure is never used anywhere, right?  So why is this
> > > > > needed to be changed and not just removed entirely?  Same for the other
> > > > > structures in this patch.
> > > >
> > > > Hello Greg,
> > > > I am unable to confirm that these structures are truly not needed in the absence
> > > > if a real device based testing. I could only validate that using the compile
> > > > build and driver loading.
> > >
> > > Think this through, if no one is actually using this structure, and it
> > > is of 0 size, then do you think it is being used?
> >
> > Hello Greg,
> > I did not find any memory allocation for these zero length array structures.
> > Also, the union or its enclosing structure do not appear to access the members.
> > Hence I am leaning towards concluding that these zero length array structures do
> > not appear to be necessary.
> >
> > There are a few other structures that are part of the same union, however, they
> > too do not appear to be used for accessing the memory assigned to the union [or
> > its enclosing structure]. I think most of the members of these unions can be
> > replaced by one max size structure of this union [e.g. struct
> > hfa384x_pdr_mkk_measurements].
> >
> > Could you please comment if I am reading the code right?
> >
> > For your quick reference, the zero length structure declaration are online 963
> > whereas the union is on line number 1080 of the file drivers/staging/wlan-ng/hfa384x.h
> 
> Hello Greg,
> can you please suggest how should I approach this clean-up/correction?
> 

Like this:

diff --git a/drivers/staging/wlan-ng/hfa384x.h b/drivers/staging/wlan-ng/hfa384x.h
index 0611e37df6ac..6a3df58c9e9c 100644
--- a/drivers/staging/wlan-ng/hfa384x.h
+++ b/drivers/staging/wlan-ng/hfa384x.h
@@ -959,10 +959,6 @@ struct hfa384x_pdr_nicid {
 	u16 minor;
 } __packed;
 
-struct hfa384x_pdr_refdac_measurements {
-	u16 value[0];
-} __packed;
-
 struct hfa384x_pdr_vgdac_measurements {
 	u16 value[0];
 } __packed;
@@ -1077,7 +1073,6 @@ struct hfa384x_pdrec {
 		struct hfa384x_pdr_mfisuprange mfisuprange;
 		struct hfa384x_pdr_cfisuprange cfisuprange;
 		struct hfa384x_pdr_nicid nicid;
-		struct hfa384x_pdr_refdac_measurements refdac_measurements;
 		struct hfa384x_pdr_vgdac_measurements vgdac_measurements;
 		struct hfa384x_pdr_level_comp_measurements level_compc_measurements;
 		struct hfa384x_pdr_mac_address mac_address;

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper
  2022-11-28  7:45         ` Deepak R Varma
  2022-11-28  7:50           ` Dan Carpenter
@ 2022-11-28  7:53           ` Greg Kroah-Hartman
  2022-11-28  8:23             ` Deepak R Varma
  1 sibling, 1 reply; 12+ messages in thread
From: Greg Kroah-Hartman @ 2022-11-28  7:53 UTC (permalink / raw)
  To: Deepak R Varma; +Cc: linux-staging, linux-kernel, gustavoars

On Mon, Nov 28, 2022 at 01:15:43PM +0530, Deepak R Varma wrote:
> On Sat, Nov 19, 2022 at 08:08:15PM +0530, Deepak R Varma wrote:
> > On Thu, Nov 17, 2022 at 07:03:21PM +0100, Greg Kroah-Hartman wrote:
> > > On Thu, Nov 17, 2022 at 06:50:55PM +0530, Deepak R Varma wrote:
> > > > On Thu, Nov 17, 2022 at 01:54:49PM +0100, Greg Kroah-Hartman wrote:
> > > > > On Thu, Nov 17, 2022 at 03:48:45PM +0530, Deepak R Varma wrote:
> > > > > >
> > > > > >  struct hfa384x_pdr_refdac_measurements {
> > > > > > -	u16 value[0];
> > > > > > +	DECLARE_FLEX_ARRAY(u16, value);
> > > > > >  } __packed;
> > > > >
> > > > > Why?  This structure is never used anywhere, right?  So why is this
> > > > > needed to be changed and not just removed entirely?  Same for the other
> > > > > structures in this patch.
> > > >
> > > > Hello Greg,
> > > > I am unable to confirm that these structures are truly not needed in the absence
> > > > if a real device based testing. I could only validate that using the compile
> > > > build and driver loading.
> > >
> > > Think this through, if no one is actually using this structure, and it
> > > is of 0 size, then do you think it is being used?
> >
> > Hello Greg,
> > I did not find any memory allocation for these zero length array structures.
> > Also, the union or its enclosing structure do not appear to access the members.
> > Hence I am leaning towards concluding that these zero length array structures do
> > not appear to be necessary.
> >
> > There are a few other structures that are part of the same union, however, they
> > too do not appear to be used for accessing the memory assigned to the union [or
> > its enclosing structure]. I think most of the members of these unions can be
> > replaced by one max size structure of this union [e.g. struct
> > hfa384x_pdr_mkk_measurements].
> >
> > Could you please comment if I am reading the code right?
> >
> > For your quick reference, the zero length structure declaration are online 963
> > whereas the union is on line number 1080 of the file drivers/staging/wlan-ng/hfa384x.h
> 
> Hello Greg,
> can you please suggest how should I approach this clean-up/correction?

Sorry, but I do not have the bandwidth to help out with this.  I will
gladly review changes submitted only.

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper
  2022-11-28  7:50           ` Dan Carpenter
@ 2022-11-28  8:21             ` Deepak R Varma
  2022-11-28  8:25               ` Dan Carpenter
  0 siblings, 1 reply; 12+ messages in thread
From: Deepak R Varma @ 2022-11-28  8:21 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: Greg Kroah-Hartman, linux-staging, linux-kernel, gustavoars

On Mon, Nov 28, 2022 at 10:50:19AM +0300, Dan Carpenter wrote:
> On Mon, Nov 28, 2022 at 01:15:43PM +0530, Deepak R Varma wrote:
> > On Sat, Nov 19, 2022 at 08:08:15PM +0530, Deepak R Varma wrote:
> > > On Thu, Nov 17, 2022 at 07:03:21PM +0100, Greg Kroah-Hartman wrote:
> > > > On Thu, Nov 17, 2022 at 06:50:55PM +0530, Deepak R Varma wrote:
> > > > > On Thu, Nov 17, 2022 at 01:54:49PM +0100, Greg Kroah-Hartman wrote:
> > > > > > On Thu, Nov 17, 2022 at 03:48:45PM +0530, Deepak R Varma wrote:
> > > > > > >
> > > > > > >  struct hfa384x_pdr_refdac_measurements {
> > > > > > > -	u16 value[0];
> > > > > > > +	DECLARE_FLEX_ARRAY(u16, value);
> > > > > > >  } __packed;
> > > > > >
> > > > > > Why?  This structure is never used anywhere, right?  So why is this
> > > > > > needed to be changed and not just removed entirely?  Same for the other
> > > > > > structures in this patch.
> > > > >
> > > > > Hello Greg,
> > > > > I am unable to confirm that these structures are truly not needed in the absence
> > > > > if a real device based testing. I could only validate that using the compile
> > > > > build and driver loading.
> > > >
> > > > Think this through, if no one is actually using this structure, and it
> > > > is of 0 size, then do you think it is being used?
> > >
> > > Hello Greg,
> > > I did not find any memory allocation for these zero length array structures.
> > > Also, the union or its enclosing structure do not appear to access the members.
> > > Hence I am leaning towards concluding that these zero length array structures do
> > > not appear to be necessary.
> > >
> > > There are a few other structures that are part of the same union, however, they
> > > too do not appear to be used for accessing the memory assigned to the union [or
> > > its enclosing structure]. I think most of the members of these unions can be
> > > replaced by one max size structure of this union [e.g. struct
> > > hfa384x_pdr_mkk_measurements].
> > >
> > > Could you please comment if I am reading the code right?
> > >
> > > For your quick reference, the zero length structure declaration are online 963
> > > whereas the union is on line number 1080 of the file drivers/staging/wlan-ng/hfa384x.h
> >
> > Hello Greg,
> > can you please suggest how should I approach this clean-up/correction?
> >
>
> Like this:

Thank you Dan. This takes me back to the very first version of this patch. I
will send in the clean up.

./drv

>
> diff --git a/drivers/staging/wlan-ng/hfa384x.h b/drivers/staging/wlan-ng/hfa384x.h
> index 0611e37df6ac..6a3df58c9e9c 100644
> --- a/drivers/staging/wlan-ng/hfa384x.h
> +++ b/drivers/staging/wlan-ng/hfa384x.h
> @@ -959,10 +959,6 @@ struct hfa384x_pdr_nicid {
>  	u16 minor;
>  } __packed;
>
> -struct hfa384x_pdr_refdac_measurements {
> -	u16 value[0];
> -} __packed;
> -
>  struct hfa384x_pdr_vgdac_measurements {
>  	u16 value[0];
>  } __packed;
> @@ -1077,7 +1073,6 @@ struct hfa384x_pdrec {
>  		struct hfa384x_pdr_mfisuprange mfisuprange;
>  		struct hfa384x_pdr_cfisuprange cfisuprange;
>  		struct hfa384x_pdr_nicid nicid;
> -		struct hfa384x_pdr_refdac_measurements refdac_measurements;
>  		struct hfa384x_pdr_vgdac_measurements vgdac_measurements;
>  		struct hfa384x_pdr_level_comp_measurements level_compc_measurements;
>  		struct hfa384x_pdr_mac_address mac_address;
>



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper
  2022-11-28  7:53           ` Greg Kroah-Hartman
@ 2022-11-28  8:23             ` Deepak R Varma
  0 siblings, 0 replies; 12+ messages in thread
From: Deepak R Varma @ 2022-11-28  8:23 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-staging, linux-kernel, gustavoars

On Mon, Nov 28, 2022 at 08:53:28AM +0100, Greg Kroah-Hartman wrote:
> On Mon, Nov 28, 2022 at 01:15:43PM +0530, Deepak R Varma wrote:
> > >
> > > For your quick reference, the zero length structure declaration are online 963
> > > whereas the union is on line number 1080 of the file drivers/staging/wlan-ng/hfa384x.h
> >
> > Hello Greg,
> > can you please suggest how should I approach this clean-up/correction?
>
> Sorry, but I do not have the bandwidth to help out with this.  I will
> gladly review changes submitted only.

No problem. I completely understand and appreciate. Thank you Greg.

./drv

>
> greg k-h
>



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper
  2022-11-28  8:21             ` Deepak R Varma
@ 2022-11-28  8:25               ` Dan Carpenter
  2022-11-28  8:26                 ` Deepak R Varma
  0 siblings, 1 reply; 12+ messages in thread
From: Dan Carpenter @ 2022-11-28  8:25 UTC (permalink / raw)
  To: Deepak R Varma
  Cc: Greg Kroah-Hartman, linux-staging, linux-kernel, gustavoars

On Mon, Nov 28, 2022 at 01:51:58PM +0530, Deepak R Varma wrote:
> On Mon, Nov 28, 2022 at 10:50:19AM +0300, Dan Carpenter wrote:
> > On Mon, Nov 28, 2022 at 01:15:43PM +0530, Deepak R Varma wrote:
> > > On Sat, Nov 19, 2022 at 08:08:15PM +0530, Deepak R Varma wrote:
> > > > On Thu, Nov 17, 2022 at 07:03:21PM +0100, Greg Kroah-Hartman wrote:
> > > > > On Thu, Nov 17, 2022 at 06:50:55PM +0530, Deepak R Varma wrote:
> > > > > > On Thu, Nov 17, 2022 at 01:54:49PM +0100, Greg Kroah-Hartman wrote:
> > > > > > > On Thu, Nov 17, 2022 at 03:48:45PM +0530, Deepak R Varma wrote:
> > > > > > > >
> > > > > > > >  struct hfa384x_pdr_refdac_measurements {
> > > > > > > > -	u16 value[0];
> > > > > > > > +	DECLARE_FLEX_ARRAY(u16, value);
> > > > > > > >  } __packed;
> > > > > > >
> > > > > > > Why?  This structure is never used anywhere, right?  So why is this
> > > > > > > needed to be changed and not just removed entirely?  Same for the other
> > > > > > > structures in this patch.
> > > > > >
> > > > > > Hello Greg,
> > > > > > I am unable to confirm that these structures are truly not needed in the absence
> > > > > > if a real device based testing. I could only validate that using the compile
> > > > > > build and driver loading.
> > > > >
> > > > > Think this through, if no one is actually using this structure, and it
> > > > > is of 0 size, then do you think it is being used?
> > > >
> > > > Hello Greg,
> > > > I did not find any memory allocation for these zero length array structures.
> > > > Also, the union or its enclosing structure do not appear to access the members.
> > > > Hence I am leaning towards concluding that these zero length array structures do
> > > > not appear to be necessary.
> > > >
> > > > There are a few other structures that are part of the same union, however, they
> > > > too do not appear to be used for accessing the memory assigned to the union [or
> > > > its enclosing structure]. I think most of the members of these unions can be
> > > > replaced by one max size structure of this union [e.g. struct
> > > > hfa384x_pdr_mkk_measurements].
> > > >
> > > > Could you please comment if I am reading the code right?
> > > >
> > > > For your quick reference, the zero length structure declaration are online 963
> > > > whereas the union is on line number 1080 of the file drivers/staging/wlan-ng/hfa384x.h
> > >
> > > Hello Greg,
> > > can you please suggest how should I approach this clean-up/correction?
> > >
> >
> > Like this:
> 
> Thank you Dan. This takes me back to the very first version of this patch. I
> will send in the clean up.

Don't just send what I sent you, look around and try to see if there are
other issues with the code.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper
  2022-11-28  8:25               ` Dan Carpenter
@ 2022-11-28  8:26                 ` Deepak R Varma
  0 siblings, 0 replies; 12+ messages in thread
From: Deepak R Varma @ 2022-11-28  8:26 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: Greg Kroah-Hartman, linux-staging, linux-kernel, gustavoars

On Mon, Nov 28, 2022 at 11:25:01AM +0300, Dan Carpenter wrote:
> On Mon, Nov 28, 2022 at 01:51:58PM +0530, Deepak R Varma wrote:
> > On Mon, Nov 28, 2022 at 10:50:19AM +0300, Dan Carpenter wrote:
> > > Like this:
> >
> > Thank you Dan. This takes me back to the very first version of this patch. I
> > will send in the clean up.
>
> Don't just send what I sent you, look around and try to see if there are
> other issues with the code.

Yes, I will follow your advise.

Thanks,
./drv

>
> regards,
> dan carpenter
>
>



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-11-28  8:27 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-17 10:18 [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper Deepak R Varma
2022-11-17 12:54 ` Greg Kroah-Hartman
2022-11-17 13:20   ` Deepak R Varma
2022-11-17 18:03     ` Greg Kroah-Hartman
2022-11-19 14:38       ` Deepak R Varma
2022-11-28  7:45         ` Deepak R Varma
2022-11-28  7:50           ` Dan Carpenter
2022-11-28  8:21             ` Deepak R Varma
2022-11-28  8:25               ` Dan Carpenter
2022-11-28  8:26                 ` Deepak R Varma
2022-11-28  7:53           ` Greg Kroah-Hartman
2022-11-28  8:23             ` Deepak R Varma

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).