xfs
[Top] [All Lists]

[REVIEW] - cleanup xfs_attr a bit

To: "xfs@xxxxxxxxxxx" <xfs@xxxxxxxxxxx>, xfs-dev <xfs-dev@xxxxxxx>
Subject: [REVIEW] - cleanup xfs_attr a bit
From: "Barry Naujok" <bnaujok@xxxxxxx>
Date: Thu, 17 Apr 2008 12:35:36 +1000
Organization: SGI
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Opera Mail/9.24 (Win32)
This patch starts using struct xfs_name more for the xattr code and
is another step for using xfs_name in xfs_da_args.

Also, the cred parameter is removed from xfs_attr_get and xfs_attr_fetch.

The new xfs_attr_name_to_name will be a place holder for the Unicode
CI checks/NLS conversion too.

---
 fs/xfs/dmapi/xfs_dm.c        |   13 ++---
 fs/xfs/linux-2.6/xfs_ioctl.c |    6 +-
 fs/xfs/xfs_acl.c             |    7 +-
fs/xfs/xfs_attr.c | 103 +++++++++++++++++++++++--------------------
 fs/xfs/xfs_attr.h            |    6 --
 fs/xfs/xfs_vnodeops.h        |    2
 6 files changed, 71 insertions(+), 66 deletions(-)

Index: kern_ci/fs/xfs/dmapi/xfs_dm.c
===================================================================
--- kern_ci.orig/fs/xfs/dmapi/xfs_dm.c
+++ kern_ci/fs/xfs/dmapi/xfs_dm.c
@@ -516,8 +516,7 @@ xfs_dm_bulkall_iget_one(
        xfs_iunlock(ip, XFS_ILOCK_SHARED);

        memset(&xbuf->dx_attrdata, 0, sizeof(dm_vardata_t));
-       error = xfs_attr_get(ip, attr_name, attr_buf,
-                                &value_len, ATTR_ROOT, sys_cred);
+       error = xfs_attr_get(ip, attr_name, attr_buf, &value_len, ATTR_ROOT);
        iput(ip->i_vnode);

        DM_EA_XLATE_ERR(error);
@@ -1691,8 +1690,8 @@ xfs_dm_get_destroy_dmattr(
        if (value == NULL)
                return(-ENOMEM);

- error = xfs_attr_get(XFS_I(inode), dkattrname.dan_chars, value, &value_len,
-                                       ATTR_ROOT, sys_cred);
+       error = xfs_attr_get(XFS_I(inode), dkattrname.dan_chars, value,
+                                                       &value_len, ATTR_ROOT);
        if (error == ERANGE) {
                kfree(value);
                alloc_size = value_len;
@@ -1701,7 +1700,7 @@ xfs_dm_get_destroy_dmattr(
                        return(-ENOMEM);

                error = xfs_attr_get(XFS_I(inode), dkattrname.dan_chars, value,
-                                       &value_len, ATTR_ROOT, sys_cred);
+                                       &value_len, ATTR_ROOT);
        }
        if (error) {
                kfree(value);
@@ -1970,7 +1969,7 @@ xfs_dm_get_dmattr(
        value_len = alloc_size;         /* in/out parameter */

        error = xfs_attr_get(XFS_I(inode), name.dan_chars, value, &value_len,
-                                       ATTR_ROOT, NULL);
+                                       ATTR_ROOT);
        DM_EA_XLATE_ERR(error);

        /* DMAPI requires an errno of ENOENT if an attribute does not exist,
@@ -2217,7 +2216,7 @@ xfs_dm_getall_dmattr(

                        error = xfs_attr_get(XFS_I(inode), entry->a_name,
                                                (void *)(ulist + 1), &value_len,
-                                               ATTR_ROOT, NULL);
+                                               ATTR_ROOT);
                        DM_EA_XLATE_ERR(error);

                        if (error || value_len != entry->a_valuelen) {
Index: kern_ci/fs/xfs/linux-2.6/xfs_ioctl.c
===================================================================
--- kern_ci.orig/fs/xfs/linux-2.6/xfs_ioctl.c
+++ kern_ci/fs/xfs/linux-2.6/xfs_ioctl.c
@@ -505,14 +505,14 @@ xfs_attrmulti_attr_get(
 {
        char                    *kbuf;
        int                     error = EFAULT;
-       
+
        if (*len > XATTR_SIZE_MAX)
                return EINVAL;
        kbuf = kmalloc(*len, GFP_KERNEL);
        if (!kbuf)
                return ENOMEM;

-       error = xfs_attr_get(XFS_I(inode), name, kbuf, (int *)len, flags, NULL);
+       error = xfs_attr_get(XFS_I(inode), name, kbuf, (int *)len, flags);
        if (error)
                goto out_kfree;

@@ -548,7 +548,7 @@ xfs_attrmulti_attr_set(

        if (copy_from_user(kbuf, ubuf, len))
                goto out_kfree;
-                       
+
        error = xfs_attr_set(XFS_I(inode), name, kbuf, len, flags);

  out_kfree:
Index: kern_ci/fs/xfs/xfs_acl.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_acl.c
+++ kern_ci/fs/xfs/xfs_acl.c
@@ -341,14 +341,15 @@ xfs_acl_iaccess(
 {
        xfs_acl_t       *acl;
        int             rval;
+       struct xfs_name acl_name = {SGI_ACL_FILE, SGI_ACL_FILE_SIZE};

        if (!(_ACL_ALLOC(acl)))
                return -1;

        /* If the file has no ACL return -1. */
        rval = sizeof(xfs_acl_t);
-       if (xfs_attr_fetch(ip, SGI_ACL_FILE, SGI_ACL_FILE_SIZE,
-                       (char *)acl, &rval, ATTR_ROOT | ATTR_KERNACCESS, cr)) {
+       if (xfs_attr_fetch(ip, &acl_name, (char *)acl, &rval,
+                                       ATTR_ROOT | ATTR_KERNACCESS)) {
                _ACL_FREE(acl);
                return -1;
        }
@@ -595,7 +596,7 @@ xfs_acl_get_attr(
        *error = xfs_attr_get(xfs_vtoi(vp),
                                        kind == _ACL_TYPE_ACCESS ?
                                        SGI_ACL_FILE : SGI_ACL_DEFAULT,
-                                       (char *)aclp, &len, flags, sys_cred);
+                                       (char *)aclp, &len, flags);
        if (*error || (flags & ATTR_KERNOVAL))
                return;
        xfs_acl_get_endian(aclp);
Index: kern_ci/fs/xfs/xfs_attr.c
===================================================================
--- kern_ci.orig/fs/xfs/xfs_attr.c
+++ kern_ci/fs/xfs/xfs_attr.c
@@ -101,14 +101,32 @@ STATIC int xfs_attr_rmtval_remove(xfs_da
 ktrace_t *xfs_attr_trace_buf;
 #endif

+STATIC int
+xfs_attr_name_to_name(
+       xfs_inode_t     *ip,
+       struct xfs_name *xname,
+       const char      *aname)
+{
+       if (!aname)
+               return EINVAL;
+       xname->name = aname;
+       xname->len = strlen(aname);
+       if (xname->len >= MAXNAMELEN)
+               return EFAULT;          /* match IRIX behaviour */
+
+       if (XFS_FORCED_SHUTDOWN(ip->i_mount))
+               return EIO;
+
+       return 0;
+}

 /*========================================================================
  * Overall external interface routines.
  *========================================================================*/

 int
-xfs_attr_fetch(xfs_inode_t *ip, const char *name, int namelen,
-              char *value, int *valuelenp, int flags, struct cred *cred)
+xfs_attr_fetch(xfs_inode_t *ip, struct xfs_name *name,
+               char *value, int *valuelenp, int flags)
 {
        xfs_da_args_t   args;
        int             error;
@@ -122,8 +140,8 @@ xfs_attr_fetch(xfs_inode_t *ip, const ch
         * Fill in the arg structure for this request.
         */
        memset((char *)&args, 0, sizeof(args));
-       args.name = name;
-       args.namelen = namelen;
+       args.name = name->name;
+       args.namelen = name->len;
        args.value = value;
        args.valuelen = *valuelenp;
        args.flags = flags;
@@ -162,31 +180,27 @@ xfs_attr_get(
        const char      *name,
        char            *value,
        int             *valuelenp,
-       int             flags,
-       cred_t          *cred)
+       int             flags)
 {
-       int             error, namelen;
+       int             error;
+       struct xfs_name xname;

        XFS_STATS_INC(xs_attr_get);

-       if (!name)
-               return(EINVAL);
-       namelen = strlen(name);
-       if (namelen >= MAXNAMELEN)
-               return(EFAULT);         /* match IRIX behaviour */
-
-       if (XFS_FORCED_SHUTDOWN(ip->i_mount))
-               return(EIO);
+       error = xfs_attr_name_to_name(ip, &xname, name);
+       if (error)
+               return error;

        xfs_ilock(ip, XFS_ILOCK_SHARED);
-       error = xfs_attr_fetch(ip, name, namelen, value, valuelenp, flags, 
cred);
+       error = xfs_attr_fetch(ip, &xname, value, valuelenp, flags);
        xfs_iunlock(ip, XFS_ILOCK_SHARED);
-       return(error);
+
+       return error;
 }

-int
-xfs_attr_set_int(xfs_inode_t *dp, const char *name, int namelen,
-                char *value, int valuelen, int flags)
+STATIC int
+xfs_attr_set_int(xfs_inode_t *dp, struct xfs_name *name,
+               char *value, int valuelen, int flags)
 {
        xfs_da_args_t   args;
        xfs_fsblock_t   firstblock;
@@ -209,7 +223,7 @@ xfs_attr_set_int(xfs_inode_t *dp, const
         */
        if (XFS_IFORK_Q(dp) == 0) {
                int sf_size = sizeof(xfs_attr_sf_hdr_t) +
-                             XFS_ATTR_SF_ENTSIZE_BYNAME(namelen, valuelen);
+                             XFS_ATTR_SF_ENTSIZE_BYNAME(name->len, valuelen);

                if ((error = xfs_bmap_add_attrfork(dp, sf_size, rsvd)))
                        return(error);
@@ -219,8 +233,8 @@ xfs_attr_set_int(xfs_inode_t *dp, const
         * Fill in the arg structure for this request.
         */
        memset((char *)&args, 0, sizeof(args));
-       args.name = name;
-       args.namelen = namelen;
+       args.name = name->name;
+       args.namelen = name->len;
        args.value = value;
        args.valuelen = valuelen;
        args.flags = flags;
@@ -236,7 +250,7 @@ xfs_attr_set_int(xfs_inode_t *dp, const
         * Determine space new attribute will use, and if it would be
         * "local" or "remote" (note: local != inline).
         */
-       size = xfs_attr_leaf_newentsize(namelen, valuelen,
+       size = xfs_attr_leaf_newentsize(name->len, valuelen,
                                        mp->m_sb.sb_blocksize, &local);

        nblks = XFS_DAENTER_SPACE_RES(mp, XFS_ATTR_FORK);
@@ -429,26 +443,24 @@ xfs_attr_set(
        int             valuelen,
        int             flags)
 {
-       int             namelen;
-
-       namelen = strlen(name);
-       if (namelen >= MAXNAMELEN)
-               return EFAULT;          /* match IRIX behaviour */
+       int             error;
+       struct xfs_name xname;

        XFS_STATS_INC(xs_attr_set);

-       if (XFS_FORCED_SHUTDOWN(dp->i_mount))
-               return (EIO);
+       error = xfs_attr_name_to_name(dp, &xname, name);
+       if (error)
+               return error;

-       return xfs_attr_set_int(dp, name, namelen, value, valuelen, flags);
+       return xfs_attr_set_int(dp, &xname, value, valuelen, flags);
 }

 /*
  * Generic handler routine to remove a name from an attribute list.
  * Transitions attribute list from Btree to shortform as necessary.
  */
-int
-xfs_attr_remove_int(xfs_inode_t *dp, const char *name, int namelen, int flags)
+STATIC int
+xfs_attr_remove_int(xfs_inode_t *dp, struct xfs_name *name, int flags)
 {
        xfs_da_args_t   args;
        xfs_fsblock_t   firstblock;
@@ -460,8 +472,8 @@ xfs_attr_remove_int(xfs_inode_t *dp, con
         * Fill in the arg structure for this request.
         */
        memset((char *)&args, 0, sizeof(args));
-       args.name = name;
-       args.namelen = namelen;
+       args.name = name->name;
+       args.namelen = name->len;
        args.flags = flags;
        args.hashval = xfs_da_hashname(args.name, args.namelen);
        args.dp = dp;
@@ -575,16 +587,14 @@ xfs_attr_remove(
        const char      *name,
        int             flags)
 {
-       int             namelen;
-
-       namelen = strlen(name);
-       if (namelen >= MAXNAMELEN)
-               return EFAULT;          /* match IRIX behaviour */
+       int             error;
+       struct xfs_name xname;

        XFS_STATS_INC(xs_attr_remove);

-       if (XFS_FORCED_SHUTDOWN(dp->i_mount))
-               return (EIO);
+       error = xfs_attr_name_to_name(dp, &xname, name);
+       if (error)
+               return error;

        xfs_ilock(dp, XFS_ILOCK_SHARED);
        if (XFS_IFORK_Q(dp) == 0 ||
@@ -595,10 +605,10 @@ xfs_attr_remove(
        }
        xfs_iunlock(dp, XFS_ILOCK_SHARED);

-       return xfs_attr_remove_int(dp, name, namelen, flags);
+       return xfs_attr_remove_int(dp, &xname, flags);
 }

-int                                                            /* error */
+STATIC int
 xfs_attr_list_int(xfs_attr_list_context_t *context)
 {
        int error;
@@ -2522,8 +2532,7 @@ attr_generic_get(
 {
        int     error, asize = size;

-       error = xfs_attr_get(xfs_vtoi(vp), name, data,
-                                   &asize, xflags, NULL);
+       error = xfs_attr_get(xfs_vtoi(vp), name, data, &asize, xflags);
        if (!error)
                return asize;
        return -error;
Index: kern_ci/fs/xfs/xfs_attr.h
===================================================================
--- kern_ci.orig/fs/xfs/xfs_attr.h
+++ kern_ci/fs/xfs/xfs_attr.h
@@ -158,14 +158,10 @@ struct xfs_da_args;
 /*
  * Overall external interface routines.
  */
-int xfs_attr_set_int(struct xfs_inode *, const char *, int, char *, int, int);
-int xfs_attr_remove_int(struct xfs_inode *, const char *, int, int);
-int xfs_attr_list_int(struct xfs_attr_list_context *);
 int xfs_attr_inactive(struct xfs_inode *dp);

 int xfs_attr_shortform_getvalue(struct xfs_da_args *);
-int xfs_attr_fetch(struct xfs_inode *, const char *, int,
-                       char *, int *, int, struct cred *);
+int xfs_attr_fetch(struct xfs_inode *, struct xfs_name *, char *, int *, int);
 int xfs_attr_rmtval_get(struct xfs_da_args *args);

 #endif /* __XFS_ATTR_H__ */
Index: kern_ci/fs/xfs/xfs_vnodeops.h
===================================================================
--- kern_ci.orig/fs/xfs/xfs_vnodeops.h
+++ kern_ci/fs/xfs/xfs_vnodeops.h
@@ -50,7 +50,7 @@ int xfs_rename(struct xfs_inode *src_dp,
                struct xfs_inode *src_ip, struct xfs_inode *target_dp,
                struct xfs_name *target_name, struct xfs_inode *target_ip);
 int xfs_attr_get(struct xfs_inode *ip, const char *name, char *value,
-               int *valuelenp, int flags, cred_t *cred);
+               int *valuelenp, int flags);
 int xfs_attr_set(struct xfs_inode *dp, const char *name, char *value,
                int valuelen, int flags);
 int xfs_attr_remove(struct xfs_inode *dp, const char *name, int flags);

Attachment: attr_name.patch
Description: Text Data

<Prev in Thread] Current Thread [Next in Thread>