| Message ID | 20240817142022.68411-2-thorsten.blum@toblux.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | NFS/flexfiles: Replace one-element array with flexible-array member | expand |
On Sat, 2024-08-17 at 16:20 +0200, Thorsten Blum wrote: > Replace the deprecated one-element array with a modern flexible-array > member in the struct nfs4_flexfile_layoutreturn_args. > > Adjust the struct size accordingly. > > There are no binary differences after this conversion. > > Link: https://github.com/KSPP/linux/issues/79 > Signed-off-by: Thorsten Blum <thorsten.blum@toblux.com> > --- > fs/nfs/flexfilelayout/flexfilelayout.c | 2 +- > fs/nfs/flexfilelayout/flexfilelayout.h | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c > b/fs/nfs/flexfilelayout/flexfilelayout.c > index 39ba9f4208aa..fc698fa9aaea 100644 > --- a/fs/nfs/flexfilelayout/flexfilelayout.c > +++ b/fs/nfs/flexfilelayout/flexfilelayout.c > @@ -2224,7 +2224,7 @@ ff_layout_prepare_layoutreturn(struct > nfs4_layoutreturn_args *args) > struct nfs4_flexfile_layoutreturn_args *ff_args; > struct nfs4_flexfile_layout *ff_layout = > FF_LAYOUT_FROM_HDR(args->layout); > > - ff_args = kmalloc(sizeof(*ff_args), nfs_io_gfp_mask()); > + ff_args = kmalloc(struct_size(ff_args, pages, 1), > nfs_io_gfp_mask()); > if (!ff_args) > goto out_nomem; > ff_args->pages[0] = alloc_page(nfs_io_gfp_mask()); > diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h > b/fs/nfs/flexfilelayout/flexfilelayout.h > index f84b3fb0dddd..a269753f9a46 100644 > --- a/fs/nfs/flexfilelayout/flexfilelayout.h > +++ b/fs/nfs/flexfilelayout/flexfilelayout.h > @@ -115,7 +115,7 @@ struct nfs4_flexfile_layoutreturn_args { > struct nfs42_layoutstat_devinfo > devinfo[FF_LAYOUTSTATS_MAXDEV]; > unsigned int num_errors; > unsigned int num_dev; > - struct page *pages[1]; > + struct page *pages[]; > }; > > static inline struct nfs4_flexfile_layout * NACK. There is no advantage to using a flexible array here. Indeed, you're replacing something that is correctly dimensioned (we only ever use 1 page), and that can be easily bounds checked by the compiler with something that has neither property.
On 17. Aug 2024, at 22:40, Trond Myklebust <trondmy@hammerspace.com> wrote: > On Sat, 2024-08-17 at 16:20 +0200, Thorsten Blum wrote: >> Replace the deprecated one-element array with a modern flexible-array >> member in the struct nfs4_flexfile_layoutreturn_args. >> >> Adjust the struct size accordingly. >> >> There are no binary differences after this conversion. >> >> Link: https://github.com/KSPP/linux/issues/79 >> Signed-off-by: Thorsten Blum <thorsten.blum@toblux.com> >> --- >> fs/nfs/flexfilelayout/flexfilelayout.c | 2 +- >> fs/nfs/flexfilelayout/flexfilelayout.h | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c >> b/fs/nfs/flexfilelayout/flexfilelayout.c >> index 39ba9f4208aa..fc698fa9aaea 100644 >> --- a/fs/nfs/flexfilelayout/flexfilelayout.c >> +++ b/fs/nfs/flexfilelayout/flexfilelayout.c >> @@ -2224,7 +2224,7 @@ ff_layout_prepare_layoutreturn(struct >> nfs4_layoutreturn_args *args) >> struct nfs4_flexfile_layoutreturn_args *ff_args; >> struct nfs4_flexfile_layout *ff_layout = >> FF_LAYOUT_FROM_HDR(args->layout); >> >> - ff_args = kmalloc(sizeof(*ff_args), nfs_io_gfp_mask()); >> + ff_args = kmalloc(struct_size(ff_args, pages, 1), >> nfs_io_gfp_mask()); >> if (!ff_args) >> goto out_nomem; >> ff_args->pages[0] = alloc_page(nfs_io_gfp_mask()); >> diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h >> b/fs/nfs/flexfilelayout/flexfilelayout.h >> index f84b3fb0dddd..a269753f9a46 100644 >> --- a/fs/nfs/flexfilelayout/flexfilelayout.h >> +++ b/fs/nfs/flexfilelayout/flexfilelayout.h >> @@ -115,7 +115,7 @@ struct nfs4_flexfile_layoutreturn_args { >> struct nfs42_layoutstat_devinfo >> devinfo[FF_LAYOUTSTATS_MAXDEV]; >> unsigned int num_errors; >> unsigned int num_dev; >> - struct page *pages[1]; >> + struct page *pages[]; >> }; >> >> static inline struct nfs4_flexfile_layout * > > NACK. There is no advantage to using a flexible array here. Indeed, > you're replacing something that is correctly dimensioned (we only ever > use 1 page), and that can be easily bounds checked by the compiler with > something that has neither property. I see. Why is it an array then and not just a pointer to a struct? Thanks, Thorsten
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c index 39ba9f4208aa..fc698fa9aaea 100644 --- a/fs/nfs/flexfilelayout/flexfilelayout.c +++ b/fs/nfs/flexfilelayout/flexfilelayout.c @@ -2224,7 +2224,7 @@ ff_layout_prepare_layoutreturn(struct nfs4_layoutreturn_args *args) struct nfs4_flexfile_layoutreturn_args *ff_args; struct nfs4_flexfile_layout *ff_layout = FF_LAYOUT_FROM_HDR(args->layout); - ff_args = kmalloc(sizeof(*ff_args), nfs_io_gfp_mask()); + ff_args = kmalloc(struct_size(ff_args, pages, 1), nfs_io_gfp_mask()); if (!ff_args) goto out_nomem; ff_args->pages[0] = alloc_page(nfs_io_gfp_mask()); diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h b/fs/nfs/flexfilelayout/flexfilelayout.h index f84b3fb0dddd..a269753f9a46 100644 --- a/fs/nfs/flexfilelayout/flexfilelayout.h +++ b/fs/nfs/flexfilelayout/flexfilelayout.h @@ -115,7 +115,7 @@ struct nfs4_flexfile_layoutreturn_args { struct nfs42_layoutstat_devinfo devinfo[FF_LAYOUTSTATS_MAXDEV]; unsigned int num_errors; unsigned int num_dev; - struct page *pages[1]; + struct page *pages[]; }; static inline struct nfs4_flexfile_layout *
Replace the deprecated one-element array with a modern flexible-array member in the struct nfs4_flexfile_layoutreturn_args. Adjust the struct size accordingly. There are no binary differences after this conversion. Link: https://github.com/KSPP/linux/issues/79 Signed-off-by: Thorsten Blum <thorsten.blum@toblux.com> --- fs/nfs/flexfilelayout/flexfilelayout.c | 2 +- fs/nfs/flexfilelayout/flexfilelayout.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)