On Fri, Oct 28, 2022 at 07:11:00PM +0530, Deepak R Varma wrote: > On Fri, Oct 28, 2022 at 04:33:59PM +0300, Dan Carpenter wrote: > > On Fri, Oct 28, 2022 at 06:08:13PM +0530, Deepak R Varma wrote: > > > Flexible-array member should be used instead of one or zero member to > > > meet the need for having a dynamically sized trailing elements in a > > > structure. Refer to links [1] and [2] for detailed guidance on this > > > suggestion. > > > > > > [1] https://en.wikipedia.org/wiki/Flexible_array_member > > > [2] https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays > > > > > > Issue identified using coccicheck. > > > > > > Signed-off-by: Deepak R Varma <drv@xxxxxxxxx> > > > --- > > > drivers/staging/wlan-ng/p80211mgmt.h | 8 ++++---- > > > drivers/staging/wlan-ng/p80211types.h | 2 +- > > > 2 files changed, 5 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/staging/wlan-ng/p80211mgmt.h b/drivers/staging/wlan-ng/p80211mgmt.h > > > index 1ef30d3f3159..d6fe52de2c8f 100644 > > > --- a/drivers/staging/wlan-ng/p80211mgmt.h > > > +++ b/drivers/staging/wlan-ng/p80211mgmt.h > > > @@ -229,14 +229,14 @@ struct wlan_ie { > > > struct wlan_ie_ssid { > > > u8 eid; > > > u8 len; > > > - u8 ssid[1]; /* may be zero, ptrs may overlap */ > > > + u8 ssid[]; /* may be zero, ptrs may overlap */ > > > > How have you ensured that changing this from a 1 byte array to a zero > > byte array does not break anything? It's not uncommon for a people > > to do math like "size - 1 + length". The "- 1" would be to take the > > 1 element into consideration. > > Hi Dan, > I did a code review to understand how this structure member is used and did not > find any computations you mentioned. I would certainly like to receive your feedback > as well. > That would be useful information for the commit message. Even if it goes under the --- cut off. --- Compile tested only. I audited all fourty two uses of the wlan_ie_ssid struct and nothing checks sizeof(struct wlan_ie_ssid) and changing this from 1 to 0 will not break anything. > > > > I was trying to read through this code to check your work, but then > > you sent a second patch which also does not explain how you are auditing > > your changes. Can you go a bit slower? > > My apologies for rushing patches in. I will hold on for feedback on these > patches before turning in any new patch involving similar change. I hope it is > okay to send a different type of patch though. Please correct if I am wrong. I'm not saying you have to wait for days for reviewers to get through your patches... I guess what I'm really trying to say is please group your patches together. Pavel Skripkin's review comments seem reasonable so if the patches are grouped then he only has to reply once but now someone has to reply to every email. Also if you write 10 patches maybe you will notice or think of something part way through and which forces you to re-write the earlier patches. When you send patches every ten minutes then everyone can see you have not tested it at all. I don't really expect people to test their staging patches, but let's not make it obvious. Wait overnight so people think "Ah, maybe they were tested". It gives people (false) confidence that this is quality work. regards, dan carpenter
