xfs
[Top] [All Lists]

Re: [PATCH] xfs: Fix error path in xfs_get_acl

To: Andreas Gruenbacher <agruenba@xxxxxxxxxx>
Subject: Re: [PATCH] xfs: Fix error path in xfs_get_acl
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Mon, 12 Oct 2015 11:45:39 +1100
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1444569791-26719-1-git-send-email-agruenba@xxxxxxxxxx>
References: <1444569791-26719-1-git-send-email-agruenba@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Sun, Oct 11, 2015 at 03:23:11PM +0200, Andreas Gruenbacher wrote:
> Error codes from xfs_attr_get other than -ENOATTR were not properly 
> reported.  Fix that, and clean the code up somewhat.

Test case for xfstests?

> In addition, the declaration of struct xfs_inode in xfs_acl.h isn't needed.
> 
> Signed-off-by: Andreas Gruenbacher <agruenba@xxxxxxxxxx>
> ---
>  fs/xfs/xfs_acl.c | 19 +++++++------------
>  fs/xfs/xfs_acl.h |  1 -
>  2 files changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/xfs/xfs_acl.c b/fs/xfs/xfs_acl.c
> index 4b64167..0f4ee92 100644
> --- a/fs/xfs/xfs_acl.c
> +++ b/fs/xfs/xfs_acl.c
> @@ -122,7 +122,7 @@ struct posix_acl *
>  xfs_get_acl(struct inode *inode, int type)
>  {
>       struct xfs_inode *ip = XFS_I(inode);
> -     struct posix_acl *acl = NULL;
> +     struct posix_acl *acl;
>       struct xfs_acl *xfs_acl;
>       unsigned char *ea_name;
>       int error;
> @@ -158,18 +158,13 @@ xfs_get_acl(struct inode *inode, int type)
>                * cache entry, for any other error assume it is transient and
>                * leave the cache entry as ACL_NOT_CACHED.
>                */
> -             if (error == -ENOATTR)
> -                     goto out_update_cache;
> -             goto out;
> -     }
> +             acl = (error == -ENOATTR) ? NULL : ERR_PTR(error);

AFAICT, this single line here is the bugfix, right?

So the entire patch could simply be replaced by:

                if (error == -ENOATTR)
                        goto out_update_cache;
+               acl = ERR_PTR(error);
                goto out;
        }

Or have I missed the missed the fix in all the code rearranging you
did?

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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