[patch] CIFS: cleanup min_t() cast in cifs_read()

[patch] CIFS: cleanup min_t() cast in cifs_read()

[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).

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 9f41a10..a61f581 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -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;

Re: [patch] CIFS: cleanup min_t() cast in cifs_read()

[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;

On Mon, Oct 17, 2011 at 2:30 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> 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).
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 9f41a10..a61f581 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -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;
>



-- 
Thanks,

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

Re: [patch] CIFS: cleanup min_t() cast in cifs_read()

[Dan Carpenter]

On Mon, Oct 17, 2011 at 09:10:01AM -0500, Steve French wrote:
> 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

[patch v2] CIFS: cleanup min_t() cast in cifs_read()

[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.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2:  use "uint" instead of "u32".

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index ab85699..34be15a 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -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;

Re: [patch v2] CIFS: cleanup min_t() cast in cifs_read()

[Steve French]

merged.

On Tue, Oct 18, 2011 at 4:41 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> 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.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2:  use "uint" instead of "u32".
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index ab85699..34be15a 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -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;
>



-- 
Thanks,

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