On Tue 17-02-26 16:47:25, Chuck Lever wrote: > From: Chuck Lever <[email protected]> > > fileattr_fill_xflags() and fileattr_fill_flags() zero the entire > file_kattr struct before populating select fields. This behavior > prevents callers from setting flags in fa->fsx_xflags before > calling these helpers; the zeroing clears any pre-set values. > > As Darrick Wong observed, when a function named "fill_xflags" > modifies more than just xflags, filesystems must understand > implementation details beyond the function's apparent scope. When > initialization occurs at entry points, helper functions need not > duplicate that zeroing. > > Move struct file_kattr zero-initialization from the fill functions > to their callers. Entry points such as ioctl_setflags(), > ioctl_fssetxattr(), and the file_getattr/file_setattr syscalls > now perform aggregate initialization directly. The fill functions > retain their field-setting logic but no longer clear the struct. > > This change enables subsequent patches where filesystem > ->fileattr_get() handlers can set case-sensitivity flags > (FS_XFLAG_CASEFOLD, FS_XFLAG_CASENONPRESERVING) in fa->fsx_xflags > before calling the fill functions. > > Suggested-by: Darrick J. Wong <[email protected]> > Signed-off-by: Chuck Lever <[email protected]>
Looks good. Feel free to add: Reviewed-by: Jan Kara <[email protected]> Honza > --- > fs/file_attr.c | 14 +++++--------- > fs/xfs/xfs_ioctl.c | 2 +- > 2 files changed, 6 insertions(+), 10 deletions(-) > > diff --git a/fs/file_attr.c b/fs/file_attr.c > index 6d2a298a786d..42aa511111a0 100644 > --- a/fs/file_attr.c > +++ b/fs/file_attr.c > @@ -15,12 +15,10 @@ > * @fa: fileattr pointer > * @xflags: FS_XFLAG_* flags > * > - * Set ->fsx_xflags, ->fsx_valid and ->flags (translated xflags). All > - * other fields are zeroed. > + * Set ->fsx_xflags, ->fsx_valid and ->flags (translated xflags). > */ > void fileattr_fill_xflags(struct file_kattr *fa, u32 xflags) > { > - memset(fa, 0, sizeof(*fa)); > fa->fsx_valid = true; > fa->fsx_xflags = xflags; > if (fa->fsx_xflags & FS_XFLAG_IMMUTABLE) > @@ -48,11 +46,9 @@ EXPORT_SYMBOL(fileattr_fill_xflags); > * @flags: FS_*_FL flags > * > * Set ->flags, ->flags_valid and ->fsx_xflags (translated flags). > - * All other fields are zeroed. > */ > void fileattr_fill_flags(struct file_kattr *fa, u32 flags) > { > - memset(fa, 0, sizeof(*fa)); > fa->flags_valid = true; > fa->flags = flags; > if (fa->flags & FS_SYNC_FL) > @@ -325,7 +321,7 @@ int ioctl_setflags(struct file *file, unsigned int __user > *argp) > { > struct mnt_idmap *idmap = file_mnt_idmap(file); > struct dentry *dentry = file->f_path.dentry; > - struct file_kattr fa; > + struct file_kattr fa = {}; > unsigned int flags; > int err; > > @@ -357,7 +353,7 @@ int ioctl_fssetxattr(struct file *file, void __user *argp) > { > struct mnt_idmap *idmap = file_mnt_idmap(file); > struct dentry *dentry = file->f_path.dentry; > - struct file_kattr fa; > + struct file_kattr fa = {}; > int err; > > err = copy_fsxattr_from_user(&fa, argp); > @@ -378,7 +374,7 @@ SYSCALL_DEFINE5(file_getattr, int, dfd, const char __user > *, filename, > struct path filepath __free(path_put) = {}; > unsigned int lookup_flags = 0; > struct file_attr fattr; > - struct file_kattr fa; > + struct file_kattr fa = {}; > int error; > > BUILD_BUG_ON(sizeof(struct file_attr) < FILE_ATTR_SIZE_VER0); > @@ -431,7 +427,7 @@ SYSCALL_DEFINE5(file_setattr, int, dfd, const char __user > *, filename, > struct path filepath __free(path_put) = {}; > unsigned int lookup_flags = 0; > struct file_attr fattr; > - struct file_kattr fa; > + struct file_kattr fa = {}; > int error; > > BUILD_BUG_ON(sizeof(struct file_attr) < FILE_ATTR_SIZE_VER0); > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c > index 4eeda4d4e3ab..369555275140 100644 > --- a/fs/xfs/xfs_ioctl.c > +++ b/fs/xfs/xfs_ioctl.c > @@ -498,7 +498,7 @@ xfs_ioc_fsgetxattra( > xfs_inode_t *ip, > void __user *arg) > { > - struct file_kattr fa; > + struct file_kattr fa = {}; > > xfs_ilock(ip, XFS_ILOCK_SHARED); > xfs_fill_fsxattr(ip, XFS_ATTR_FORK, &fa); > -- > 2.53.0 > -- Jan Kara <[email protected]> SUSE Labs, CR _______________________________________________ Linux-f2fs-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
