Re: [PATCH 10/10] xattr handlers: Simplify list operation

2015-12-01 Thread Andreas Grünbacher
2015-12-01 10:53 GMT+01:00 James Morris :
> On Mon, 30 Nov 2015, Andreas Gruenbacher wrote:
>
>> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
>> index 906022d..61dd93f 100644
>> --- a/fs/ocfs2/xattr.c
>> +++ b/fs/ocfs2/xattr.c
>> @@ -895,6 +895,8 @@ static int ocfs2_xattr_list_entry(char *buffer, size_t 
>> size,
>>
>>   *result += total_len;
>>
>> + /* FIXME: Not checking the ->list operation here ... */
>> +
>
> What does this mean?

ocfs2 defines list xattr handler operations for "user.*" and
"trusted.*" xattrs but never bothers calling them. Should be fixed
separately.

Thanks,
Andreas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 10/10] xattr handlers: Simplify list operation

2015-12-01 Thread James Morris
On Mon, 30 Nov 2015, Andreas Gruenbacher wrote:

> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
> index 906022d..61dd93f 100644
> --- a/fs/ocfs2/xattr.c
> +++ b/fs/ocfs2/xattr.c
> @@ -895,6 +895,8 @@ static int ocfs2_xattr_list_entry(char *buffer, size_t 
> size,
>  
>   *result += total_len;
>  
> + /* FIXME: Not checking the ->list operation here ... */
> +

What does this mean? 

-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 10/10] xattr handlers: Simplify list operation

2015-11-30 Thread Andreas Gruenbacher
Change the list operation to only return whether or not an attribute
should be listed.  Copying the attribute names into the buffer is moved
to the callers.

Since the result only depends on the dentry and not on the attribute
name, we do not pass the attribute name to list operations.

Signed-off-by: Andreas Gruenbacher 
---
 fs/ext2/xattr.c  | 15 +---
 fs/ext2/xattr_security.c | 17 --
 fs/ext2/xattr_trusted.c  | 19 ++-
 fs/ext2/xattr_user.c | 19 ++-
 fs/ext4/xattr.c  | 17 ++
 fs/ext4/xattr_security.c | 18 --
 fs/ext4/xattr_trusted.c  | 19 ++-
 fs/ext4/xattr_user.c | 19 ++-
 fs/f2fs/xattr.c  | 81 ++--
 fs/jffs2/security.c  | 16 -
 fs/jffs2/xattr.c | 26 --
 fs/jffs2/xattr_trusted.c | 17 ++
 fs/jffs2/xattr_user.c| 16 -
 fs/nfs/nfs4proc.c| 14 ++--
 fs/ocfs2/xattr.c | 54 -
 fs/posix_acl.c   | 17 ++
 fs/reiserfs/xattr.c  | 13 +++
 fs/reiserfs/xattr_security.c | 16 ++---
 fs/reiserfs/xattr_trusted.c  | 15 ++--
 fs/reiserfs/xattr_user.c | 14 ++--
 fs/squashfs/xattr.c  | 35 +--
 fs/xattr.c   | 18 ++
 include/linux/xattr.h|  4 +--
 23 files changed, 119 insertions(+), 380 deletions(-)

diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index fa70848..cd95d14 100644
--- a/fs/ext2/xattr.c
+++ b/fs/ext2/xattr.c
@@ -292,16 +292,21 @@ bad_block:ext2_error(inode->i_sb, 
"ext2_xattr_list",
const struct xattr_handler *handler =
ext2_xattr_handler(entry->e_name_index);
 
-   if (handler) {
-   size_t size = handler->list(handler, dentry, buffer,
-   rest, entry->e_name,
-   entry->e_name_len);
+   if (handler && (!handler->list || handler->list(dentry))) {
+   const char *prefix = handler->prefix ?: handler->name;
+   size_t prefix_len = strlen(prefix);
+   size_t size = prefix_len + entry->e_name_len + 1;
+
if (buffer) {
if (size > rest) {
error = -ERANGE;
goto cleanup;
}
-   buffer += size;
+   memcpy(buffer, prefix, prefix_len);
+   buffer += prefix_len;
+   memcpy(buffer, entry->e_name, 
entry->e_name_len);
+   buffer += entry->e_name_len;
+   *buffer++ = 0;
}
rest -= size;
}
diff --git a/fs/ext2/xattr_security.c b/fs/ext2/xattr_security.c
index 118bf23..ba97f24 100644
--- a/fs/ext2/xattr_security.c
+++ b/fs/ext2/xattr_security.c
@@ -7,22 +7,6 @@
 #include 
 #include "xattr.h"
 
-static size_t
-ext2_xattr_security_list(const struct xattr_handler *handler,
-struct dentry *dentry, char *list, size_t list_size,
-const char *name, size_t name_len)
-{
-   const int prefix_len = XATTR_SECURITY_PREFIX_LEN;
-   const size_t total_len = prefix_len + name_len + 1;
-
-   if (list && total_len <= list_size) {
-   memcpy(list, XATTR_SECURITY_PREFIX, prefix_len);
-   memcpy(list+prefix_len, name, name_len);
-   list[prefix_len + name_len] = '\0';
-   }
-   return total_len;
-}
-
 static int
 ext2_xattr_security_get(const struct xattr_handler *handler,
struct dentry *dentry, const char *name,
@@ -67,7 +51,6 @@ ext2_init_security(struct inode *inode, struct inode *dir,
 
 const struct xattr_handler ext2_xattr_security_handler = {
.prefix = XATTR_SECURITY_PREFIX,
-   .list   = ext2_xattr_security_list,
.get= ext2_xattr_security_get,
.set= ext2_xattr_security_set,
 };
diff --git a/fs/ext2/xattr_trusted.c b/fs/ext2/xattr_trusted.c
index 3f8f2bc..2c94d19 100644
--- a/fs/ext2/xattr_trusted.c
+++ b/fs/ext2/xattr_trusted.c
@@ -8,23 +8,10 @@
 #include "ext2.h"
 #include "xattr.h"
 
-static size_t
-ext2_xattr_trusted_list(const struct xattr_handler *handler,
-   struct dentry *dentry, char *list, size_t list_size,
-   const char *name, size_t name_len)
+static bool
+ext2_xattr_trusted_list(struct dentry *dentry)
 {
-   const int prefix_len = XATTR_TRUSTED_PREFIX_LEN;
-   const size_t total_len = prefix_len + name_len + 1;
-
-   if (!capable(CAP_SYS_ADMIN))
-   return 0;
-
-   if (list && total_le