public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Deepak R Varma <drv@mailo.com>
Cc: linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: staging/wlan-ng query: convert to flexible array member
Date: Tue, 8 Nov 2022 16:52:44 +0100	[thread overview]
Message-ID: <Y2p7TOeg8vzK0rvB@kroah.com> (raw)
In-Reply-To: <Y2p5hqdFo5UZoHUY@qemulion>

On Tue, Nov 08, 2022 at 09:15:10PM +0530, Deepak R Varma wrote:
> On Tue, Nov 08, 2022 at 04:34:15PM +0100, Greg Kroah-Hartman wrote:
> > On Tue, Nov 08, 2022 at 08:42:59PM +0530, Deepak R Varma wrote:
> > > Hello,
> > >
> > > First, my apologies for the long email.
> > > I am requesting guidance on how to approach resolving the zero element flexible
> > > VLO struct implementation in this driver in file drivers/staging/waln-ng/hfa384x.f
> > >
> > > The struct hfa384x_pdrec contains nested structs with zero element arrays.  These
> > > zero element structs are part of a union 'data' inside the struct container. This
> > > union 'data' is the last element of this container. Please see the code snip below:
> > >
> > > <snip>
> > >
> > > 	1068 struct hfa384x_pdrec {
> > > 	   1         __le16 len;             /* in words */
> > > 	   2         __le16 code;
> > > 	   3         union pdr {
> > > 	   4                 struct hfa384x_pdr_pcb_partnum pcb_partnum;
> > > 	  11                 struct hfa384x_pdr_nicid nicid;
> > > 	  12                 struct hfa384x_pdr_refdac_measurements refdac_measurements;
> > > 	  13                 struct hfa384x_pdr_vgdac_measurements vgdac_measurements;
> > > 	  14                 struct hfa384x_pdr_level_comp_measurements level_compc_measurements;
> > > 	  15                 struct hfa384x_pdr_mac_address mac_address;
> > > 	  39         } data;
> > > 	  40 } __packed;
> > >
> > > </snip>
> > >
> > > The three structures in question are declared as follows in the same file:
> > >
> > > <snip>
> > > 	962  struct hfa384x_pdr_refdac_measurements {
> > > 	   1         u16 value[0];
> > > 	   2 } __packed;
> > > 	   3
> > > 	   4 struct hfa384x_pdr_vgdac_measurements {
> > > 	   5         u16 value[0];
> > > 	   6 } __packed;
> > > 	   7
> > > 	   8 struct hfa384x_pdr_level_comp_measurements {
> > > 	   9         u16 value[0];
> > > 	  10 } __packed;
> > > </snip>
> > >
> > > As per the C99 specifications, the flexible array struct should have at least
> > > one member other than the true flexible array member. So converting these from
> > > [0] to [] is not feasible in the current form.
> > >
> > > I did not find these struct variables being used for memory allocation in the
> > > code directly. My find may be short since the implementation appears to get very
> > > complex as I tried to get deeper.
> > >
> > > Can you please suggest how should I approach correcting the zero element flex
> > > array implementation here? Can these structs be removed if they are unused?
> >
> > Are you sure they are unused?
> >
> > They look like structures that are read from the memory of a device,
> > right?  Try removing the structures from the union and see what happens
> > :)
> 
> I did remove the structs from the union and it built fine. Is there anything else I
> can check/test to verify the impact?
> 
> <snip>
> 	drv@qemulion:~/git/kernels/staging$ git diff
> 	diff --git a/drivers/staging/wlan-ng/hfa384x.h b/drivers/staging/wlan-ng/hfa384x.h
> 	index 0611e37df6ac..8fe10aa93dfb 100644
> 	--- a/drivers/staging/wlan-ng/hfa384x.h
> 	+++ b/drivers/staging/wlan-ng/hfa384x.h
> 	@@ -1077,9 +1077,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;
> 			struct hfa384x_pdr_mkk_callname mkk_callname;
> 			struct hfa384x_pdr_regdomain regdomain;
> 	drv@qemulion:~/git/kernels/staging$ make M=drivers/staging/wlan-ng/
> 	  CC [M]  drivers/staging/wlan-ng/prism2usb.o
> 	  CC [M]  drivers/staging/wlan-ng/p80211netdev.o
> 	  LD [M]  drivers/staging/wlan-ng/prism2_usb.o
> 	  MODPOST drivers/staging/wlan-ng/Module.symvers
> 	  LD [M]  drivers/staging/wlan-ng/prism2_usb.ko
> 	  BTF [M] drivers/staging/wlan-ng/prism2_usb.ko
> 	drv@qemulion:~/git/kernels/staging$
> </snip>
> 

Test the device to make sure it still works?

  reply	other threads:[~2022-11-08 15:52 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-08 15:12 staging/wlan-ng query: convert to flexible array member Deepak R Varma
2022-11-08 15:34 ` Greg Kroah-Hartman
2022-11-08 15:45   ` Deepak R Varma
2022-11-08 15:52     ` Greg Kroah-Hartman [this message]
2022-11-08 17:37       ` Deepak R Varma
2022-11-08 18:32         ` Greg Kroah-Hartman
2022-11-08 18:38           ` Deepak R Varma
2022-11-08 19:43             ` Greg Kroah-Hartman
2022-11-09  6:51               ` Deepak R Varma
2022-11-13  6:33 ` Gustavo A. R. Silva
2022-11-13  7:38   ` Deepak R Varma

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y2p7TOeg8vzK0rvB@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=drv@mailo.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox