xfs: rework the attr ->put_listent code a bit
cover xfs: rework the attr ->put_listent code a bit
Lots of cruft in there, maybe more to do but here's the obvious stuff; removing bad error returns, unused variables and structure members, and then consolidating a little code.
xfs_attr.h | 4 +- xfs_attr_list.c | 86 ++++++++++++++++---------------------------------------- xfs_xattr.c | 17 +++++++---- 3 files changed, 39 insertions(+), 68 deletions(-)
-Eric
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
PATCH 1/4 xfs: only return -errno or success from attr ->put_listent
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.
--- 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(-)
fs/xfs/xfs_attr.h +1 -0
@@ -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 *);
fs/xfs/xfs_attr_list.c +3 -5
@@ -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];
fs/xfs/xfs_xattr.c +10 -4
@@ -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 ;
Signature · 6 lines
1.7.1 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 1/4] xfs: only return -errno or success from attr ->put_listent
Looks fine,
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
PATCH 2/4 xfs: don't pass value into attr ->put_listent
In reply to Eric SandeenThe value is not used; only names and value lengths are returned. Remove the argument.
--- 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(-)
fs/xfs/xfs_attr.h +1 -1
@@ -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 */
fs/xfs/xfs_attr_list.c +6 -12
@@ -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;
fs/xfs/xfs_xattr.c +1 -2
@@ -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;
Signature · 6 lines
1.7.1 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 2/4] xfs: don't pass value into attr ->put_listent
Looks fine,
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
PATCH 3/4 xfs: remove put_value from attr ->put_listent context
In reply to Eric SandeenThe put_value context member is never set; remove it and the conditional test in xfs_attr3_leaf_list_int().
--- fs/xfs/xfs_attr.h | 1 - fs/xfs/xfs_attr_list.c | 26 +------------------------- 2 files changed, 1 insertions(+), 26 deletions(-)
fs/xfs/xfs_attr.h +0 -1
@@ -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;
fs/xfs/xfs_attr_list.c +1 -25
@@ -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; }
Signature · 6 lines
1.7.1 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 3/4] xfs: remove put_value from attr ->put_listent context
Quoted 2 lines · Eric Sandeen
The
put_valuecontext member is never set; remove it and the conditional test inxfs_attr3_leaf_list_int().
Looks fine,
(although this should probably go along with the previous patch)
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
PATCH 4/4 xfs: collapse cases in xfs_attr3_leaf_list_int
In reply to Eric SandeenConsolidate 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.
--- fs/xfs/xfs_attr_list.c | 42 ++++++++++++++++++++---------------------- 1 files changed, 20 insertions(+), 22 deletions(-)
fs/xfs/xfs_attr_list.c +20 -22
@@ -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++;
Signature · 6 lines
1.7.1 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 4/4] xfs: collapse cases in xfs_attr3_leaf_list_int
Looks fine,
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs