[linux-cifs-client] [PATCH 1/4] cifs: clean up variables in cifs_unlink

[Jeff Layton]

Change the "inode" param in cifs_unlink to "pinode" to make it clear
that it's the parent directory inode. Add a new "inode" pointer to
refer to the dentry->d_inode.

Also, add a superblock pointer and fix up a potential NULL pointer
dereference near the bottom of the function if pinode was ever NULL.

Signed-off-by: Jeff Layton <jlayton at redhat.com>
---
 fs/cifs/inode.c |   48 +++++++++++++++++++++++++-----------------------
 1 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index ed2b5df..9161c7b 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -665,7 +665,7 @@ struct inode *cifs_iget(struct super_block *sb, unsigned long ino)
 	return inode;
 }
 
-int cifs_unlink(struct inode *inode, struct dentry *direntry)
+int cifs_unlink(struct inode *pinode, struct dentry *direntry)
 {
 	int rc = 0;
 	int xid;
@@ -673,23 +673,25 @@ int cifs_unlink(struct inode *inode, struct dentry *direntry)
 	struct cifsTconInfo *pTcon;
 	char *full_path = NULL;
 	struct cifsInodeInfo *cifsInode;
+	struct super_block *sb;
+	struct inode *inode = direntry->d_inode;
 	FILE_BASIC_INFO *pinfo_buf;
 
-	cFYI(1, ("cifs_unlink, inode = 0x%p", inode));
+	cFYI(1, ("cifs_unlink, pinode = 0x%p, dentry=0x%p", pinode, direntry));
 
 	xid = GetXid();
 
-	if (inode)
-		cifs_sb = CIFS_SB(inode->i_sb);
+	if (pinode)
+		sb = pinode->i_sb;
 	else
-		cifs_sb = CIFS_SB(direntry->d_sb);
+		sb = direntry->d_sb;
+
+	cifs_sb = CIFS_SB(sb);
 	pTcon = cifs_sb->tcon;
 
-	/* Unlink can be called from rename so we can not grab the sem here
-	   since we deadlock otherwise */
-/*	mutex_lock(&direntry->d_sb->s_vfs_rename_mutex);*/
+	/* Unlink can be called from rename so we can not take the
+	 * sb->s_vfs_rename_mutex here */
 	full_path = build_path_from_dentry(direntry);
-/*	mutex_unlock(&direntry->d_sb->s_vfs_rename_mutex);*/
 	if (full_path == NULL) {
 		FreeXid(xid);
 		return -ENOMEM;
@@ -710,8 +712,8 @@ int cifs_unlink(struct inode *inode, struct dentry *direntry)
 			cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
 psx_del_no_retry:
 	if (!rc) {
-		if (direntry->d_inode)
-			drop_nlink(direntry->d_inode);
+		if (inode)
+			drop_nlink(inode);
 	} else if (rc == -ENOENT) {
 		d_drop(direntry);
 	} else if (rc == -ETXTBSY) {
@@ -729,8 +731,8 @@ psx_del_no_retry:
 					      cifs_sb->mnt_cifs_flags &
 						CIFS_MOUNT_MAP_SPECIAL_CHR);
 			CIFSSMBClose(xid, pTcon, netfid);
-			if (direntry->d_inode)
-				drop_nlink(direntry->d_inode);
+			if (inode)
+				drop_nlink(inode);
 		}
 	} else if (rc == -EACCES) {
 		/* try only if r/o attribute set in local lookup data? */
@@ -784,8 +786,8 @@ psx_del_no_retry:
 					    cifs_sb->mnt_cifs_flags &
 						CIFS_MOUNT_MAP_SPECIAL_CHR);
 			if (!rc) {
-				if (direntry->d_inode)
-					drop_nlink(direntry->d_inode);
+				if (inode)
+					drop_nlink(inode);
 			} else if (rc == -ETXTBSY) {
 				int oplock = 0;
 				__u16 netfid;
@@ -805,22 +807,22 @@ psx_del_no_retry:
 						cifs_sb->mnt_cifs_flags &
 						    CIFS_MOUNT_MAP_SPECIAL_CHR);
 					CIFSSMBClose(xid, pTcon, netfid);
-					if (direntry->d_inode)
-						drop_nlink(direntry->d_inode);
+					if (inode)
+						drop_nlink(inode);
 				}
 			/* BB if rc = -ETXTBUSY goto the rename logic BB */
 			}
 		}
 	}
-	if (direntry->d_inode) {
-		cifsInode = CIFS_I(direntry->d_inode);
+	if (inode) {
+		cifsInode = CIFS_I(inode);
 		cifsInode->time = 0;	/* will force revalidate to get info
 					   when needed */
-		direntry->d_inode->i_ctime = current_fs_time(inode->i_sb);
+		inode->i_ctime = current_fs_time(sb);
 	}
-	if (inode) {
-		inode->i_ctime = inode->i_mtime = current_fs_time(inode->i_sb);
-		cifsInode = CIFS_I(inode);
+	if (pinode) {
+		pinode->i_ctime = pinode->i_mtime = current_fs_time(sb);
+		cifsInode = CIFS_I(pinode);
 		cifsInode->time = 0;	/* force revalidate of dir as well */
 	}
 
-- 
1.5.5.1