On 8/15/19 12:20 PM, 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;
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);
Good Idea, but using the same argument structure for set and get I would
be concerned about the loss of compiler protection for the buffer
argument; it is void* for get, and const void* for set. And if we pulled
out buffer (and size since it is paired with it) from the structure to
solve, the 'mixed' argument approach (resulting in 4 args) adds to the
difficulty/complexity.
Good news is the same structure(s) can get passed to __vfs_getxattr and
__vfs_setxattr, so one less issue with getting the argument order
correct from the caller.
From an optimization standpoint, passing an argument to a pointer to a
structure assembled on the stack constrains the compiler. Whereas
individual arguments allow for the optimization to place all the
arguments into registers. All modern processors have no issue with tens
of arguments.
So, I will look into what the patch set will look like by splitting into
set and get, and trying to reuse the structure down the call chain.
struct getxattr_args {
struct dentry *dentry;
struct inode *inode;
const char *name;
void *buffer;
size_t size;
int flags;
};
struct setxattr_args {
struct dentry *dentry;
struct inode *inode;
const char *name;
const void *buffer;
size_t size;
int flags;
};
-- Mark
