Message ID | Y3YKhee8L+kAfHM4@qemulion |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:f944:0:0:0:0:0 with SMTP id q4csp320949wrr; Thu, 17 Nov 2022 02:27:47 -0800 (PST) X-Google-Smtp-Source: AA0mqf7ZRVGy/slXUJYsZ2b4I+Dwec5x58Y9CJKnyokEIKBHJ2AUqd5lcGdUAmKqVKuxEwF3ypmr X-Received: by 2002:a17:90a:ff84:b0:213:1e05:f992 with SMTP id hf4-20020a17090aff8400b002131e05f992mr8185596pjb.191.1668680867013; Thu, 17 Nov 2022 02:27:47 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1668680867; cv=none; d=google.com; s=arc-20160816; b=mVOyUg4fCJlR0V8j0vjv6ZVpJf9hJHJnSC+JADf20rIs4bcwhXBmEa/Ge/1hrFmIp2 /8qwdUck3q55TXQ1JjBQxFM3k0TLxxh1/Zo6gcEt92okNGdzzhLpvqKerv9DSlelnMif ISa+eU+I6SsQl4+bybmE5RxkcFUiSa920ygYh/wpxSFLGVYa4bBbYTjtoaGtQPi9OaCq rrIBPvaWhS9lN6SJstuz4mBbQENAdRVF95nzneKIZf6z6/ERA0lZRazsD1BQINUBe5wJ yYKIOKTYpHsCnyFiXAqQNZsRaMim20bSQLYW/nxEkXww7aKRS+w66ujed+5+X8YjuMpW sJzw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-disposition:mime-version:message-id :subject:cc:to:from:date:dkim-signature; bh=bWjU9Uh40dzPHwLcnJrqI3pgAhGTk+l8NO+ICwrYIUg=; b=cLutMIfDgnIqLWuprokvSL8n/kVccUudxAhjf8UclbUnIx/DBSXFAv/QqOkuRRRHI9 hygKLJEts+9Em4uJU/Fah7txfSEsVasOTplsMd/EuxwZshzsI83E2rtRxmeYSbEf9P/M WhuSsX3EsVjbu4EWBjBG0Gxs/iE+qDhph7HUzfQZkvLCwi9HjGopdmKAQoBI4ueoMfBy 6p9M5Qj/Pdo4yAuCp3DrXvMkw/P6OE7aAwKCvfbGrq9F4g3aBepFgWvw3Nzfa5VJ7Xdi A+hzH0Ggm799mjIAZYyQnvDEAt/4hxO1SjGGChwCX6AbKY560z+QSXS6ZFzpZ5bvytUB oDFQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@mailo.com header.s=mailo header.b="LvbMT/p3"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=mailo.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id ng18-20020a17090b1a9200b0020d3fa4d1edsi1813579pjb.64.2022.11.17.02.27.34; Thu, 17 Nov 2022 02:27:47 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=fail header.i=@mailo.com header.s=mailo header.b="LvbMT/p3"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=mailo.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239480AbiKQKTQ (ORCPT <rfc822;just.gull.subs@gmail.com> + 99 others); Thu, 17 Nov 2022 05:19:16 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54592 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229931AbiKQKTM (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 17 Nov 2022 05:19:12 -0500 Received: from msg-1.mailo.com (msg-1.mailo.com [213.182.54.11]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B1C554D5ED for <linux-kernel@vger.kernel.org>; Thu, 17 Nov 2022 02:19:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=mailo.com; s=mailo; t=1668680340; bh=SzVxtKeetSOTjll6GIenMJHr7FVUowUUcMEMgbJoa9M=; h=X-EA-Auth:Date:From:To:Cc:Subject:Message-ID:MIME-Version: Content-Type; b=LvbMT/p3+qBL53qZms+nv4EtBnzKNLk+Cn+km56ubnzcPc/mjKgv0eUv3uorg61JB zrb0lUuLtPzZRx4B09DlEV4MaDBqIpgPIbFGMgfCAWPxHYHaZFR+2D1ijOyXM2CsMf VsIl2r3Bg3f9lY2gkoAr64ubnMBTv7As/DBijdm8= Received: by b-4.in.mailobj.net [192.168.90.14] with ESMTP via ip-206.mailobj.net [213.182.55.206] Thu, 17 Nov 2022 11:19:00 +0100 (CET) X-EA-Auth: usK3miqBlpaK6KvcL4GXadFp8n6JQ3Xl8zXAoWm5N3DuIV6veHX1uibGv0qvoiLHAPSFUCRL/fEV5ZzpFdDcgFINefyT/hZR Date: Thu, 17 Nov 2022 15:48:45 +0530 From: Deepak R Varma <drv@mailo.com> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>, linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org Cc: gustavoars@kernel.org Subject: [PATCH] staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper Message-ID: <Y3YKhee8L+kAfHM4@qemulion> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1749738708885324329?= X-GMAIL-MSGID: =?utf-8?q?1749738708885324329?= |
Series |
staging: wlan-ng: Replace zero-length arrays with DECLARE_FLEX_ARRAY() helper
|
|
Commit Message
Deepak R Varma
Nov. 17, 2022, 10:18 a.m. UTC
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(-)
--
2.34.1
Comments
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
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
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
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 >
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 > >
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;
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
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; >
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 >
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
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 > >
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 {