Skip to contentSkip to conversation

CIFS: cleanup min_t() cast in cifs_read()

PATCH Superseded Dan Carpenter 2 participants 5 messages Oct 17, 2011 – Oct 18, 2011 3 messages not in the archive

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 CIFS: cleanup min_t() cast in cifs_read()

Smatch complains that the cast to "int" in min_t() changes very large values of current_read_size into negative values and so min_t() could return the wrong value.

Also casting the "const" here doesn't make sense.

I've changed it to u32 ("unsigned int" take puts us past the 80 character limit).

fs/cifs/file.c +2 -2
index 9f41a10..a61f581 100644
@@ -1861,13 +1861,13 @@ static ssize_t cifs_read(struct file *file, char *read_data, size_t read_size,
 	for (total_read = 0, current_offset = read_data;
 	     read_size > total_read;
 	     total_read += bytes_read, current_offset += bytes_read) {
-		current_read_size = min_t(const int, read_size - total_read,
+		current_read_size = min_t(u32, read_size - total_read,
 					  cifs_sb->rsize);
 		/* For windows me and 9x we do not want to request more
 		than it negotiated since it will refuse the read then */
 		if ((pTcon->ses) &&
 			!(pTcon->ses->capabilities & CAP_LARGE_FILES)) {
-			current_read_size = min_t(const int, current_read_size,
+			current_read_size = min_t(u32, current_read_size,
 					pTcon->ses->server->maxBuf - 128);
 		}
 		rc = -EAGAIN;

A message that is not in the archive (sent off-list or not yet received): [email protected]

#2 lore raw

I prefer the change to unsigned int rather than u32 so it is clearer that you are not casting the value from 64 bit to 32 bit on some architectures. To avoid reformatting can't you can use "uint" if you want the shorter form see:

include/linux/types.h:typedef unsigned int uint;

Quoted 32 lines · Dan Carpenter

Smatch complains that the cast to "int" in min_t() changes very large values of current_read_size into negative values and so min_t() could return the wrong value.

Also casting the "const" here doesn't make sense.

I've changed it to u32 ("unsigned int" take puts us past the 80 character limit).

fs/cifs/file.c +1 -1
index 9f41a10..a61f581 100644
@@ -1861,13 +1861,13 @@ static ssize_t cifs_read(struct file *file, char *read_data, size_t read_size,
        for (total_read = 0, current_offset = read_data;
             read_size > total_read;
             total_read += bytes_read, current_offset += bytes_read) {
-               current_read_size = min_t(const int, read_size - total_read,
+               current_read_size = min_t(u32, read_size - total_read,
                                          cifs_sb->rsize);
                /* For windows me and 9x we do not want to request more

               than it negotiated since it will refuse the read then */ if ((pTcon->ses) &&
                       !(pTcon->ses->capabilities & CAP_LARGE_FILES)) {
-                       current_read_size = min_t(const int, current_read_size,
+                       current_read_size = min_t(u32, current_read_size, pTcon->ses->server->maxBuf - 128); }
               rc = -EAGAIN;

Signature · 7 lines
Thanks,

Steve
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): CAH2r5mufq0=BsZvfGuVdd1BU-gxRpnUmWAm-vzhsOR4KbvGSbg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#4 lore raw
Quoted 7 lines · Steve French

I prefer the change to unsigned int rather than u32 so it is clearer that you are not casting the value from 64 bit to 32 bit on some architectures. To avoid reformatting can't you can use "uint" if you want the shorter form see:

include/linux/types.h:typedef unsigned int uint;

Yes. That looks nicer. I'll resend.

regards,
dan carpenter

#5 lore raw

PATCH v2 1/1 CIFS: cleanup min_t() cast in cifs_read()

In reply to a message not in the archive

Smatch complains that the cast to "int" in min_t() changes very large values of current_read_size into negative values and so min_t() could return the wrong value. I removed the const as well, as that doesn't do anything here.

---
v2: use "uint" instead of "u32".

fs/cifs/file.c +2 -2
index ab85699..34be15a 100644
@@ -1900,13 +1900,13 @@ static ssize_t cifs_read(struct file *file, char *read_data, size_t read_size,
 	for (total_read = 0, current_offset = read_data;
 	     read_size > total_read;
 	     total_read += bytes_read, current_offset += bytes_read) {
-		current_read_size = min_t(const int, read_size - total_read,
+		current_read_size = min_t(uint, read_size - total_read,
 					  cifs_sb->rsize);
 		/* For windows me and 9x we do not want to request more
 		than it negotiated since it will refuse the read then */
 		if ((pTcon->ses) &&
 			!(pTcon->ses->capabilities & CAP_LARGE_FILES)) {
-			current_read_size = min_t(const int, current_read_size,
+			current_read_size = min_t(uint, current_read_size,
 					CIFSMaxBufSize);
 		}
 		rc = -EAGAIN;

A message that is not in the archive (sent off-list or not yet received): 20111018094135.GE25814-dZEljifmRObu9KfB+GxooP8+0UxHXcjY@public.gmane.org

#7 lore raw

merged.

Quoted 30 lines · Dan Carpenter

Smatch complains that the cast to "int" in min_t() changes very large values of current_read_size into negative values and so min_t() could return the wrong value.  I removed the const as well, as that doesn't do anything here.

---
v2:  use "uint" instead of "u32".

fs/cifs/file.c +1 -1
index ab85699..34be15a 100644
@@ -1900,13 +1900,13 @@ static ssize_t cifs_read(struct file *file, char *read_data, size_t read_size,
        for (total_read = 0, current_offset = read_data;
             read_size > total_read;
             total_read += bytes_read, current_offset += bytes_read) {
-               current_read_size = min_t(const int, read_size - total_read,
+               current_read_size = min_t(uint, read_size - total_read,
                                          cifs_sb->rsize);
                /* For windows me and 9x we do not want to request more

               than it negotiated since it will refuse the read then */ if ((pTcon->ses) &&
                       !(pTcon->ses->capabilities & CAP_LARGE_FILES)) {
-                       current_read_size = min_t(const int, current_read_size,
+                       current_read_size = min_t(uint, current_read_size, CIFSMaxBufSize);
               }
               rc = -EAGAIN;

Signature · 7 lines
Thanks,

Steve
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html