| Message ID | cover.1645513670.git.gustavoars@kernel.org |
|---|---|
| Headers | show |
| Series | scsi: aacraid: Replace one-element arrays with flexible-array members | expand |
Hi all, Friendly ping: who can review or comment on this series, please? Thanks -- Gustavo On Tue, Feb 22, 2022 at 01:28:12AM -0600, Gustavo A. R. Silva wrote: > This series aims to replace one-element arrays with flexible-array > members in multiple structures in drivers/scsi/aacraid/aacraid.h. > > There is a regular need in the kernel to provide a way to declare having > a dynamically sized set of trailing elements in a structure. Kernel code > should always use “flexible array members”[1] for these cases. The older > style of one-element or zero-length arrays should no longer be used[2]. > > This helps with the ongoing efforts to globally enable -Warray-bounds > and get us closer to being able to tighten the FORTIFY_SOURCE routines > on memcpy(). > > These issues were found with the help of Coccinelle and audited and fixed, > manually. > > [1] https://en.wikipedia.org/wiki/Flexible_array_member > [2] https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays > > Link: https://github.com/KSPP/linux/issues/79 > > Gustavo A. R. Silva (8): > scsi: aacraid: Replace one-element array with flexible-array member > scsi: aacraid: Replace one-element array with flexible-array member in > struct sgmap > scsi: aacraid: Replace one-element array with flexible-array member in > struct user_sgmap > scsi: aacraid: Replace one-element array with flexible-array member in > struct sgmap64 > scsi: aacraid: Replace one-element array with flexible-array member in > struct user_sgmap64 > scsi: aacraid: Replace one-element array with flexible-array member in > struct sgmapraw > scsi: aacraid: Replace one-element array with flexible-array member in > struct user_sgmapraw > scsi: aacraid: Replace one-element array with flexible-array member in > struct aac_aifcmd > > drivers/scsi/aacraid/aachba.c | 76 +++++++++++---------------------- > drivers/scsi/aacraid/aacraid.h | 16 +++---- > drivers/scsi/aacraid/commctrl.c | 5 +-- > drivers/scsi/aacraid/comminit.c | 3 +- > 4 files changed, 37 insertions(+), 63 deletions(-) > > -- > 2.27.0 >
Gustavo,
> Friendly ping: who can review or comment on this series, please?
I'm afraid I don't have any hardware to test it on and the generated
output differs substantially from the original code.
On Tue, Mar 15, 2022 at 12:02:13AM -0400, Martin K. Petersen wrote: > > Gustavo, > > > Friendly ping: who can review or comment on this series, please? > > I'm afraid I don't have any hardware to test it on and the generated > output differs substantially from the original code. Yeah; this series requires careful review from the people that knows the code better. It took me a day of work to go through all the places that needed to be changed due to the flexible array transformation. However, due to the kind of changes, it'd be great to have a second opinion or at least someone that could take a look at the changes. Thanks -- Gustavo
On Mon, Mar 14, 2022 at 11:22:23PM -0500, Gustavo A. R. Silva wrote: > On Tue, Mar 15, 2022 at 12:02:13AM -0400, Martin K. Petersen wrote: > > > > Gustavo, > > > > > Friendly ping: who can review or comment on this series, please? > > > > I'm afraid I don't have any hardware to test it on and the generated > > output differs substantially from the original code. > > Yeah; this series requires careful review from the people that > knows the code better. > > It took me a day of work to go through all the places that needed > to be changed due to the flexible array transformation. However, > due to the kind of changes, it'd be great to have a second opinion > or at least someone that could take a look at the changes. If the int/size_t changes are separated from the array size change, it's easier to see the array size change is a binary no-op. (i.e. diffoscope shows no executable changes.) I'd recommend splitting the int/size_t changes from the array size changes.
This series aims to replace one-element arrays with flexible-array members in multiple structures in drivers/scsi/aacraid/aacraid.h. There is a regular need in the kernel to provide a way to declare having a dynamically sized set of trailing elements in a structure. Kernel code should always use “flexible array members”[1] for these cases. The older style of one-element or zero-length arrays should no longer be used[2]. This helps with the ongoing efforts to globally enable -Warray-bounds and get us closer to being able to tighten the FORTIFY_SOURCE routines on memcpy(). These issues were found with the help of Coccinelle and audited and fixed, manually. [1] https://en.wikipedia.org/wiki/Flexible_array_member [2] https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays Link: https://github.com/KSPP/linux/issues/79 Gustavo A. R. Silva (8): scsi: aacraid: Replace one-element array with flexible-array member scsi: aacraid: Replace one-element array with flexible-array member in struct sgmap scsi: aacraid: Replace one-element array with flexible-array member in struct user_sgmap scsi: aacraid: Replace one-element array with flexible-array member in struct sgmap64 scsi: aacraid: Replace one-element array with flexible-array member in struct user_sgmap64 scsi: aacraid: Replace one-element array with flexible-array member in struct sgmapraw scsi: aacraid: Replace one-element array with flexible-array member in struct user_sgmapraw scsi: aacraid: Replace one-element array with flexible-array member in struct aac_aifcmd drivers/scsi/aacraid/aachba.c | 76 +++++++++++---------------------- drivers/scsi/aacraid/aacraid.h | 16 +++---- drivers/scsi/aacraid/commctrl.c | 5 +-- drivers/scsi/aacraid/comminit.c | 3 +- 4 files changed, 37 insertions(+), 63 deletions(-)