On Tue, Apr 12, 2022 at 06:57:11AM +0200, Greg Kroah-Hartman wrote:
> On Tue, Apr 12, 2022 at 06:56:12AM +0200, Greg Kroah-Hartman wrote:
> > On Mon, Apr 11, 2022 at 05:14:11PM -0400, Jaehee Park wrote:
> > > Zero-length and one-element arrays are deprecated. Flexible-array
> > > members should be used instead. Flexible-array members are
> > > recommended because this is the way the kernel expects dynamically
> > > sized trailing elements to be declared.
> > > Refer to Documentation/process/deprecated.rst.
> > >
> > > Change the zero-length array, buf, in the struct
> > > gb_usb_hub_control_response to a flexible array. And add wLength as a
> > > member of the struct so that the struct is not a zero-sized struct.
> > >
> > > Issue found by flexible_array coccinelle script.
> > >
> > > Signed-off-by: Jaehee Park <jhpark1013@xxxxxxxxx>
> > > ---
> > >
> > > I have a question for the authors:
> > > I saw a fixme comment in the hub_control function in usb.c:
> > > / FIXME: handle unspecified lengths /
> > >
> > > I was wondering why this comment was left there?
> > >
> > > In this patch, I'm using this struct:
> > >
> > > struct gb_usb_hub_control_response {
> > > __le16 wLength;
> > > u8 buf[];
> > > };
> > >
> > > And instead of using response_size, I'm doing this:
> > >
> > > struct gb_usb_hub_control_response *response;
> > > And using sizeof(*response) as the input to gb_operation_create.
> > >
> > > Would the flexible array address the handling of unspecified lengths
> > > issue (in the fixme comment)?
> >
> > No, you can not change the format of the data on the bus without also
> > changing the firmware in the device and usually the specification as
> > well.
> >
> > > drivers/staging/greybus/usb.c | 7 +++----
> > > 1 file changed, 3 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/staging/greybus/usb.c b/drivers/staging/greybus/usb.c
> > > index 8e9d9d59a357..d0b2422401df 100644
> > > --- a/drivers/staging/greybus/usb.c
> > > +++ b/drivers/staging/greybus/usb.c
> > > @@ -27,7 +27,8 @@ struct gb_usb_hub_control_request {
> > > };
> > >
> > > struct gb_usb_hub_control_response {
> > > - u8 buf[0];
> > > + __le16 wLength;
> > > + u8 buf[];
> >
> > What is wrong with buf[0] here?
> >
> > You can fix this in other ways if you really understand the difference
> > between [0] and [] in C. Please look at many of the other conversions
> > if you wish to do this.
>
> And I would not recommend this as an "outreachy introduction task"
> unless you understand this. There are much easier first patch tasks you
> can accomplish instead.
>
Hi Greg,
I should've made this into a question for the maintainers instead of a
patch. Sorry about that. Dan's and your comments are well noted.
Thank you,
Jaehee
> good luck!
>
> greg k-h
_______________________________________________
greybus-dev mailing list -- greybus-dev@xxxxxxxxxxxxxxxx
To unsubscribe send an email to greybus-dev-leave@xxxxxxxxxxxxxxxx
|