From: Dan Carpenter <error27@gmail.com>
To: Deepak R Varma <drv@mailo.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
gustavoars@kernel.org
Subject: Re: [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper
Date: Mon, 28 Nov 2022 10:50:19 +0300 [thread overview]
Message-ID: <Y4RoOxgo76Hc5RRP@kadam> (raw)
In-Reply-To: <Y4RnJyiCDYavLAJq@qemulion>
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;
next prev parent reply other threads:[~2022-11-28 7:50 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
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=Y4RoOxgo76Hc5RRP@kadam \
--to=error27@gmail.com \
--cc=drv@mailo.com \
--cc=gregkh@linuxfoundation.org \
--cc=gustavoars@kernel.org \
--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 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.