2012-04-30 15:34:08

by Matthew Treinish

Subject: [PATCH] Fixed goto readability in nfs_update_inode.

Simplified error gotos to make it slightly easier to read,
it doesn't affect the functionality of the routine.

Signed-off-by: Matthew Treinish <[email protected]>
---
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;
}


--
1.7.7.6

2012-04-30 16:09:49

by Malahal Naineni

Subject: Re: [PATCH] Fixed goto readability in nfs_update_inode.

Matthew Treinish [[email protected]] 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 <[email protected]>
> ---
> 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 <[email protected]>