Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 14 Jul 2022 18:08:00 +0530
From:   Ritesh Harjani <ritesh.list@gmail.com>
To:     Jan Kara <jack@suse.cz>
Cc:     Ted Tso <tytso@mit.edu>, linux-ext4@vger.kernel.org
Subject: Re: [PATCH 07/10] ext2: Unindent codeblock in ext2_xattr_set()
Message-ID: <20220714123800.sdrkkidu3yq6uyhh@riteshh-domain>
References: <20220712104519.29887-1-jack@suse.cz>
 <20220712105436.32204-7-jack@suse.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220712105436.32204-7-jack@suse.cz>
Precedence: bulk

On 22/07/12 12:54PM, Jan Kara wrote:
> Replace one else in ext2_xattr_set() with a goto. This makes following
> code changes simpler to follow. No functional changes.

Straight forward refactoring. Looks good to me.

Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>

>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/ext2/xattr.c | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
> index 9885294993ef..37ce495eb279 100644
> --- a/fs/ext2/xattr.c
> +++ b/fs/ext2/xattr.c
> @@ -517,7 +517,8 @@ ext2_xattr_set(struct inode *inode, int name_index, const char *name,
>  	/* Here we know that we can set the new attribute. */
>
>  	if (header) {
> -		/* assert(header == HDR(bh)); */
> +		int offset;
> +
>  		lock_buffer(bh);
>  		if (header->h_refcount == cpu_to_le32(1)) {
>  			__u32 hash = le32_to_cpu(header->h_hash);
> @@ -531,22 +532,20 @@ ext2_xattr_set(struct inode *inode, int name_index, const char *name,
>  					      bh->b_blocknr);
>
>  			/* keep the buffer locked while modifying it. */
> -		} else {
> -			int offset;
> -
> -			unlock_buffer(bh);
> -			ea_bdebug(bh, "cloning");
> -			header = kmemdup(HDR(bh), bh->b_size, GFP_KERNEL);
> -			error = -ENOMEM;
> -			if (header == NULL)
> -				goto cleanup;
> -			header->h_refcount = cpu_to_le32(1);
> -
> -			offset = (char *)here - bh->b_data;
> -			here = ENTRY((char *)header + offset);
> -			offset = (char *)last - bh->b_data;
> -			last = ENTRY((char *)header + offset);
> +			goto update_block;
>  		}
> +		unlock_buffer(bh);
> +		ea_bdebug(bh, "cloning");
> +		header = kmemdup(HDR(bh), bh->b_size, GFP_KERNEL);
> +		error = -ENOMEM;
> +		if (header == NULL)
> +			goto cleanup;
> +		header->h_refcount = cpu_to_le32(1);
> +
> +		offset = (char *)here - bh->b_data;
> +		here = ENTRY((char *)header + offset);
> +		offset = (char *)last - bh->b_data;
> +		last = ENTRY((char *)header + offset);
>  	} else {
>  		/* Allocate a buffer where we construct the new block. */
>  		header = kzalloc(sb->s_blocksize, GFP_KERNEL);
> @@ -559,6 +558,7 @@ ext2_xattr_set(struct inode *inode, int name_index, const char *name,
>  		last = here = ENTRY(header+1);
>  	}
>
> +update_block:
>  	/* Iff we are modifying the block in-place, bh is locked here. */
>
>  	if (not_found) {
> --
> 2.35.3
>