From mboxrd@z Thu Jan 1 00:00:00 1970 From: Greg Kroah-Hartman Subject: Re: [PATCH] Add flags option to get xattr method paired to __vfs_getxattr Date: Thu, 15 Aug 2019 22:43:22 +0200 Message-ID: <20190815204322.GB6782@kroah.com> References: <20190812193320.200472-1-salyzyn@android.com> <20190813084801.GA972@kroah.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linux-f2fs-devel-bounces@lists.sourceforge.net To: James Morris Cc: Latchesar Ionkov , Dave Kleikamp , jfs-discussion@lists.sourceforge.net, linux-integrity@vger.kernel.org, Martin Brandenburg , samba-technical@lists.samba.org, Dominique Martinet , Mimi Zohar , linux-unionfs@vger.kernel.org, David Howells , Chris Mason , "David S. Miller" , Andreas Dilger , Eric Paris , netdev@vger.kernel.org, Tyler Hicks , linux-afs@lists.infradead.org, Mike Marshall , linux-xfs@vger.kernel.org, Andreas Gruenbacher , Sage Weil , Miklos Szeredi , Richard Weinberger , Mark Fasheh , Hugh Dickins , Ernesto 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