From: David Sterba <dsterba@suse.cz>
To: Anand Jain <anand.jain@oracle.com>
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH v5 9/9] btrfs: kill btrfs_setxattr
Date: Fri, 8 Mar 2019 15:56:42 +0100 [thread overview]
Message-ID: <20190308145642.GS31119@twin.jikos.cz> (raw)
In-Reply-To: <1551414895-22925-10-git-send-email-anand.jain@oracle.com>
On Fri, Mar 01, 2019 at 12:34:55PM +0800, Anand Jain wrote:
> Now btrfs_setxattr() is a very small function with just check for
> readonly FS and redirect the call to do_setxattr(). So instead
> move that checks to the parent functions and call do_setxattr()
> directly. Delete original btrfs_setxattr(), and rename do_setxattr()
> to btrfs_setxattr(). Also add few c-style. Kindly note the arguments
> of original do_setxattr() and original btrfs_setxattr() are same, so the
> diff obliterates the changes as described above.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> v5: rename do_setxattr() to btrfs_setxattr(). change log update. fix c-style.
> v4: born
> fs/btrfs/acl.c | 7 +++++++
> fs/btrfs/props.c | 16 ++++++++++------
> fs/btrfs/xattr.c | 29 +++++++++--------------------
> fs/btrfs/xattr.h | 5 ++---
> 4 files changed, 28 insertions(+), 29 deletions(-)
>
> diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
> index 2b9c3394fc34..a8a1060c8cbe 100644
> --- a/fs/btrfs/acl.c
> +++ b/fs/btrfs/acl.c
> @@ -60,6 +60,7 @@ static int do_set_acl(struct btrfs_trans_handle *trans, struct inode *inode,
> int ret, size = 0;
> const char *name;
> char *value = NULL;
> + struct btrfs_root *root = BTRFS_I(inode)->root;
>
> switch (type) {
> case ACL_TYPE_ACCESS:
> @@ -95,7 +96,13 @@ static int do_set_acl(struct btrfs_trans_handle *trans, struct inode *inode,
> goto out;
> }
>
> + if (btrfs_root_readonly(root)) {
> + ret = -EROFS;
> + goto out;
All the readonly checks needs to go first as it's the global condition,
the following checks make sure that the arguments are valid and depend
on the previous.
> - ret = btrfs_setxattr(trans, inode, handler->xattr_name,
> - NULL, 0, flags);
> + ret = btrfs_setxattr(trans, inode, handler->xattr_name, NULL, 0,
> + flags);
drive-by change
> if (ret)
> return ret;
>
> @@ -85,14 +89,14 @@ static int btrfs_set_prop(struct btrfs_trans_handle *trans, struct inode *inode,
> ret = handler->validate(value, value_len);
> if (ret)
> return ret;
> - ret = btrfs_setxattr(trans, inode, handler->xattr_name,
> - value, value_len, flags);
> + ret = btrfs_setxattr(trans, inode, handler->xattr_name, value,
> + value_len, flags);
same
> if (ret)
> return ret;
> ret = handler->apply(inode, value, value_len);
> if (ret) {
> - btrfs_setxattr(trans, inode, handler->xattr_name,
> - NULL, 0, flags);
> + ret = btrfs_setxattr(trans, inode, handler->xattr_name, NULL, 0,
> + flags);
And that one silently changes the return value semantics but looks like
the other two, "just fixing the indentation". The original code does not
set 'ret' as the whole operation returns what the property handler
returned.
The setxattr call here resets the property. If this is not the
right, then fixed separately.
next prev parent reply other threads:[~2019-03-08 14:55 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-01 4:34 [PATCH v5 0/9] Misc props.c cleanups Anand Jain
2019-03-01 4:34 ` [PATCH v5 1/9] btrfs: kill __btrfs_set_prop() Anand Jain
2019-03-01 4:34 ` [PATCH v5 2/9] btrfs: drop redundant forward declaration in props.c Anand Jain
2019-03-01 4:34 ` [PATCH v5 3/9] btrfs: rename fs_info argument to fs_private Anand Jain
2019-03-01 4:34 ` [PATCH v5 4/9] btrfs: refactor btrfs_set_prop add btrfs_set_prop_trans Anand Jain
2019-03-08 14:49 ` David Sterba
2019-03-01 4:34 ` [PATCH v5 5/9] btrfs: start transaction in btrfs_set_prop_trans Anand Jain
2019-03-01 4:34 ` [PATCH v5 6/9] btrfs: start transaction in btrfs_set_acl Anand Jain
2019-03-08 14:52 ` David Sterba
2019-03-01 4:34 ` [PATCH v5 7/9] btrfs: start transaction in btrfs_xattr_handler_set Anand Jain
2019-03-01 4:34 ` [PATCH v5 8/9] btrfs: btrfs_setxattr argument trans is never NULL Anand Jain
2019-03-01 4:34 ` [PATCH v5 9/9] btrfs: kill btrfs_setxattr Anand Jain
2019-03-08 14:56 ` David Sterba [this message]
2019-03-09 1:18 ` Anand Jain
2019-03-08 15:01 ` [PATCH v5 0/9] Misc props.c cleanups David Sterba
2019-03-09 0:04 ` Anand Jain
2019-03-11 14:41 ` David Sterba
2019-03-12 11:12 ` Anand Jain
2019-03-12 17:22 ` David Sterba
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190308145642.GS31119@twin.jikos.cz \
--to=dsterba@suse.cz \
--cc=anand.jain@oracle.com \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).