On Oct 22, 2019, at 2:44 PM, Mark Salyzyn <salyzyn@xxxxxxxxxxx> wrote: > > Replace arguments for get and set xattr methods, and __vfs_getxattr > and __vfs_setaxtr functions with a reference to the following now > common argument structure: > > struct xattr_gs_args { > struct dentry *dentry; > struct inode *inode; > const char *name; > union { > void *buffer; > const void *value; > }; > size_t size; > int flags; > }; As part of this change (which is touching all of the uses of these fields anyway) it would be useful to give these structure fields a prefix like "xga_" so that they can be easily found with tags. Otherwise, there are so many different "dentry" and "inode" fields in various structures that it is hard to find the right one. > #define __USE_KERNEL_XATTR_DEFS > > -#define XATTR_CREATE 0x1 /* set value, fail if attr already exists */ > -#define XATTR_REPLACE 0x2 /* set value, fail if attr does not exist */ > +#define XATTR_CREATE 0x1 /* set value, fail if attr already exists */ > +#define XATTR_REPLACE 0x2 /* set value, fail if attr does not exist */ > +#ifdef __KERNEL__ /* following is kernel internal, colocated for maintenance */ > +#define XATTR_NOSECURITY 0x4 /* get value, do not involve security check */ > +#endif Now that these arguments are separated out into their own structure, rather than using "int flags" (there are a million different flags in the kernel and easily confused) it would be immediately clear *which* flags are used here with a named enum, like: enum xattr_flags { XATTR_CREATE = 0x1, /* set value, fail if attr already exists */ XATTR_REPLACE = 0x2, /* set value, fail if attr does not exist */ #ifdef __KERNEL__ /* following is kernel internal, colocated for maintenance */ XATTR_NOSECURITY= 0x4, /* get value, do not involve security check */ #endif }; and use this in the struct like: struct xattr_gs_args { struct dentry *xga_dentry; struct inode *xga_inode; const char *xga_name; union { void *xga_buffer; const void *xga_value; }; size_t xga_size; enum xattr_flags xga_flags; }; Beyond the benefit for the reader to understand the code better, this can also allow the compiler to warn if incorrect values are being assigned to this field. Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP