* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.