[PATCH] in cifssmb.c add copy_from_user return value check and do minor formatting/whitespace cleanups.

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH] in cifssmb.c add copy_from_user return value check and do minor formatting/whitespace cleanups.
@ 2004-12-21 23:33 Jesper Juhl
  2004-12-21 23:54 ` Domen Puncer
  0 siblings, 1 reply; 4+ messages in thread
From: Jesper Juhl @ 2004-12-21 23:33 UTC (permalink / raw)
  To: Steve French, Steve French; +Cc: samba-technical, linux-kernel


Hi, 

Seeing this warning :

fs/cifs/cifssmb.c:902: warning: ignoring return value of `copy_from_user', declared with attribute warn_unused_result

lead to the following patch.

The patch adds a check of the copy_from_user return value and returns 
-EFAULT if the call fails. In addition to that I did some 
formatting/whitespace changes - the code uses primarily tabs for 
indentation, but this little bit used spaces, so I changed that to tabs; I 
also added a few curly braces {} for a few if statements, in the same 
area, that seemed to be becomming quite hard to read without.

Patch has been compile tested, but I have no real way to test it properly 
beyond that. 
I hope the patch is acceptable and mergable :)

Btw, I'm only subscribed to LKML, so please keep me on CC.


Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk>

diff -up linux-2.6.10-rc3-bk13-orig/fs/cifs/cifssmb.c linux-2.6.10-rc3-bk13/fs/cifs/cifssmb.c
--- linux-2.6.10-rc3-bk13-orig/fs/cifs/cifssmb.c	2004-12-20 22:19:42.000000000 +0100
+++ linux-2.6.10-rc3-bk13/fs/cifs/cifssmb.c	2004-12-22 00:21:38.000000000 +0100
@@ -895,14 +895,19 @@ CIFSSMBWrite(const int xid, struct cifsT
 		bytes_sent = count;
 	pSMB->DataLengthHigh = 0;
 	pSMB->DataOffset =
-	    cpu_to_le16(offsetof(struct smb_com_write_req,Data) - 4);
-    if(buf)
-	    memcpy(pSMB->Data,buf,bytes_sent);
-	else if(ubuf)
-		copy_from_user(pSMB->Data,ubuf,bytes_sent);
-    else {
+		cpu_to_le16(offsetof(struct smb_com_write_req,Data) - 4);
+
+	if (buf) {
+		memcpy(pSMB->Data,buf,bytes_sent);
+	} else if (ubuf) {
+		if (copy_from_user(pSMB->Data,ubuf,bytes_sent)) {
+			if (pSMB)
+				cifs_buf_release(pSMB);
+			return -EFAULT;
+		}
+	} else {
 		/* No buffer */
-		if(pSMB)
+		if (pSMB)
 			cifs_buf_release(pSMB);
 		return -EINVAL;
 	}




^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: in cifssmb.c add copy_from_user return value check and do minor formatting/whitespace cleanups.
  2004-12-21 23:33 [PATCH] in cifssmb.c add copy_from_user return value check and do minor formatting/whitespace cleanups Jesper Juhl
@ 2004-12-21 23:54 ` Domen Puncer
  2004-12-22  0:27   ` Jesper Juhl
  0 siblings, 1 reply; 4+ messages in thread
From: Domen Puncer @ 2004-12-21 23:54 UTC (permalink / raw)
  To: Jesper Juhl; +Cc: Steve French, Steve French, samba-technical, linux-kernel

On 22/12/04 00:33 +0100, Jesper Juhl wrote:
> 
> Hi, 
> 
> Seeing this warning :
> 
> fs/cifs/cifssmb.c:902: warning: ignoring return value of `copy_from_user', declared with attribute warn_unused_result
> 
> lead to the following patch.
> 
> The patch adds a check of the copy_from_user return value and returns 
> -EFAULT if the call fails. In addition to that I did some 
> formatting/whitespace changes - the code uses primarily tabs for 
> indentation, but this little bit used spaces, so I changed that to tabs; I 
> also added a few curly braces {} for a few if statements, in the same 
> area, that seemed to be becomming quite hard to read without.
> 
> Patch has been compile tested, but I have no real way to test it properly 
> beyond that. 
> I hope the patch is acceptable and mergable :)
> 
> Btw, I'm only subscribed to LKML, so please keep me on CC.
> 
> 
> Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk>
> 
> diff -up linux-2.6.10-rc3-bk13-orig/fs/cifs/cifssmb.c linux-2.6.10-rc3-bk13/fs/cifs/cifssmb.c
> --- linux-2.6.10-rc3-bk13-orig/fs/cifs/cifssmb.c	2004-12-20 22:19:42.000000000 +0100
> +++ linux-2.6.10-rc3-bk13/fs/cifs/cifssmb.c	2004-12-22 00:21:38.000000000 +0100
> @@ -895,14 +895,19 @@ CIFSSMBWrite(const int xid, struct cifsT
>  		bytes_sent = count;
>  	pSMB->DataLengthHigh = 0;
>  	pSMB->DataOffset =
> -	    cpu_to_le16(offsetof(struct smb_com_write_req,Data) - 4);
> -    if(buf)
> -	    memcpy(pSMB->Data,buf,bytes_sent);
> -	else if(ubuf)
> -		copy_from_user(pSMB->Data,ubuf,bytes_sent);
> -    else {
> +		cpu_to_le16(offsetof(struct smb_com_write_req,Data) - 4);
> +
> +	if (buf) {
> +		memcpy(pSMB->Data,buf,bytes_sent);
> +	} else if (ubuf) {
> +		if (copy_from_user(pSMB->Data,ubuf,bytes_sent)) {
> +			if (pSMB)

How can this be NULL, and code not Oopsing?

> +				cifs_buf_release(pSMB);
> +			return -EFAULT;
> +		}
> +	} else {
>  		/* No buffer */
> -		if(pSMB)
> +		if (pSMB)

Same here.

>  			cifs_buf_release(pSMB);
>  		return -EINVAL;
>  	}
> 
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: in cifssmb.c add copy_from_user return value check and do minor formatting/whitespace cleanups.
  2004-12-21 23:54 ` Domen Puncer
@ 2004-12-22  0:27   ` Jesper Juhl
  2004-12-22  1:03     ` [third patch version] " Jesper Juhl
  0 siblings, 1 reply; 4+ messages in thread
From: Jesper Juhl @ 2004-12-22  0:27 UTC (permalink / raw)
  To: Domen Puncer; +Cc: Steve French, Steve French, samba-technical, linux-kernel

On Wed, 22 Dec 2004, Domen Puncer wrote:

> On 22/12/04 00:33 +0100, Jesper Juhl wrote:
> > 
[...]
> > +	if (buf) {
> > +		memcpy(pSMB->Data,buf,bytes_sent);
> > +	} else if (ubuf) {
> > +		if (copy_from_user(pSMB->Data,ubuf,bytes_sent)) {
> > +			if (pSMB)
> 
> How can this be NULL, and code not Oopsing?
> 
Right, it would have Oopsed in tons of places above if pSMB was NULL, so 
there's no point in checking it at this stage. And if I'm reading 
smb_init() correctly, then it will return an error if it can't give pSMB a 
sane value, so we already bail out at the smb_init() call in that case.

How about this patch instead? (same as the previous one except it also 
removes those aparently pointless checks of pSMB and changes a single 
instance of a pointer being compared to 0 to instead compare against NULL 
so sparse won't complain about that) :


Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk>

diff -up linux-2.6.10-rc3-bk13-orig/fs/cifs/cifssmb.c linux-2.6.10-rc3-bk13/fs/cifs/cifssmb.c
--- linux-2.6.10-rc3-bk13-orig/fs/cifs/cifssmb.c	2004-12-20 22:19:42.000000000 +0100
+++ linux-2.6.10-rc3-bk13/fs/cifs/cifssmb.c	2004-12-22 01:20:21.000000000 +0100
@@ -246,7 +246,7 @@ smb_init(int smb_command, int wct, struc
 		return rc;
 
 	*request_buf = cifs_buf_get();
-	if (*request_buf == 0) {
+	if (*request_buf == NULL) {
 		/* BB should we add a retry in here if not a writepage? */
 		return -ENOMEM;
 	}
@@ -895,15 +895,18 @@ CIFSSMBWrite(const int xid, struct cifsT
 		bytes_sent = count;
 	pSMB->DataLengthHigh = 0;
 	pSMB->DataOffset =
-	    cpu_to_le16(offsetof(struct smb_com_write_req,Data) - 4);
-    if(buf)
-	    memcpy(pSMB->Data,buf,bytes_sent);
-	else if(ubuf)
-		copy_from_user(pSMB->Data,ubuf,bytes_sent);
-    else {
-		/* No buffer */
-		if(pSMB)
+		cpu_to_le16(offsetof(struct smb_com_write_req,Data) - 4);
+
+	if (buf) {
+		memcpy(pSMB->Data,buf,bytes_sent);
+	} else if (ubuf) {
+		if (copy_from_user(pSMB->Data,ubuf,bytes_sent)) {
 			cifs_buf_release(pSMB);
+			return -EFAULT;
+		}
+	} else {
+		/* No buffer */
+		cifs_buf_release(pSMB);
 		return -EINVAL;
 	}
 
@@ -921,8 +924,7 @@ CIFSSMBWrite(const int xid, struct cifsT
 	} else
 		*nbytes = le16_to_cpu(pSMBr->Count);
 
-	if (pSMB)
-		cifs_buf_release(pSMB);
+	cifs_buf_release(pSMB);
 
 	/* Note: On -EAGAIN error only caller can retry on handle based calls 
 		since file handle passed in no longer valid */




^ permalink raw reply	[flat|nested] 4+ messages in thread

* [third patch version] Re: in cifssmb.c add copy_from_user return value check and do minor formatting/whitespace cleanups.
  2004-12-22  0:27   ` Jesper Juhl
@ 2004-12-22  1:03     ` Jesper Juhl
  0 siblings, 0 replies; 4+ messages in thread
From: Jesper Juhl @ 2004-12-22  1:03 UTC (permalink / raw)
  To: Domen Puncer; +Cc: Steve French, Steve French, samba-technical, linux-kernel

On Wed, 22 Dec 2004, Jesper Juhl wrote:

> On Wed, 22 Dec 2004, Domen Puncer wrote:
> 
> > On 22/12/04 00:33 +0100, Jesper Juhl wrote:
> > > 
> [...]
> > > +	if (buf) {
> > > +		memcpy(pSMB->Data,buf,bytes_sent);
> > > +	} else if (ubuf) {
> > > +		if (copy_from_user(pSMB->Data,ubuf,bytes_sent)) {
> > > +			if (pSMB)
> > 
> > How can this be NULL, and code not Oopsing?
> > 
> Right, it would have Oopsed in tons of places above if pSMB was NULL, so 
> there's no point in checking it at this stage. And if I'm reading 
> smb_init() correctly, then it will return an error if it can't give pSMB a 
> sane value, so we already bail out at the smb_init() call in that case.
> 
> How about this patch instead? (same as the previous one except it also 
> removes those aparently pointless checks of pSMB and changes a single 
> instance of a pointer being compared to 0 to instead compare against NULL 
> so sparse won't complain about that) :
> 
And if it is correct that those pSMB checks can go (and it certainly looks 
to me as if they can), then why not clean them out from the entire file? 
and while we are at it fix up a few lines that are seriously whitespace 
challenged as well.  That's what this third version does - same stuff as 
the first two, but with a few more whitespace cleanups and the (aparently) 
useless pSMB checks gone from the entire file.

Please review and consider for inclusion :-)


Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk>

diff -up linux-2.6.10-rc3-bk13-orig/fs/cifs/cifssmb.c linux-2.6.10-rc3-bk13/fs/cifs/cifssmb.c
--- linux-2.6.10-rc3-bk13-orig/fs/cifs/cifssmb.c	2004-12-20 22:19:42.000000000 +0100
+++ linux-2.6.10-rc3-bk13/fs/cifs/cifssmb.c	2004-12-22 01:54:32.000000000 +0100
@@ -246,7 +246,7 @@ smb_init(int smb_command, int wct, struc
 		return rc;
 
 	*request_buf = cifs_buf_get();
-	if (*request_buf == 0) {
+	if (*request_buf == NULL) {
 		/* BB should we add a retry in here if not a writepage? */
 		return -ENOMEM;
 	}
@@ -409,8 +409,8 @@ CIFSSMBNegotiate(unsigned int xid, struc
 		}
 				
 	}
-	if (pSMB)
-		cifs_buf_release(pSMB);
+	cifs_buf_release(pSMB);
+
 	return rc;
 }
 
@@ -523,8 +523,7 @@ CIFSSMBLogoff(const int xid, struct cifs
 			rc = -ESHUTDOWN;
 		}
 	}
-	if (pSMB)
-		cifs_small_buf_release(pSMB);
+	cifs_small_buf_release(pSMB);
 	up(&ses->sesSem);
 
 	/* if session dead then we do not need to do ulogoff,
@@ -579,8 +578,7 @@ DelFileRetry:
         }
 #endif
 
-	if (pSMB)
-		cifs_buf_release(pSMB);
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto DelFileRetry;
 
@@ -630,10 +628,10 @@ RmDirRetry:
         }
 #endif
 
-	if (pSMB)
-		cifs_buf_release(pSMB);
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto RmDirRetry;
+
 	return rc;
 }
 
@@ -679,10 +677,10 @@ MkDirRetry:
 		atomic_inc(&tcon->num_mkdirs);
         }
 #endif
-	if (pSMB)
-		cifs_buf_release(pSMB);
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto MkDirRetry;
+
 	return rc;
 }
 
@@ -780,10 +778,10 @@ openRetry:
 		atomic_inc(&tcon->num_opens);
 #endif
 	}
-	if (pSMB)
-		cifs_buf_release(pSMB);
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto openRetry;
+
 	return rc;
 }
 
@@ -846,12 +844,10 @@ CIFSSMBRead(const int xid, struct cifsTc
 			    memcpy(*buf,pReadData,data_length);
 		}
 	}
-	if (pSMB) {
-		if(*buf)
-			cifs_buf_release(pSMB);
-		else
-			*buf = (char *)pSMB;
-	}
+	if(*buf)
+		cifs_buf_release(pSMB);
+	else
+		*buf = (char *)pSMB;
 
 	/* Note: On -EAGAIN error only caller can retry on handle based calls 
 		since file handle passed in no longer valid */
@@ -895,15 +891,18 @@ CIFSSMBWrite(const int xid, struct cifsT
 		bytes_sent = count;
 	pSMB->DataLengthHigh = 0;
 	pSMB->DataOffset =
-	    cpu_to_le16(offsetof(struct smb_com_write_req,Data) - 4);
-    if(buf)
-	    memcpy(pSMB->Data,buf,bytes_sent);
-	else if(ubuf)
-		copy_from_user(pSMB->Data,ubuf,bytes_sent);
-    else {
-		/* No buffer */
-		if(pSMB)
+		cpu_to_le16(offsetof(struct smb_com_write_req,Data) - 4);
+
+	if (buf) {
+		memcpy(pSMB->Data,buf,bytes_sent);
+	} else if (ubuf) {
+		if (copy_from_user(pSMB->Data,ubuf,bytes_sent)) {
 			cifs_buf_release(pSMB);
+			return -EFAULT;
+		}
+	} else {
+		/* No buffer */
+		cifs_buf_release(pSMB);
 		return -EINVAL;
 	}
 
@@ -921,12 +920,10 @@ CIFSSMBWrite(const int xid, struct cifsT
 	} else
 		*nbytes = le16_to_cpu(pSMBr->Count);
 
-	if (pSMB)
-		cifs_buf_release(pSMB);
+	cifs_buf_release(pSMB);
 
 	/* Note: On -EAGAIN error only caller can retry on handle based calls 
 		since file handle passed in no longer valid */
-
 	return rc;
 }
 
@@ -946,7 +943,7 @@ int CIFSSMBWrite2(const int xid, struct 
 	rc = small_smb_init(SMB_COM_WRITE_ANDX, 14, tcon, (void **) &pSMB);
 	pSMBr = (WRITE_RSP *)pSMB; /* BB removeme BB */
     
-    if (rc)
+	if (rc)
 		return rc;
 	/* tcon and ses pointer are checked in smb_init */
 	if (tcon->ses->server == NULL)
@@ -980,8 +977,7 @@ int CIFSSMBWrite2(const int xid, struct 
 	} else
 		*nbytes = le16_to_cpu(pSMBr->Count);
 
-	if (pSMB)
-		cifs_small_buf_release(pSMB);
+	cifs_small_buf_release(pSMB);
 
 	/* Note: On -EAGAIN error only caller can retry on handle based calls 
 		since file handle passed in no longer valid */
@@ -1046,8 +1042,7 @@ CIFSSMBLock(const int xid, struct cifsTc
 	if (rc) {
 		cFYI(1, ("Send error in Lock = %d", rc));
 	}
-	if (pSMB)
-		cifs_buf_release(pSMB);
+	cifs_buf_release(pSMB);
 
 	/* Note: On -EAGAIN error only caller can retry on handle based calls 
 	since file handle passed in no longer valid */
@@ -1082,9 +1077,7 @@ CIFSSMBClose(const int xid, struct cifsT
 			cERROR(1, ("Send error in Close = %d", rc));
 		}
 	}
-
-	if (pSMB)
-		cifs_small_buf_release(pSMB);
+	cifs_small_buf_release(pSMB);
 
 	/* Since session is dead, file will be closed on server already */
 	if(rc == -EAGAIN)
@@ -1161,9 +1154,7 @@ renameRetry:
 	}
 #endif
 
-	if (pSMB)
-		cifs_buf_release(pSMB);
-
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto renameRetry;
 
@@ -1241,12 +1232,11 @@ int CIFSSMBRenameOpenFile(const int xid,
 		atomic_inc(&pTcon->num_t2renames);
 	}
 #endif
-	if (pSMB)
-		cifs_buf_release(pSMB);
+
+	cifs_buf_release(pSMB);
 
 	/* Note: On -EAGAIN error only caller can retry on handle based calls
 		since file handle passed in no longer valid */
-
 	return rc;
 }
 
@@ -1311,9 +1301,7 @@ copyRetry:
 		cFYI(1, ("Send error in copy = %d with %d files copied",
 			rc, le16_to_cpu(pSMBr->CopyCount)));
 	}
-	if (pSMB)
-		cifs_buf_release(pSMB);
-
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto copyRetry;
 
@@ -1403,9 +1391,7 @@ createSymLinkRetry:
 		      rc));
 	}
 
-	if (pSMB)
-		cifs_buf_release(pSMB);
-
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto createSymLinkRetry;
 
@@ -1492,8 +1478,7 @@ createHardLinkRetry:
 		cFYI(1, ("Send error in SetPathInfo (hard link) = %d", rc));
 	}
 
-	if (pSMB)
-		cifs_buf_release(pSMB);
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto createHardLinkRetry;
 
@@ -1564,8 +1549,7 @@ winCreateHardLinkRetry:
 	if (rc) {
 		cFYI(1, ("Send error in hard link (NT rename) = %d", rc));
 	}
-	if (pSMB)
-		cifs_buf_release(pSMB);
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto winCreateHardLinkRetry;
 
@@ -1665,15 +1649,13 @@ querySymLinkRetry:
 	/* just in case so calling code does not go off the end of buffer */
 		}
 	}
-	if (pSMB)
-		cifs_buf_release(pSMB);
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto querySymLinkRetry;
+
 	return rc;
 }
 
-
-
 int
 CIFSSMBQueryReparseLinkInfo(const int xid, struct cifsTconInfo *tcon,
 			const unsigned char *searchName,
@@ -1749,12 +1731,10 @@ CIFSSMBQueryReparseLinkInfo(const int xi
 			cFYI(1,("readlink result - %s ",symlinkinfo));
 		}
 	}
-	if (pSMB)
-		cifs_buf_release(pSMB);
+	cifs_buf_release(pSMB);
 
 	/* Note: On -EAGAIN error only caller can retry on handle based calls
 		since file handle passed in no longer valid */
-
 	return rc;
 }
 
@@ -1969,10 +1949,10 @@ queryAclRetry:
 				buflen,acl_type,count);
 		}
 	}
-	if (pSMB)
-		cifs_buf_release(pSMB);
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto queryAclRetry;
+
 	return rc;
 }
 
@@ -1996,7 +1976,8 @@ setAclRetry:
                       (void **) &pSMBr);
 	if (rc)
 		return rc;
-                                                                                                               	if (pSMB->hdr.Flags2 & SMBFLG2_UNICODE) {
+
+	if (pSMB->hdr.Flags2 & SMBFLG2_UNICODE) {
 		name_len =
 			cifs_strtoUCS((wchar_t *) pSMB->FileName, fileName, PATH_MAX
 				/* BB fixme find define for this maxpathcomponent */
@@ -2049,11 +2030,11 @@ setAclRetry:
 	}
 
 setACLerrorExit:
-	if (pSMB)
-		cifs_buf_release(pSMB);
-                                                                                                            	if (rc == -EAGAIN)
+	cifs_buf_release(pSMB);
+	if (rc == -EAGAIN)
 		goto setAclRetry;
-                                                                                                           	return rc;
+	
+	return rc;
 }
 
 #endif
@@ -2133,8 +2114,7 @@ QPathInfoRetry:
 		} else
 		    rc = -ENOMEM;
 	}
-	if (pSMB)
-		cifs_buf_release(pSMB);
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto QPathInfoRetry;
 
@@ -2217,8 +2197,7 @@ UnixQPathInfoRetry:
 			       sizeof (FILE_UNIX_BASIC_INFO));
 		}
 	}
-	if (pSMB)
-		cifs_buf_release(pSMB);
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto UnixQPathInfoRetry;
 
@@ -2297,8 +2276,7 @@ findUniqueRetry:
 
 		/* BB fill in */
 	}
-	if (pSMB)
-		cifs_buf_release(pSMB);
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto findUniqueRetry;
 
@@ -2406,9 +2384,7 @@ findFirstRetry:
 				le16_to_cpu(pSMBr->t2.DataCount));
 		}
 	}
-	if (pSMB)
-		cifs_buf_release(pSMB);
-
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto findFirstRetry;
 
@@ -2498,8 +2474,7 @@ findFirst2Retry:
 		/* BB Add code to handle unsupported level rc */
 		cFYI(1, ("Error in FindFirst = %d", rc));
 
-		if (pSMB)
-			cifs_buf_release(pSMB);
+		cifs_buf_release(pSMB);
 
 		/* BB eventually could optimize out free and realloc of buf */
 		/*    for this case */
@@ -2533,8 +2508,7 @@ findFirst2Retry:
 /*cFYI(1,("entries in buf %d index_of_last %d",psrch_inf->entries_in_buffer,psrch_inf->index_of_last_entry));  */
 			*pnetfid = parms->SearchHandle;
 		} else {
-			if(pSMB)
-				cifs_buf_release(pSMB);
+			cifs_buf_release(pSMB);
 		}
 	}
 
@@ -2659,7 +2633,7 @@ int CIFSFindNext2(const int xid, struct 
 	/* Note: On -EAGAIN error only caller can retry on handle based calls
 	since file handle passed in no longer valid */
 FNext2_err_exit:
-	if ((rc != 0) && pSMB)
+	if (rc != 0)
 		cifs_buf_release(pSMB);
                                                                                               
 	return rc;
@@ -2763,12 +2737,10 @@ CIFSFindNext(const int xid, struct cifsT
 			memcpy(findData,response_data,le16_to_cpu(pSMBr->t2.DataCount));
 		}
 	}
-	if (pSMB)
-		cifs_buf_release(pSMB);
+	cifs_buf_release(pSMB);
 
 	/* Note: On -EAGAIN error only caller can retry on handle based calls
 		since file handle passed in no longer valid */
-
 	return rc;
 }
 
@@ -2831,11 +2803,10 @@ GetInodeNumberRetry:
 
 /* BB add missing code here */
 
-	if (pSMB)
-		cifs_buf_release(pSMB);
-                                                                                                                         
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto GetInodeNumberRetry;
+
 	return rc;
 }
 #endif /* CIFS_EXPERIMENTAL */
@@ -3002,9 +2973,7 @@ getDFSRetry:
 
 	}
 GetDFSRefExit:
-	if (pSMB)
-		cifs_buf_release(pSMB);
-
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto getDFSRetry;
 
@@ -3087,9 +3056,7 @@ QFSInfoRetry:
 			      FSData->f_bsize));
 		}
 	}
-	if (pSMB)
-		cifs_buf_release(pSMB);
-
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto QFSInfoRetry;
 
@@ -3157,9 +3124,7 @@ QFSAttributeRetry:
 			       sizeof (FILE_SYSTEM_ATTRIBUTE_INFO));
 		}
 	}
-	if (pSMB)
-		cifs_buf_release(pSMB);
-
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto QFSAttributeRetry;
 
@@ -3228,9 +3193,7 @@ QFSDeviceRetry:
 			       sizeof (FILE_SYSTEM_DEVICE_INFO));
 		}
 	}
-	if (pSMB)
-		cifs_buf_release(pSMB);
-
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto QFSDeviceRetry;
 
@@ -3298,13 +3261,10 @@ QFSUnixRetry:
 			       sizeof (FILE_SYSTEM_UNIX_INFO));
 		}
 	}
-	if (pSMB)
-		cifs_buf_release(pSMB);
-
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto QFSUnixRetry;
 
-
 	return rc;
 }
 
@@ -3396,9 +3356,7 @@ SetEOFRetry:
 		cFYI(1, ("SetPathInfo (file size) returned %d", rc));
 	}
 
-	if (pSMB)
-		cifs_buf_release(pSMB);
-
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto SetEOFRetry;
 
@@ -3482,12 +3440,10 @@ CIFSSMBSetFileSize(const int xid, struct
 		      rc));
 	}
 
-	if (pSMB)
-		cifs_buf_release(pSMB);
+	cifs_buf_release(pSMB);
 
 	/* Note: On -EAGAIN error only caller can retry on handle based calls 
 		since file handle passed in no longer valid */
-
 	return rc;
 }
 
@@ -3563,9 +3519,7 @@ SetTimesRetry:
 		cFYI(1, ("SetPathInfo (times) returned %d", rc));
 	}
 
-	if (pSMB)
-		cifs_buf_release(pSMB);
-
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto SetTimesRetry;
 
@@ -3641,9 +3595,7 @@ SetTimesRetryLegacy:
 		cFYI(1, ("SetPathInfo (times legacy) returned %d", rc));
 	}
 
-	if (pSMB)
-		cifs_buf_release(pSMB);
-
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto SetTimesRetryLegacy;
 
@@ -3742,10 +3694,10 @@ setPermsRetry:
 		cFYI(1, ("SetPathInfo (perms) returned %d", rc));
 	}
 
-	if (pSMB)
-		cifs_buf_release(pSMB);
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto setPermsRetry;
+
 	return rc;
 }
 
@@ -3789,10 +3741,10 @@ int CIFSSMBNotify(const int xid, struct 
 	if (rc) {
 		cFYI(1, ("Error in Notify = %d", rc));
 	}
-	if (pSMB)
-		cifs_buf_release(pSMB);
-/*		if (rc == -EAGAIN)
-			goto NotifyRetry; */
+	cifs_buf_release(pSMB);
+/*	if (rc == -EAGAIN)
+		goto NotifyRetry; */
+
 	return rc;	
 }
 #ifdef CONFIG_CIFS_XATTR
@@ -3934,8 +3886,7 @@ QAllEAsRetry:
 			}
 		}
 	}
-	if (pSMB)
-		cifs_buf_release(pSMB);
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto QAllEAsRetry;
 
@@ -4082,8 +4033,7 @@ QEARetry:
 			} 
 		}
 	}
-	if (pSMB)
-		cifs_buf_release(pSMB);
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto QEARetry;
 
@@ -4185,9 +4135,7 @@ SetEARetry:
 		cFYI(1, ("SetPathInfo (EA) returned %d", rc));
 	}
 
-	if (pSMB)
-		cifs_buf_release(pSMB);
-
+	cifs_buf_release(pSMB);
 	if (rc == -EAGAIN)
 		goto SetEARetry;
 



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2004-12-22  0:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-12-21 23:33 [PATCH] in cifssmb.c add copy_from_user return value check and do minor formatting/whitespace cleanups Jesper Juhl
2004-12-21 23:54 ` Domen Puncer
2004-12-22  0:27   ` Jesper Juhl
2004-12-22  1:03     ` [third patch version] " Jesper Juhl

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.