[PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper

[PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper

[Deepak R Varma]

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

Re: [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper

[Greg Kroah-Hartman]

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

Re: [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper

[Deepak R Varma]

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

Re: [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper

[Greg Kroah-Hartman]

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

Re: [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper

[Deepak R Varma]

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
>

Re: [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper

[Deepak R Varma]

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
> >

Re: [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper

[Dan Carpenter]

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;

Re: [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper

[Deepak R Varma]

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;
>

Re: [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper

[Dan Carpenter]

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

Re: [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper

[Deepak R Varma]

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
>
>

Re: [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper

[Greg Kroah-Hartman]

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

Re: [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper

[Deepak R Varma]

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
>