staging: greybus: Replace zero-length array by DECLARE_FLEX_ARRAY() helper

Message ID Y7T5MYfANf2xVsEx@qemulion
State New
Headers
Series staging: greybus: Replace zero-length array by DECLARE_FLEX_ARRAY() helper |

Commit Message

Deepak R Varma Jan. 4, 2023, 3:57 a.m. UTC
  The code currently uses C90 standard extension based zero length array
struct which is now deprecated and the new C99 standard extension of
flexible array declarations are to be used instead. Also, the macro
DECLARE_FLEX_ARRAY() allows to use single flexible array 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 flexible_array.cocci semantic patch.

Signed-off-by: Deepak R Varma <drv@mailo.com>
---
 drivers/staging/greybus/usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--
2.34.1
  

Comments

Alex Elder Jan. 4, 2023, 1:33 p.m. UTC | #1
On 1/3/23 9:57 PM, Deepak R Varma wrote:
> The code currently uses C90 standard extension based zero length array
> struct which is now deprecated and the new C99 standard extension of
> flexible array declarations are to be used instead. Also, the macro
> DECLARE_FLEX_ARRAY() allows to use single flexible array member in a
> structure. Refer to these links [1], [2] for details.

Thank you for citing some references in your commit, it's
a good and helpful practice.  This might have been another
helpful one:
 
https://lore.kernel.org/lkml/20210827163015.3141722-2-keescook@chromium.org/

> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> [2] https://lkml.kernel.org/r/YxKY6O2hmdwNh8r8@work

FYI, Linux mailing lists hosted by kernel.org are normally
cited using "lore.kernel.org" now, e.g.:
   https://lore.kernel.org/lkml/YxKY6O2hmdwNh8r8@work
Your patch is fine, this is just so you can consider this
in the future.

> Issue identified using Coccinelle flexible_array.cocci semantic patch.
> 
> Signed-off-by: Deepak R Varma <drv@mailo.com>

Looks good to me.

Reviewed-by: Alex Elder <elder@linaro.org>

> ---
>   drivers/staging/greybus/usb.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/greybus/usb.c b/drivers/staging/greybus/usb.c
> index 8e9d9d59a357..b7badf87a3f0 100644
> --- a/drivers/staging/greybus/usb.c
> +++ b/drivers/staging/greybus/usb.c
> @@ -27,7 +27,7 @@ struct gb_usb_hub_control_request {
>   };
> 
>   struct gb_usb_hub_control_response {
> -	u8 buf[0];
> +	DECLARE_FLEX_ARRAY(u8, buf);
>   };
> 
>   struct gb_usb_device {
> --
> 2.34.1
> 
> 
>
  
Deepak R Varma Jan. 4, 2023, 3:02 p.m. UTC | #2
On Wed, Jan 04, 2023 at 07:33:57AM -0600, Alex Elder wrote:
> On 1/3/23 9:57 PM, Deepak R Varma wrote:
> > The code currently uses C90 standard extension based zero length array
> > struct which is now deprecated and the new C99 standard extension of
> > flexible array declarations are to be used instead. Also, the macro
> > DECLARE_FLEX_ARRAY() allows to use single flexible array member in a
> > structure. Refer to these links [1], [2] for details.
>
> Thank you for citing some references in your commit, it's
> a good and helpful practice.  This might have been another
> helpful one:
>
> https://lore.kernel.org/lkml/20210827163015.3141722-2-keescook@chromium.org/

Hello Alex,
Thank you so much for the feedback. I will include the additional reference and
send in a v2.

>
> > [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> > [2] https://lkml.kernel.org/r/YxKY6O2hmdwNh8r8@work
>
> FYI, Linux mailing lists hosted by kernel.org are normally
> cited using "lore.kernel.org" now, e.g.:
>   https://lore.kernel.org/lkml/YxKY6O2hmdwNh8r8@work
> Your patch is fine, this is just so you can consider this
> in the future.

Sure, I will. This is very helpful to know.

>
> > Issue identified using Coccinelle flexible_array.cocci semantic patch.
> >
> > Signed-off-by: Deepak R Varma <drv@mailo.com>
>
> Looks good to me.

Appreciate your time for the review and comments. I will include the reviewed by
tag in my v2.

Regards,
./drv

>
> Reviewed-by: Alex Elder <elder@linaro.org>
>
> > ---
> >   drivers/staging/greybus/usb.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/greybus/usb.c b/drivers/staging/greybus/usb.c
> > index 8e9d9d59a357..b7badf87a3f0 100644
> > --- a/drivers/staging/greybus/usb.c
> > +++ b/drivers/staging/greybus/usb.c
> > @@ -27,7 +27,7 @@ struct gb_usb_hub_control_request {
> >   };
> >
> >   struct gb_usb_hub_control_response {
> > -	u8 buf[0];
> > +	DECLARE_FLEX_ARRAY(u8, buf);
> >   };
> >
> >   struct gb_usb_device {
> > --
> > 2.34.1
> >
> >
> >
>
  

Patch

diff --git a/drivers/staging/greybus/usb.c b/drivers/staging/greybus/usb.c
index 8e9d9d59a357..b7badf87a3f0 100644
--- a/drivers/staging/greybus/usb.c
+++ b/drivers/staging/greybus/usb.c
@@ -27,7 +27,7 @@  struct gb_usb_hub_control_request {
 };

 struct gb_usb_hub_control_response {
-	u8 buf[0];
+	DECLARE_FLEX_ARRAY(u8, buf);
 };

 struct gb_usb_device {