* [PATCH 1/4] xfs: only return -errno or success from attr ->put_listent
2016-03-11 22:10 [PATCH 0/4] xfs: rework the attr ->put_listent code a bit Eric Sandeen
@ 2016-03-11 22:11 ` Eric Sandeen
2016-03-15 7:55 ` Christoph Hellwig
2016-03-11 22:11 ` [PATCH 2/4] xfs: don't pass value into " Eric Sandeen
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Eric Sandeen @ 2016-03-11 22:11 UTC (permalink / raw)
To: xfs
Today, the put_listent formatters return either 1 or 0; if
they return 1, some callers treat this as an error and return
it up the stack, despite "1" not being a valid (negative)
error code.
The intent seems to be that if the input buffer is full,
we set seen_enough or set count = -1, and return 1;
but some callers check the return before checking the
seen_enough or count fields of the context.
Fix this by only returning non-zero for actual errors
encountered, and rely on the caller to first check the
return value, then check the values in the context to
decide what to do.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
fs/xfs/xfs_attr.h | 1 +
fs/xfs/xfs_attr_list.c | 8 +++-----
fs/xfs/xfs_xattr.c | 14 ++++++++++----
3 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/fs/xfs/xfs_attr.h b/fs/xfs/xfs_attr.h
index dd48245..2343312 100644
--- a/fs/xfs/xfs_attr.h
+++ b/fs/xfs/xfs_attr.h
@@ -112,6 +112,7 @@ typedef struct attrlist_cursor_kern {
*========================================================================*/
+/* Return 0 on success, or -errno; other state communicated via *context */
typedef int (*put_listent_func_t)(struct xfs_attr_list_context *, int,
unsigned char *, int, int, unsigned char *);
diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
index 0ef7c2e..d5ab59f 100644
--- a/fs/xfs/xfs_attr_list.c
+++ b/fs/xfs/xfs_attr_list.c
@@ -108,16 +108,14 @@ xfs_attr_shortform_list(xfs_attr_list_context_t *context)
(int)sfe->namelen,
(int)sfe->valuelen,
&sfe->nameval[sfe->namelen]);
-
+ if (error)
+ return error;
/*
* Either search callback finished early or
* didn't fit it all in the buffer after all.
*/
if (context->seen_enough)
break;
-
- if (error)
- return error;
sfe = XFS_ATTR_SF_NEXTENTRY(sfe);
}
trace_xfs_attr_list_sf_all(context);
@@ -580,7 +578,7 @@ xfs_attr_put_listent(
trace_xfs_attr_list_full(context);
alist->al_more = 1;
context->seen_enough = 1;
- return 1;
+ return 0;
}
aep = (attrlist_ent_t *)&context->alist[context->firstu];
diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c
index 110f1d7..f220129 100644
--- a/fs/xfs/xfs_xattr.c
+++ b/fs/xfs/xfs_xattr.c
@@ -146,7 +146,7 @@ __xfs_xattr_put_listent(
arraytop = context->count + prefix_len + namelen + 1;
if (arraytop > context->firstu) {
context->count = -1; /* insufficient space */
- return 1;
+ return 0;
}
offset = (char *)context->alist + context->count;
strncpy(offset, prefix, prefix_len);
@@ -221,11 +221,15 @@ xfs_xattr_put_listent(
}
ssize_t
-xfs_vn_listxattr(struct dentry *dentry, char *data, size_t size)
+xfs_vn_listxattr(
+ struct dentry *dentry,
+ char *data,
+ size_t size)
{
struct xfs_attr_list_context context;
struct attrlist_cursor_kern cursor = { 0 };
- struct inode *inode = d_inode(dentry);
+ struct inode *inode = d_inode(dentry);
+ int error;
/*
* First read the regular on-disk attributes.
@@ -239,7 +243,9 @@ xfs_vn_listxattr(struct dentry *dentry, char *data, size_t size)
context.firstu = context.bufsize;
context.put_listent = xfs_xattr_put_listent;
- xfs_attr_list_int(&context);
+ error = xfs_attr_list_int(&context);
+ if (error)
+ return error;
if (context.count < 0)
return -ERANGE;
--
1.7.1
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/4] xfs: don't pass value into attr ->put_listent
2016-03-11 22:10 [PATCH 0/4] xfs: rework the attr ->put_listent code a bit Eric Sandeen
2016-03-11 22:11 ` [PATCH 1/4] xfs: only return -errno or success from attr ->put_listent Eric Sandeen
@ 2016-03-11 22:11 ` Eric Sandeen
2016-03-15 7:55 ` Christoph Hellwig
2016-03-11 22:12 ` [PATCH 3/4] xfs: remove put_value from attr ->put_listent context Eric Sandeen
2016-03-11 22:12 ` [PATCH 4/4] xfs: collapse cases in xfs_attr3_leaf_list_int Eric Sandeen
3 siblings, 1 reply; 9+ messages in thread
From: Eric Sandeen @ 2016-03-11 22:11 UTC (permalink / raw)
To: xfs
The value is not used; only names and value lengths are
returned. Remove the argument.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
fs/xfs/xfs_attr.h | 2 +-
fs/xfs/xfs_attr_list.c | 18 ++++++------------
fs/xfs/xfs_xattr.c | 3 +--
3 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/fs/xfs/xfs_attr.h b/fs/xfs/xfs_attr.h
index 2343312..dab4f41 100644
--- a/fs/xfs/xfs_attr.h
+++ b/fs/xfs/xfs_attr.h
@@ -114,7 +114,7 @@ typedef struct attrlist_cursor_kern {
/* Return 0 on success, or -errno; other state communicated via *context */
typedef int (*put_listent_func_t)(struct xfs_attr_list_context *, int,
- unsigned char *, int, int, unsigned char *);
+ unsigned char *, int, int);
typedef struct xfs_attr_list_context {
struct xfs_inode *dp; /* inode */
diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
index d5ab59f..1d36d78 100644
--- a/fs/xfs/xfs_attr_list.c
+++ b/fs/xfs/xfs_attr_list.c
@@ -106,8 +106,7 @@ xfs_attr_shortform_list(xfs_attr_list_context_t *context)
sfe->flags,
sfe->nameval,
(int)sfe->namelen,
- (int)sfe->valuelen,
- &sfe->nameval[sfe->namelen]);
+ (int)sfe->valuelen);
if (error)
return error;
/*
@@ -198,8 +197,7 @@ xfs_attr_shortform_list(xfs_attr_list_context_t *context)
sbp->flags,
sbp->name,
sbp->namelen,
- sbp->valuelen,
- &sbp->name[sbp->namelen]);
+ sbp->valuelen);
if (error)
return error;
if (context->seen_enough)
@@ -428,8 +426,7 @@ xfs_attr3_leaf_list_int(
entry->flags,
name_loc->nameval,
(int)name_loc->namelen,
- be16_to_cpu(name_loc->valuelen),
- &name_loc->nameval[name_loc->namelen]);
+ be16_to_cpu(name_loc->valuelen));
if (retval)
return retval;
} else {
@@ -458,16 +455,14 @@ xfs_attr3_leaf_list_int(
entry->flags,
name_rmt->name,
(int)name_rmt->namelen,
- valuelen,
- args.value);
+ valuelen);
kmem_free(args.value);
} else {
retval = context->put_listent(context,
entry->flags,
name_rmt->name,
(int)name_rmt->namelen,
- valuelen,
- NULL);
+ valuelen);
}
if (retval)
return retval;
@@ -548,8 +543,7 @@ xfs_attr_put_listent(
int flags,
unsigned char *name,
int namelen,
- int valuelen,
- unsigned char *value)
+ int valuelen)
{
struct attrlist *alist = (struct attrlist *)context->alist;
attrlist_ent_t *aep;
diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c
index f220129..7fdcf33 100644
--- a/fs/xfs/xfs_xattr.c
+++ b/fs/xfs/xfs_xattr.c
@@ -166,8 +166,7 @@ xfs_xattr_put_listent(
int flags,
unsigned char *name,
int namelen,
- int valuelen,
- unsigned char *value)
+ int valuelen)
{
char *prefix;
int prefix_len;
--
1.7.1
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/4] xfs: remove put_value from attr ->put_listent context
2016-03-11 22:10 [PATCH 0/4] xfs: rework the attr ->put_listent code a bit Eric Sandeen
2016-03-11 22:11 ` [PATCH 1/4] xfs: only return -errno or success from attr ->put_listent Eric Sandeen
2016-03-11 22:11 ` [PATCH 2/4] xfs: don't pass value into " Eric Sandeen
@ 2016-03-11 22:12 ` Eric Sandeen
2016-03-15 7:56 ` Christoph Hellwig
2016-03-11 22:12 ` [PATCH 4/4] xfs: collapse cases in xfs_attr3_leaf_list_int Eric Sandeen
3 siblings, 1 reply; 9+ messages in thread
From: Eric Sandeen @ 2016-03-11 22:12 UTC (permalink / raw)
To: xfs
The put_value context member is never set; remove it
and the conditional test in xfs_attr3_leaf_list_int().
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
fs/xfs/xfs_attr.h | 1 -
fs/xfs/xfs_attr_list.c | 26 +-------------------------
2 files changed, 1 insertions(+), 26 deletions(-)
diff --git a/fs/xfs/xfs_attr.h b/fs/xfs/xfs_attr.h
index dab4f41..e3da5d4 100644
--- a/fs/xfs/xfs_attr.h
+++ b/fs/xfs/xfs_attr.h
@@ -127,7 +127,6 @@ typedef struct xfs_attr_list_context {
int firstu; /* first used byte in buffer */
int flags; /* from VOP call */
int resynch; /* T/F: resynch with cursor */
- int put_value; /* T/F: need value for listent */
put_listent_func_t put_listent; /* list output fmt function */
int index; /* index into output buffer */
} xfs_attr_list_context_t;
diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
index 1d36d78..900164c 100644
--- a/fs/xfs/xfs_attr_list.c
+++ b/fs/xfs/xfs_attr_list.c
@@ -435,35 +435,11 @@ xfs_attr3_leaf_list_int(
int valuelen = be32_to_cpu(name_rmt->valuelen);
- if (context->put_value) {
- xfs_da_args_t args;
-
- memset((char *)&args, 0, sizeof(args));
- args.geo = context->dp->i_mount->m_attr_geo;
- args.dp = context->dp;
- args.whichfork = XFS_ATTR_FORK;
- args.valuelen = valuelen;
- args.rmtvaluelen = valuelen;
- args.value = kmem_alloc(valuelen, KM_SLEEP | KM_NOFS);
- args.rmtblkno = be32_to_cpu(name_rmt->valueblk);
- args.rmtblkcnt = xfs_attr3_rmt_blocks(
- args.dp->i_mount, valuelen);
- retval = xfs_attr_rmtval_get(&args);
- if (retval)
- return retval;
- retval = context->put_listent(context,
- entry->flags,
- name_rmt->name,
- (int)name_rmt->namelen,
- valuelen);
- kmem_free(args.value);
- } else {
- retval = context->put_listent(context,
+ retval = context->put_listent(context,
entry->flags,
name_rmt->name,
(int)name_rmt->namelen,
valuelen);
- }
if (retval)
return retval;
}
--
1.7.1
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 4/4] xfs: collapse cases in xfs_attr3_leaf_list_int
2016-03-11 22:10 [PATCH 0/4] xfs: rework the attr ->put_listent code a bit Eric Sandeen
` (2 preceding siblings ...)
2016-03-11 22:12 ` [PATCH 3/4] xfs: remove put_value from attr ->put_listent context Eric Sandeen
@ 2016-03-11 22:12 ` Eric Sandeen
2016-03-15 7:57 ` Christoph Hellwig
3 siblings, 1 reply; 9+ messages in thread
From: Eric Sandeen @ 2016-03-11 22:12 UTC (permalink / raw)
To: xfs
Consolidate the 2 calls to ->put_listent in
xfs_attr3_leaf_list_int(), by setting up name, namelen, and
valuelen for the local vs remote cases, then call ->put_listent
and do the error handling all in one spot.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
fs/xfs/xfs_attr_list.c | 42 ++++++++++++++++++++----------------------
1 files changed, 20 insertions(+), 22 deletions(-)
diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
index 900164c..007be5c 100644
--- a/fs/xfs/xfs_attr_list.c
+++ b/fs/xfs/xfs_attr_list.c
@@ -410,6 +410,9 @@ xfs_attr3_leaf_list_int(
*/
retval = 0;
for (; i < ichdr.count; entry++, i++) {
+ char *name;
+ int namelen, valuelen;
+
if (be32_to_cpu(entry->hashval) != cursor->hashval) {
cursor->hashval = be32_to_cpu(entry->hashval);
cursor->offset = 0;
@@ -419,30 +422,25 @@ xfs_attr3_leaf_list_int(
continue; /* skip incomplete entries */
if (entry->flags & XFS_ATTR_LOCAL) {
- xfs_attr_leaf_name_local_t *name_loc =
- xfs_attr3_leaf_name_local(leaf, i);
-
- retval = context->put_listent(context,
- entry->flags,
- name_loc->nameval,
- (int)name_loc->namelen,
- be16_to_cpu(name_loc->valuelen));
- if (retval)
- return retval;
+ xfs_attr_leaf_name_local_t *name_loc;
+
+ name_loc = xfs_attr3_leaf_name_local(leaf, i);
+ name = name_loc->nameval;
+ namelen = name_loc->namelen;
+ valuelen = be16_to_cpu(name_loc->valuelen);
} else {
- xfs_attr_leaf_name_remote_t *name_rmt =
- xfs_attr3_leaf_name_remote(leaf, i);
-
- int valuelen = be32_to_cpu(name_rmt->valuelen);
-
- retval = context->put_listent(context,
- entry->flags,
- name_rmt->name,
- (int)name_rmt->namelen,
- valuelen);
- if (retval)
- return retval;
+ xfs_attr_leaf_name_remote_t *name_rmt;
+
+ name_rmt = xfs_attr3_leaf_name_remote(leaf, i);
+ name = name_rmt->name;
+ namelen = name_rmt->namelen;
+ valuelen = be32_to_cpu(name_rmt->valuelen);
}
+
+ retval = context->put_listent(context, entry->flags,
+ name, namelen, valuelen);
+ if (retval)
+ return retval;
if (context->seen_enough)
break;
cursor->offset++;
--
1.7.1
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 9+ messages in thread