* [PATCH v2] mei: Avoid the use of one-element arrays @ 2020-07-14 21:45 Gustavo A. R. Silva 2020-07-22 18:27 ` Gustavo A. R. Silva 0 siblings, 1 reply; 8+ messages in thread From: Gustavo A. R. Silva @ 2020-07-14 21:45 UTC (permalink / raw) To: Tomas Winkler, Arnd Bergmann, Greg Kroah-Hartman Cc: linux-kernel, Gustavo A. R. Silva One-element arrays are being deprecated[1]. Replace the one-element arrays with a simple value type u8 reserved, once this is just a placeholder for alignment. Also, while there, use the preferred form for passing a size of a struct. The alternative form where struct name is spelled out hurts readability and introduces an opportunity for a bug when the variable type is changed but the corresponding sizeof that is passed as argument is not. [1] https://github.com/KSPP/linux/issues/79 Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> --- Changes in v2: - Use a more concise changelog text. drivers/misc/mei/hbm.c | 4 ++-- drivers/misc/mei/hw.h | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/misc/mei/hbm.c b/drivers/misc/mei/hbm.c index a44094cdbc36..f020d5594154 100644 --- a/drivers/misc/mei/hbm.c +++ b/drivers/misc/mei/hbm.c @@ -408,14 +408,14 @@ static int mei_hbm_add_cl_resp(struct mei_device *dev, u8 addr, u8 status) { struct mei_msg_hdr mei_hdr; struct hbm_add_client_response resp; - const size_t len = sizeof(struct hbm_add_client_response); + const size_t len = sizeof(resp); int ret; dev_dbg(dev->dev, "adding client response\n"); mei_hbm_hdr(&mei_hdr, len); - memset(&resp, 0, sizeof(struct hbm_add_client_response)); + memset(&resp, 0, len); resp.hbm_cmd = MEI_HBM_ADD_CLIENT_RES_CMD; resp.me_addr = addr; resp.status = status; diff --git a/drivers/misc/mei/hw.h b/drivers/misc/mei/hw.h index b1a8d5ec88b3..8c0297f0e7f3 100644 --- a/drivers/misc/mei/hw.h +++ b/drivers/misc/mei/hw.h @@ -346,13 +346,13 @@ struct hbm_add_client_request { * @hbm_cmd: bus message command header * @me_addr: address of the client in ME * @status: if HBMS_SUCCESS then the client can now accept connections. - * @reserved: reserved + * @reserved: reserved for alignment. */ struct hbm_add_client_response { u8 hbm_cmd; u8 me_addr; u8 status; - u8 reserved[1]; + u8 reserved; } __packed; /** @@ -461,7 +461,7 @@ struct hbm_notification { u8 hbm_cmd; u8 me_addr; u8 host_addr; - u8 reserved[1]; + u8 reserved; } __packed; /** -- 2.27.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] mei: Avoid the use of one-element arrays 2020-07-14 21:45 [PATCH v2] mei: Avoid the use of one-element arrays Gustavo A. R. Silva @ 2020-07-22 18:27 ` Gustavo A. R. Silva 2020-07-22 18:49 ` Greg Kroah-Hartman 2020-07-22 19:04 ` Winkler, Tomas 0 siblings, 2 replies; 8+ messages in thread From: Gustavo A. R. Silva @ 2020-07-22 18:27 UTC (permalink / raw) To: Gustavo A. R. Silva, Tomas Winkler, Arnd Bergmann, Greg Kroah-Hartman Cc: linux-kernel Hi all, Friendly ping: who can take this? :) Thanks -- Gustavo On 7/14/20 16:45, Gustavo A. R. Silva wrote: > One-element arrays are being deprecated[1]. Replace the one-element > arrays with a simple value type u8 reserved, once this is just a > placeholder for alignment. > > Also, while there, use the preferred form for passing a size of a struct. > The alternative form where struct name is spelled out hurts readability > and introduces an opportunity for a bug when the variable type is changed > but the corresponding sizeof that is passed as argument is not. > > [1] https://github.com/KSPP/linux/issues/79 > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > --- > Changes in v2: > - Use a more concise changelog text. > > drivers/misc/mei/hbm.c | 4 ++-- > drivers/misc/mei/hw.h | 6 +++--- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/misc/mei/hbm.c b/drivers/misc/mei/hbm.c > index a44094cdbc36..f020d5594154 100644 > --- a/drivers/misc/mei/hbm.c > +++ b/drivers/misc/mei/hbm.c > @@ -408,14 +408,14 @@ static int mei_hbm_add_cl_resp(struct mei_device *dev, u8 addr, u8 status) > { > struct mei_msg_hdr mei_hdr; > struct hbm_add_client_response resp; > - const size_t len = sizeof(struct hbm_add_client_response); > + const size_t len = sizeof(resp); > int ret; > > dev_dbg(dev->dev, "adding client response\n"); > > mei_hbm_hdr(&mei_hdr, len); > > - memset(&resp, 0, sizeof(struct hbm_add_client_response)); > + memset(&resp, 0, len); > resp.hbm_cmd = MEI_HBM_ADD_CLIENT_RES_CMD; > resp.me_addr = addr; > resp.status = status; > diff --git a/drivers/misc/mei/hw.h b/drivers/misc/mei/hw.h > index b1a8d5ec88b3..8c0297f0e7f3 100644 > --- a/drivers/misc/mei/hw.h > +++ b/drivers/misc/mei/hw.h > @@ -346,13 +346,13 @@ struct hbm_add_client_request { > * @hbm_cmd: bus message command header > * @me_addr: address of the client in ME > * @status: if HBMS_SUCCESS then the client can now accept connections. > - * @reserved: reserved > + * @reserved: reserved for alignment. > */ > struct hbm_add_client_response { > u8 hbm_cmd; > u8 me_addr; > u8 status; > - u8 reserved[1]; > + u8 reserved; > } __packed; > > /** > @@ -461,7 +461,7 @@ struct hbm_notification { > u8 hbm_cmd; > u8 me_addr; > u8 host_addr; > - u8 reserved[1]; > + u8 reserved; > } __packed; > > /** > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] mei: Avoid the use of one-element arrays 2020-07-22 18:27 ` Gustavo A. R. Silva @ 2020-07-22 18:49 ` Greg Kroah-Hartman 2020-07-22 19:00 ` Gustavo A. R. Silva 2020-07-22 19:04 ` Winkler, Tomas 1 sibling, 1 reply; 8+ messages in thread From: Greg Kroah-Hartman @ 2020-07-22 18:49 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Gustavo A. R. Silva, Tomas Winkler, Arnd Bergmann, linux-kernel On Wed, Jul 22, 2020 at 01:27:10PM -0500, Gustavo A. R. Silva wrote: > Hi all, > > Friendly ping: who can take this? :) I will, give me a chance... ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] mei: Avoid the use of one-element arrays 2020-07-22 18:49 ` Greg Kroah-Hartman @ 2020-07-22 19:00 ` Gustavo A. R. Silva 0 siblings, 0 replies; 8+ messages in thread From: Gustavo A. R. Silva @ 2020-07-22 19:00 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Gustavo A. R. Silva, Tomas Winkler, Arnd Bergmann, linux-kernel Hey Greg, On 7/22/20 13:49, Greg Kroah-Hartman wrote: > On Wed, Jul 22, 2020 at 01:27:10PM -0500, Gustavo A. R. Silva wrote: >> Hi all, >> >> Friendly ping: who can take this? :) > > I will, give me a chance... > Oops, sorry... I just wanted to make sure this makes it to 5.9-rc1. :) Thanks! -- Gustavo ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH v2] mei: Avoid the use of one-element arrays 2020-07-22 18:27 ` Gustavo A. R. Silva 2020-07-22 18:49 ` Greg Kroah-Hartman @ 2020-07-22 19:04 ` Winkler, Tomas 2020-07-22 19:29 ` Gustavo A. R. Silva 1 sibling, 1 reply; 8+ messages in thread From: Winkler, Tomas @ 2020-07-22 19:04 UTC (permalink / raw) To: Gustavo A. R. Silva, Gustavo A. R. Silva, Arnd Bergmann, Greg Kroah-Hartman Cc: linux-kernel@vger.kernel.org > > Hi all, > > Friendly ping: who can take this? :) > > Thanks > -- > Gustavo > > On 7/14/20 16:45, Gustavo A. R. Silva wrote: > > One-element arrays are being deprecated[1]. Replace the one-element > > arrays with a simple value type u8 reserved, once this is just a > > placeholder for alignment. > > > > Also, while there, use the preferred form for passing a size of a struct. > > The alternative form where struct name is spelled out hurts > > readability and introduces an opportunity for a bug when the variable > > type is changed but the corresponding sizeof that is passed as argument is > not. > > > > [1] https://github.com/KSPP/linux/issues/79 > > > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > > --- > > Changes in v2: > > - Use a more concise changelog text. > > > > drivers/misc/mei/hbm.c | 4 ++-- > > drivers/misc/mei/hw.h | 6 +++--- > > 2 files changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/misc/mei/hbm.c b/drivers/misc/mei/hbm.c index > > a44094cdbc36..f020d5594154 100644 > > --- a/drivers/misc/mei/hbm.c > > +++ b/drivers/misc/mei/hbm.c > > @@ -408,14 +408,14 @@ static int mei_hbm_add_cl_resp(struct mei_device > > *dev, u8 addr, u8 status) { > > struct mei_msg_hdr mei_hdr; > > struct hbm_add_client_response resp; > > - const size_t len = sizeof(struct hbm_add_client_response); > > + const size_t len = sizeof(resp); > > int ret; > > > > dev_dbg(dev->dev, "adding client response\n"); > > > > mei_hbm_hdr(&mei_hdr, len); > > > > - memset(&resp, 0, sizeof(struct hbm_add_client_response)); > > + memset(&resp, 0, len); > > resp.hbm_cmd = MEI_HBM_ADD_CLIENT_RES_CMD; > > resp.me_addr = addr; > > resp.status = status; This should be probably in a different patch it's not related to the second part. > > diff --git a/drivers/misc/mei/hw.h b/drivers/misc/mei/hw.h index > > b1a8d5ec88b3..8c0297f0e7f3 100644 > > --- a/drivers/misc/mei/hw.h > > +++ b/drivers/misc/mei/hw.h I have second thoughts of this part as all reserved fields in this file are of form u8 reserved[X], so we will lose that uniformity with this change, you have to look at the file as whole not just at the patch. So I prefer we drop that part of the patch. > > @@ -346,13 +346,13 @@ struct hbm_add_client_request { > > * @hbm_cmd: bus message command header > > * @me_addr: address of the client in ME > > * @status: if HBMS_SUCCESS then the client can now accept connections. > > - * @reserved: reserved > > + * @reserved: reserved for alignment. > > */ > > struct hbm_add_client_response { > > u8 hbm_cmd; > > u8 me_addr; > > u8 status; > > - u8 reserved[1]; > > + u8 reserved; > > } __packed; > > > > /** > > @@ -461,7 +461,7 @@ struct hbm_notification { > > u8 hbm_cmd; > > u8 me_addr; > > u8 host_addr; > > - u8 reserved[1]; > > + u8 reserved; > > } __packed; > > > > /** > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] mei: Avoid the use of one-element arrays 2020-07-22 19:04 ` Winkler, Tomas @ 2020-07-22 19:29 ` Gustavo A. R. Silva 2020-07-22 22:40 ` Winkler, Tomas 0 siblings, 1 reply; 8+ messages in thread From: Gustavo A. R. Silva @ 2020-07-22 19:29 UTC (permalink / raw) To: Winkler, Tomas, Gustavo A. R. Silva, Arnd Bergmann, Greg Kroah-Hartman Cc: linux-kernel@vger.kernel.org, Kees Cook Hi Tomas, Please, see my comments below... On 7/22/20 14:04, Winkler, Tomas wrote: > >> >> Hi all, >> >> Friendly ping: who can take this? :) >> >> Thanks >> -- >> Gustavo >> >> On 7/14/20 16:45, Gustavo A. R. Silva wrote: >>> One-element arrays are being deprecated[1]. Replace the one-element >>> arrays with a simple value type u8 reserved, once this is just a >>> placeholder for alignment. >>> >>> Also, while there, use the preferred form for passing a size of a struct. >>> The alternative form where struct name is spelled out hurts >>> readability and introduces an opportunity for a bug when the variable >>> type is changed but the corresponding sizeof that is passed as argument is >> not. >>> >>> [1] https://github.com/KSPP/linux/issues/79 >>> >>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> >>> --- >>> Changes in v2: >>> - Use a more concise changelog text. >>> >>> drivers/misc/mei/hbm.c | 4 ++-- >>> drivers/misc/mei/hw.h | 6 +++--- >>> 2 files changed, 5 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/misc/mei/hbm.c b/drivers/misc/mei/hbm.c index >>> a44094cdbc36..f020d5594154 100644 >>> --- a/drivers/misc/mei/hbm.c >>> +++ b/drivers/misc/mei/hbm.c >>> @@ -408,14 +408,14 @@ static int mei_hbm_add_cl_resp(struct mei_device >>> *dev, u8 addr, u8 status) { >>> struct mei_msg_hdr mei_hdr; >>> struct hbm_add_client_response resp; >>> - const size_t len = sizeof(struct hbm_add_client_response); >>> + const size_t len = sizeof(resp); >>> int ret; >>> >>> dev_dbg(dev->dev, "adding client response\n"); >>> >>> mei_hbm_hdr(&mei_hdr, len); >>> >>> - memset(&resp, 0, sizeof(struct hbm_add_client_response)); >>> + memset(&resp, 0, len); >>> resp.hbm_cmd = MEI_HBM_ADD_CLIENT_RES_CMD; >>> resp.me_addr = addr; >>> resp.status = status; > > This should be probably in a different patch it's not related to the second part. > >>> diff --git a/drivers/misc/mei/hw.h b/drivers/misc/mei/hw.h index >>> b1a8d5ec88b3..8c0297f0e7f3 100644 >>> --- a/drivers/misc/mei/hw.h >>> +++ b/drivers/misc/mei/hw.h > I have second thoughts of this part as all reserved fields in this file are of form u8 reserved[X], > so we will lose that uniformity with this change, you have to look at the file as whole > not just at the patch. So I prefer we drop that part of the patch. > This is actually the main point of this patch: the removal of one-element arrays. And yeah, every place in the kernel that uses the form that you mention will see it's uniformity slightly modified, and that's for a good cause: the removal of one-element arrays, so we can enable bounds checking. Thanks -- Gustavo >>> @@ -346,13 +346,13 @@ struct hbm_add_client_request { >>> * @hbm_cmd: bus message command header >>> * @me_addr: address of the client in ME >>> * @status: if HBMS_SUCCESS then the client can now accept connections. >>> - * @reserved: reserved >>> + * @reserved: reserved for alignment. >>> */ >>> struct hbm_add_client_response { >>> u8 hbm_cmd; >>> u8 me_addr; >>> u8 status; >>> - u8 reserved[1]; >>> + u8 reserved; >>> } __packed; >>> >>> /** >>> @@ -461,7 +461,7 @@ struct hbm_notification { >>> u8 hbm_cmd; >>> u8 me_addr; >>> u8 host_addr; >>> - u8 reserved[1]; >>> + u8 reserved; >>> } __packed; >>> >>> /** >>> ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH v2] mei: Avoid the use of one-element arrays 2020-07-22 19:29 ` Gustavo A. R. Silva @ 2020-07-22 22:40 ` Winkler, Tomas 2020-07-22 23:01 ` Gustavo A. R. Silva 0 siblings, 1 reply; 8+ messages in thread From: Winkler, Tomas @ 2020-07-22 22:40 UTC (permalink / raw) To: Gustavo A. R. Silva, Gustavo A. R. Silva, Arnd Bergmann, Greg Kroah-Hartman Cc: linux-kernel@vger.kernel.org, Kees Cook > > Hi Tomas, > > Please, see my comments below... > > On 7/22/20 14:04, Winkler, Tomas wrote: > > > >> > >> Hi all, > >> > >> Friendly ping: who can take this? :) > >> > >> Thanks > >> -- > >> Gustavo > >> > >> On 7/14/20 16:45, Gustavo A. R. Silva wrote: > >>> One-element arrays are being deprecated[1]. Replace the one-element > >>> arrays with a simple value type u8 reserved, once this is just a > >>> placeholder for alignment. > >>> > >>> Also, while there, use the preferred form for passing a size of a struct. > >>> The alternative form where struct name is spelled out hurts > >>> readability and introduces an opportunity for a bug when the > >>> variable type is changed but the corresponding sizeof that is passed > >>> as argument is > >> not. > >>> > >>> [1] https://github.com/KSPP/linux/issues/79 > >>> > >>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > >>> --- > >>> Changes in v2: > >>> - Use a more concise changelog text. > >>> > >>> drivers/misc/mei/hbm.c | 4 ++-- > >>> drivers/misc/mei/hw.h | 6 +++--- > >>> 2 files changed, 5 insertions(+), 5 deletions(-) > >>> > >>> diff --git a/drivers/misc/mei/hbm.c b/drivers/misc/mei/hbm.c index > >>> a44094cdbc36..f020d5594154 100644 > >>> --- a/drivers/misc/mei/hbm.c > >>> +++ b/drivers/misc/mei/hbm.c > >>> @@ -408,14 +408,14 @@ static int mei_hbm_add_cl_resp(struct > >>> mei_device *dev, u8 addr, u8 status) { > >>> struct mei_msg_hdr mei_hdr; > >>> struct hbm_add_client_response resp; > >>> - const size_t len = sizeof(struct hbm_add_client_response); > >>> + const size_t len = sizeof(resp); > >>> int ret; > >>> > >>> dev_dbg(dev->dev, "adding client response\n"); > >>> > >>> mei_hbm_hdr(&mei_hdr, len); > >>> > >>> - memset(&resp, 0, sizeof(struct hbm_add_client_response)); > >>> + memset(&resp, 0, len); > >>> resp.hbm_cmd = MEI_HBM_ADD_CLIENT_RES_CMD; > >>> resp.me_addr = addr; > >>> resp.status = status; > > > > This should be probably in a different patch it's not related to the second > part. Frankly I will post other version of this that cleans the whole file. > > > >>> diff --git a/drivers/misc/mei/hw.h b/drivers/misc/mei/hw.h index > >>> b1a8d5ec88b3..8c0297f0e7f3 100644 > >>> --- a/drivers/misc/mei/hw.h > >>> +++ b/drivers/misc/mei/hw.h > > I have second thoughts of this part as all reserved fields in this > > file are of form u8 reserved[X], so we will lose that uniformity with > > this change, you have to look at the file as whole not just at the patch. So I > prefer we drop that part of the patch. > > > > This is actually the main point of this patch: the removal of one-element > arrays. > And yeah, every place in the kernel that uses the form that you mention will > see it's uniformity slightly modified, and that's for a good cause: the removal > of one-element arrays, so we can enable bounds checking. I was going over https://github.com/KSPP/linux/issues/79, I'm not sure this all related to flexible arrays, those are just reserved struct members. So because it's hard to identify a legitimate usage of single element arrays we are going to kill them all? It's more esthetic / readability issue here but there might be some legit use case for one element array, no? > > Thanks > -- > Gustavo > > >>> @@ -346,13 +346,13 @@ struct hbm_add_client_request { > >>> * @hbm_cmd: bus message command header > >>> * @me_addr: address of the client in ME > >>> * @status: if HBMS_SUCCESS then the client can now accept > connections. > >>> - * @reserved: reserved > >>> + * @reserved: reserved for alignment. > >>> */ > >>> struct hbm_add_client_response { > >>> u8 hbm_cmd; > >>> u8 me_addr; > >>> u8 status; > >>> - u8 reserved[1]; > >>> + u8 reserved; > >>> } __packed; > >>> > >>> /** > >>> @@ -461,7 +461,7 @@ struct hbm_notification { > >>> u8 hbm_cmd; > >>> u8 me_addr; > >>> u8 host_addr; > >>> - u8 reserved[1]; > >>> + u8 reserved; > >>> } __packed; > >>> > >>> /** > >>> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] mei: Avoid the use of one-element arrays 2020-07-22 22:40 ` Winkler, Tomas @ 2020-07-22 23:01 ` Gustavo A. R. Silva 0 siblings, 0 replies; 8+ messages in thread From: Gustavo A. R. Silva @ 2020-07-22 23:01 UTC (permalink / raw) To: Winkler, Tomas, Gustavo A. R. Silva, Arnd Bergmann, Greg Kroah-Hartman Cc: linux-kernel@vger.kernel.org, Kees Cook On 7/22/20 17:40, Winkler, Tomas wrote: >> >> Hi Tomas, >> >> Please, see my comments below... >> >> On 7/22/20 14:04, Winkler, Tomas wrote: >>> >>>> >>>> Hi all, >>>> >>>> Friendly ping: who can take this? :) >>>> >>>> Thanks >>>> -- >>>> Gustavo >>>> >>>> On 7/14/20 16:45, Gustavo A. R. Silva wrote: >>>>> One-element arrays are being deprecated[1]. Replace the one-element >>>>> arrays with a simple value type u8 reserved, once this is just a >>>>> placeholder for alignment. >>>>> >>>>> Also, while there, use the preferred form for passing a size of a struct. >>>>> The alternative form where struct name is spelled out hurts >>>>> readability and introduces an opportunity for a bug when the >>>>> variable type is changed but the corresponding sizeof that is passed >>>>> as argument is >>>> not. >>>>> >>>>> [1] https://github.com/KSPP/linux/issues/79 >>>>> >>>>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> >>>>> --- >>>>> Changes in v2: >>>>> - Use a more concise changelog text. >>>>> >>>>> drivers/misc/mei/hbm.c | 4 ++-- >>>>> drivers/misc/mei/hw.h | 6 +++--- >>>>> 2 files changed, 5 insertions(+), 5 deletions(-) >>>>> >>>>> diff --git a/drivers/misc/mei/hbm.c b/drivers/misc/mei/hbm.c index >>>>> a44094cdbc36..f020d5594154 100644 >>>>> --- a/drivers/misc/mei/hbm.c >>>>> +++ b/drivers/misc/mei/hbm.c >>>>> @@ -408,14 +408,14 @@ static int mei_hbm_add_cl_resp(struct >>>>> mei_device *dev, u8 addr, u8 status) { >>>>> struct mei_msg_hdr mei_hdr; >>>>> struct hbm_add_client_response resp; >>>>> - const size_t len = sizeof(struct hbm_add_client_response); >>>>> + const size_t len = sizeof(resp); >>>>> int ret; >>>>> >>>>> dev_dbg(dev->dev, "adding client response\n"); >>>>> >>>>> mei_hbm_hdr(&mei_hdr, len); >>>>> >>>>> - memset(&resp, 0, sizeof(struct hbm_add_client_response)); >>>>> + memset(&resp, 0, len); >>>>> resp.hbm_cmd = MEI_HBM_ADD_CLIENT_RES_CMD; >>>>> resp.me_addr = addr; >>>>> resp.status = status; >>> >>> This should be probably in a different patch it's not related to the second >> part. > > > Frankly I will post other version of this that cleans the whole file. > Sounds good. :) >>> >>>>> diff --git a/drivers/misc/mei/hw.h b/drivers/misc/mei/hw.h index >>>>> b1a8d5ec88b3..8c0297f0e7f3 100644 >>>>> --- a/drivers/misc/mei/hw.h >>>>> +++ b/drivers/misc/mei/hw.h >>> I have second thoughts of this part as all reserved fields in this >>> file are of form u8 reserved[X], so we will lose that uniformity with >>> this change, you have to look at the file as whole not just at the patch. So I >> prefer we drop that part of the patch. >>> >> >> This is actually the main point of this patch: the removal of one-element >> arrays. >> And yeah, every place in the kernel that uses the form that you mention will >> see it's uniformity slightly modified, and that's for a good cause: the removal >> of one-element arrays, so we can enable bounds checking. > > I was going over https://github.com/KSPP/linux/issues/79, I'm not sure this all related to flexible arrays, I've opened a new issue for this: https://github.com/KSPP/linux/issues/86 And I'm already including the link above in these sorts of patches; please, see: https://lore.kernel.org/lkml/20200717215500.GA13910@embeddedor/ > those are just reserved struct members. So because it's hard to identify a legitimate usage of single element arrays > we are going to kill them all? It's more esthetic / readability issue here but there might be some legit use case for one element array, no? > The code is continuously evolving and, if a one-element array is not intended to be used as a variable-length array at all, I frankly cannot think of any another use that is not merely esthetic/readability. :) Thanks -- Gustavo > >> >> Thanks >> -- >> Gustavo >> >>>>> @@ -346,13 +346,13 @@ struct hbm_add_client_request { >>>>> * @hbm_cmd: bus message command header >>>>> * @me_addr: address of the client in ME >>>>> * @status: if HBMS_SUCCESS then the client can now accept >> connections. >>>>> - * @reserved: reserved >>>>> + * @reserved: reserved for alignment. >>>>> */ >>>>> struct hbm_add_client_response { >>>>> u8 hbm_cmd; >>>>> u8 me_addr; >>>>> u8 status; >>>>> - u8 reserved[1]; >>>>> + u8 reserved; >>>>> } __packed; >>>>> >>>>> /** >>>>> @@ -461,7 +461,7 @@ struct hbm_notification { >>>>> u8 hbm_cmd; >>>>> u8 me_addr; >>>>> u8 host_addr; >>>>> - u8 reserved[1]; >>>>> + u8 reserved; >>>>> } __packed; >>>>> >>>>> /** >>>>> ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-07-22 23:20 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-07-14 21:45 [PATCH v2] mei: Avoid the use of one-element arrays Gustavo A. R. Silva 2020-07-22 18:27 ` Gustavo A. R. Silva 2020-07-22 18:49 ` Greg Kroah-Hartman 2020-07-22 19:00 ` Gustavo A. R. Silva 2020-07-22 19:04 ` Winkler, Tomas 2020-07-22 19:29 ` Gustavo A. R. Silva 2020-07-22 22:40 ` Winkler, Tomas 2020-07-22 23:01 ` Gustavo A. R. Silva
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox