Date: Mon, 30 Apr 2012 11:09:42 -0500
From: Malahal Naineni <malahal@us.ibm.com>
To: linux-nfs@vger.kernel.org
Subject: Re: [PATCH] Fixed goto readability in nfs_update_inode.
Message-ID: <20120430160942.GA536@us.ibm.com>
References: <1335799977-5587-1-git-send-email-treinish@linux.vnet.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
In-Reply-To: <1335799977-5587-1-git-send-email-treinish@linux.vnet.ibm.com>
Sender: linux-nfs-owner@vger.kernel.org

Matthew Treinish [treinish@linux.vnet.ibm.com] wrote:
> Simplified error gotos to make it slightly easier to read,
> it doesn't affect the functionality of the routine.
> 
> Signed-off-by: Matthew Treinish <treinish@linux.vnet.ibm.com>
> ---
>  fs/nfs/inode.c |   34 +++++++++++++++++-----------------
>  1 files changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index e8bbfa5..943d1d0 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -1279,14 +1279,26 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
>  			nfs_display_fhandle_hash(NFS_FH(inode)),
>  			atomic_read(&inode->i_count), fattr->valid);
> 
> -	if ((fattr->valid & NFS_ATTR_FATTR_FILEID) && nfsi->fileid != fattr->fileid)
> -		goto out_fileid;
> +	if ((fattr->valid & NFS_ATTR_FATTR_FILEID) && nfsi->fileid != fattr->fileid) {
> +		printk(KERN_ERR "NFS: server %s error: fileid changed\n"
> +			"fsid %s: expected fileid 0x%Lx, got 0x%Lx\n",
> +			NFS_SERVER(inode)->nfs_client->cl_hostname,
> +			inode->i_sb->s_id, (long long)nfsi->fileid,
> +			(long long)fattr->fileid);
> +		goto out_err;
> +	}
> 
>  	/*
>  	 * Make sure the inode's type hasn't changed.
>  	 */
> -	if ((fattr->valid & NFS_ATTR_FATTR_TYPE) && (inode->i_mode & S_IFMT) != (fattr->mode & S_IFMT))
> -		goto out_changed;
> +	if ((fattr->valid & NFS_ATTR_FATTR_TYPE) && (inode->i_mode & S_IFMT) != (fattr->mode & S_IFMT)) {
> +		/*
> +		* Big trouble! The inode has become a different object.
> +		*/
> +		printk(KERN_DEBUG "NFS: %s: inode %ld mode changed, %07o to %07o\n",
> +				__func__, inode->i_ino, inode->i_mode, fattr->mode);
> +		goto out_err;
> +	}
> 
>  	server = NFS_SERVER(inode);
>  	/* Update the fsid? */
> @@ -1466,12 +1479,6 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
>  		nfsi->cache_validity |= invalid;
> 
>  	return 0;
> - out_changed:
> -	/*
> -	 * Big trouble! The inode has become a different object.
> -	 */
> -	printk(KERN_DEBUG "NFS: %s: inode %ld mode changed, %07o to %07o\n",
> -			__func__, inode->i_ino, inode->i_mode, fattr->mode);
>   out_err:
>  	/*
>  	 * No need to worry about unhashing the dentry, as the
> @@ -1480,13 +1487,6 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
>  	 */
>  	nfs_invalidate_inode(inode);
>  	return -ESTALE;
> -
> - out_fileid:
> -	printk(KERN_ERR "NFS: server %s error: fileid changed\n"
> -		"fsid %s: expected fileid 0x%Lx, got 0x%Lx\n",
> -		NFS_SERVER(inode)->nfs_client->cl_hostname, inode->i_sb->s_id,
> -		(long long)nfsi->fileid, (long long)fattr->fileid);
> -	goto out_err;
>  }

Reviewed-by: Malahal Naineni <malahal@us.ibm.com>