On Fri, Aug 16, 2019 at 05:20:36AM +1000, James Morris wrote:
> On Tue, 13 Aug 2019, Greg Kroah-Hartman wrote:
>
> > On Mon, Aug 12, 2019 at 12:32:49PM -0700, Mark Salyzyn wrote:
> > > --- a/include/linux/xattr.h
> > > +++ b/include/linux/xattr.h
> > > @@ -30,10 +30,10 @@ struct xattr_handler {
> > > const char *prefix;
> > > int flags; /* fs private flags */
> > > bool (*list)(struct dentry *dentry);
> > > - int (*get)(const struct xattr_handler *, struct dentry *dentry,
> > > + int (*get)(const struct xattr_handler *handler, struct dentry *dentry,
> > > struct inode *inode, const char *name, void *buffer,
> > > - size_t size);
> > > - int (*set)(const struct xattr_handler *, struct dentry *dentry,
> > > + size_t size, int flags);
> > > + int (*set)(const struct xattr_handler *handler, struct dentry *dentry,
> > > struct inode *inode, const char *name, const void *buffer,
> > > size_t size, int flags);
> >
> > Wow, 7 arguments. Isn't there some nice rule of thumb that says once
> > you get more then 5, a function becomes impossible to understand?
> >
> > Surely this could be a structure passed in here somehow, that way when
> > you add the 8th argument in the future, you don't have to change
> > everything yet again? :)
> >
> > I don't have anything concrete to offer as a replacement fix for this,
> > but to me this just feels really wrong...
>
> How about something like:
>
> struct xattr_gs_args {
> struct dentry *dentry;
> struct inode *inode;
As he said in a later message, dentry and inode is redundant, only 1 is
needed (dentry I think?)
> const char *name;
> const void *buffer;
> size_t size;
> int flags;
> };
>
> int (*get)(const struct xattr_handler *handler, struct xattr_gs_args *args);
> int (*set)(const struct xattr_handler *handler, struct xattr_gs_args *args);
But yes, that would be much much better.
thanks,
greg k-h
