[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20180417083854.7dgypv3eu4tc4e2c@mwanda>
Date: Tue, 17 Apr 2018 11:38:54 +0300
From: Dan Carpenter <dan.carpenter@...cle.com>
To: James Simmons <jsimmons@...radead.org>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
devel@...verdev.osuosl.org,
Andreas Dilger <andreas.dilger@...el.com>,
Oleg Drokin <oleg.drokin@...el.com>,
NeilBrown <neilb@...e.com>,
Dmitry Eremin <dmitry.eremin@...el.com>,
"John L. Hammond" <john.hammond@...el.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Lustre Development List <lustre-devel@...ts.lustre.org>
Subject: Re: [PATCH 19/22] staging: lustre: llite: add support set_acl method
in inode operations
On Mon, Apr 16, 2018 at 12:15:08AM -0400, James Simmons wrote:
> +int ll_set_acl(struct inode *inode, struct posix_acl *acl, int type)
> +{
> + struct ll_sb_info *sbi = ll_i2sbi(inode);
> + struct ptlrpc_request *req = NULL;
> + const char *name = NULL;
> + size_t value_size = 0;
> + char *value = NULL;
> + int rc;
> +
> + switch (type) {
> + case ACL_TYPE_ACCESS:
> + name = XATTR_NAME_POSIX_ACL_ACCESS;
> + if (acl) {
> + rc = posix_acl_update_mode(inode, &inode->i_mode, &acl);
> + if (rc)
> + goto out;
> + }
> +
> + break;
> +
> + case ACL_TYPE_DEFAULT:
> + name = XATTR_NAME_POSIX_ACL_DEFAULT;
> + if (!S_ISDIR(inode->i_mode)) {
> + rc = acl ? -EACCES : 0;
> + goto out;
I just hate "goto out;" labels... They're so impossible to review...
Why are we calling forget_cached_acl() when we haven't set anything? I
have no idea. Perhaps it's not even intentional.
> + }
> +
> + break;
> +
> + default:
> + rc = -EINVAL;
> + goto out;
And on this path we're calling forget_cached_acl(inode, type) with an
invalid "type" so it will trigger a BUG()... That's obviously not
intended... Who knows what's going on when all the names are vague and
hand wavy.
regards,
dan carpenter
Powered by blists - more mailing lists