[PATCH 1/2] cifs: Use min_t() when comparing "size_t" and "unsigned long"

[PATCH 1/2] cifs: Use min_t() when comparing "size_t" and "unsigned long"

[Geert Uytterhoeven]

On 32 bit, size_t is "unsigned int", not "unsigned long", causing the
following warning when comparing with PAGE_SIZE, which is always "unsigned
long":

fs/cifs/file.c: In function ‘cifs_readdata_to_iov’:
fs/cifs/file.c:2757: warning: comparison of distinct pointer types lacks a cast

Introduced by commit 7f25bba819a38ab7310024a9350655f374707e20
("cifs_iovec_read: keep iov_iter between the calls of
cifs_readdata_to_iov()"), which changed the signedness of "remaining"
and the code from min_t() to min().

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
PAGE_SIZE should really be size_t, but that would require lots of changes
all over the place.

 fs/cifs/file.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 8807442c94dd..8add25538a3b 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2754,7 +2754,7 @@ cifs_readdata_to_iov(struct cifs_readdata *rdata, struct iov_iter *iter)
 
 	for (i = 0; i < rdata->nr_pages; i++) {
 		struct page *page = rdata->pages[i];
-		size_t copy = min(remaining, PAGE_SIZE);
+		size_t copy = min_t(size_t, remaining, PAGE_SIZE);
 		size_t written = copy_page_to_iter(page, 0, copy, iter);
 		remaining -= written;
 		if (written < copy && iov_iter_count(iter) > 0)
-- 
1.7.9.5

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/2] cifs: Use min_t() when comparing "size_t" and "unsigned long"

[Geert Uytterhoeven]

On 32 bit, size_t is "unsigned int", not "unsigned long", causing the
following warning when comparing with PAGE_SIZE, which is always "unsigned
long":

fs/cifs/file.c: In function ‘cifs_readdata_to_iov’:
fs/cifs/file.c:2757: warning: comparison of distinct pointer types lacks a cast

Introduced by commit 7f25bba819a38ab7310024a9350655f374707e20
("cifs_iovec_read: keep iov_iter between the calls of
cifs_readdata_to_iov()"), which changed the signedness of "remaining"
and the code from min_t() to min().

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
PAGE_SIZE should really be size_t, but that would require lots of changes
all over the place.

 fs/cifs/file.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 8807442c94dd..8add25538a3b 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2754,7 +2754,7 @@ cifs_readdata_to_iov(struct cifs_readdata *rdata, struct iov_iter *iter)
 
 	for (i = 0; i < rdata->nr_pages; i++) {
 		struct page *page = rdata->pages[i];
-		size_t copy = min(remaining, PAGE_SIZE);
+		size_t copy = min_t(size_t, remaining, PAGE_SIZE);
 		size_t written = copy_page_to_iter(page, 0, copy, iter);
 		remaining -= written;
 		if (written < copy && iov_iter_count(iter) > 0)
-- 
1.7.9.5

[PATCH 1/2] cifs: Use min_t() when comparing "size_t" and "unsigned long"

[Geert Uytterhoeven]

On 32 bit, size_t is "unsigned int", not "unsigned long", causing the
following warning when comparing with PAGE_SIZE, which is always "unsigned
long":

fs/cifs/file.c: In function a??cifs_readdata_to_iova??:
fs/cifs/file.c:2757: warning: comparison of distinct pointer types lacks a cast

Introduced by commit 7f25bba819a38ab7310024a9350655f374707e20
("cifs_iovec_read: keep iov_iter between the calls of
cifs_readdata_to_iov()"), which changed the signedness of "remaining"
and the code from min_t() to min().

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
PAGE_SIZE should really be size_t, but that would require lots of changes
all over the place.

 fs/cifs/file.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 8807442c94dd..8add25538a3b 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2754,7 +2754,7 @@ cifs_readdata_to_iov(struct cifs_readdata *rdata, struct iov_iter *iter)
 
 	for (i = 0; i < rdata->nr_pages; i++) {
 		struct page *page = rdata->pages[i];
-		size_t copy = min(remaining, PAGE_SIZE);
+		size_t copy = min_t(size_t, remaining, PAGE_SIZE);
 		size_t written = copy_page_to_iter(page, 0, copy, iter);
 		remaining -= written;
 		if (written < copy && iov_iter_count(iter) > 0)
-- 
1.7.9.5

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/2] mm: Initialize error in shmem_file_aio_read()

[Geert Uytterhoeven]

mm/shmem.c: In function ‘shmem_file_aio_read’:
mm/shmem.c:1414: warning: ‘error’ may be used uninitialized in this function

If the loop is aborted during the first iteration by one of the two first
break statements, error will be uninitialized.

Introduced by commit 6e58e79db8a16222b31fc8da1ca2ac2dccfc4237
("introduce copy_page_to_iter, kill loop over iovec in
generic_file_aio_read()").

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
The code is too complex to see if this is an obvious false positive.

 mm/shmem.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/shmem.c b/mm/shmem.c
index 8f1a95406bae..9f70e02111c6 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -1411,7 +1411,7 @@ static ssize_t shmem_file_aio_read(struct kiocb *iocb,
 	pgoff_t index;
 	unsigned long offset;
 	enum sgp_type sgp = SGP_READ;
-	int error;
+	int error = 0;
 	ssize_t retval;
 	size_t count;
 	loff_t *ppos = &iocb->ki_pos;
-- 
1.7.9.5

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/2] mm: Initialize error in shmem_file_aio_read()

[Geert Uytterhoeven]

mm/shmem.c: In function ‘shmem_file_aio_read’:
mm/shmem.c:1414: warning: ‘error’ may be used uninitialized in this function

If the loop is aborted during the first iteration by one of the two first
break statements, error will be uninitialized.

Introduced by commit 6e58e79db8a16222b31fc8da1ca2ac2dccfc4237
("introduce copy_page_to_iter, kill loop over iovec in
generic_file_aio_read()").

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
The code is too complex to see if this is an obvious false positive.

 mm/shmem.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/shmem.c b/mm/shmem.c
index 8f1a95406bae..9f70e02111c6 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -1411,7 +1411,7 @@ static ssize_t shmem_file_aio_read(struct kiocb *iocb,
 	pgoff_t index;
 	unsigned long offset;
 	enum sgp_type sgp = SGP_READ;
-	int error;
+	int error = 0;
 	ssize_t retval;
 	size_t count;
 	loff_t *ppos = &iocb->ki_pos;
-- 
1.7.9.5

[PATCH 2/2] mm: Initialize error in shmem_file_aio_read()

[Geert Uytterhoeven]

mm/shmem.c: In function a??shmem_file_aio_reada??:
mm/shmem.c:1414: warning: a??errora?? may be used uninitialized in this function

If the loop is aborted during the first iteration by one of the two first
break statements, error will be uninitialized.

Introduced by commit 6e58e79db8a16222b31fc8da1ca2ac2dccfc4237
("introduce copy_page_to_iter, kill loop over iovec in
generic_file_aio_read()").

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
The code is too complex to see if this is an obvious false positive.

 mm/shmem.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/shmem.c b/mm/shmem.c
index 8f1a95406bae..9f70e02111c6 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -1411,7 +1411,7 @@ static ssize_t shmem_file_aio_read(struct kiocb *iocb,
 	pgoff_t index;
 	unsigned long offset;
 	enum sgp_type sgp = SGP_READ;
-	int error;
+	int error = 0;
 	ssize_t retval;
 	size_t count;
 	loff_t *ppos = &iocb->ki_pos;
-- 
1.7.9.5

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 2/2] mm: Initialize error in shmem_file_aio_read()

[Al Viro]

On Sun, Apr 13, 2014 at 08:46:22PM +0200, Geert Uytterhoeven wrote:
> mm/shmem.c: In function ‘shmem_file_aio_read’:
> mm/shmem.c:1414: warning: ‘error’ may be used uninitialized in this function
> 
> If the loop is aborted during the first iteration by one of the two first
> break statements, error will be uninitialized.
> 
> Introduced by commit 6e58e79db8a16222b31fc8da1ca2ac2dccfc4237
> ("introduce copy_page_to_iter, kill loop over iovec in
> generic_file_aio_read()").
> 
> Signed-off-by: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
> ---
> The code is too complex to see if this is an obvious false positive.

Good catch; sadly, it *can* be triggered - read() starting past the EOF
will step into it.  Applied, will push today.

Re: [PATCH 2/2] mm: Initialize error in shmem_file_aio_read()

[Al Viro]

On Sun, Apr 13, 2014 at 08:46:22PM +0200, Geert Uytterhoeven wrote:
> mm/shmem.c: In function ‘shmem_file_aio_read’:
> mm/shmem.c:1414: warning: ‘error’ may be used uninitialized in this function
> 
> If the loop is aborted during the first iteration by one of the two first
> break statements, error will be uninitialized.
> 
> Introduced by commit 6e58e79db8a16222b31fc8da1ca2ac2dccfc4237
> ("introduce copy_page_to_iter, kill loop over iovec in
> generic_file_aio_read()").
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> The code is too complex to see if this is an obvious false positive.

Good catch; sadly, it *can* be triggered - read() starting past the EOF
will step into it.  Applied, will push today.

Re: [PATCH 1/2] cifs: Use min_t() when comparing "size_t" and "unsigned long"

[Jeff Layton]

On Sun, 13 Apr 2014 20:46:21 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> On 32 bit, size_t is "unsigned int", not "unsigned long", causing the
> following warning when comparing with PAGE_SIZE, which is always "unsigned
> long":
> 
> fs/cifs/file.c: In function ‘cifs_readdata_to_iov’:
> fs/cifs/file.c:2757: warning: comparison of distinct pointer types lacks a cast
> 
> Introduced by commit 7f25bba819a38ab7310024a9350655f374707e20
> ("cifs_iovec_read: keep iov_iter between the calls of
> cifs_readdata_to_iov()"), which changed the signedness of "remaining"
> and the code from min_t() to min().
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> PAGE_SIZE should really be size_t, but that would require lots of changes
> all over the place.
> 
>  fs/cifs/file.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 8807442c94dd..8add25538a3b 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -2754,7 +2754,7 @@ cifs_readdata_to_iov(struct cifs_readdata *rdata, struct iov_iter *iter)
>  
>  	for (i = 0; i < rdata->nr_pages; i++) {
>  		struct page *page = rdata->pages[i];
> -		size_t copy = min(remaining, PAGE_SIZE);
> +		size_t copy = min_t(size_t, remaining, PAGE_SIZE);
>  		size_t written = copy_page_to_iter(page, 0, copy, iter);
>  		remaining -= written;
>  		if (written < copy && iov_iter_count(iter) > 0)

Reviewed-by: Jeff Layton <jlayton@poochiereds.net>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/2] cifs: Use min_t() when comparing "size_t" and "unsigned long"

[Jeff Layton]

On Sun, 13 Apr 2014 20:46:21 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> On 32 bit, size_t is "unsigned int", not "unsigned long", causing the
> following warning when comparing with PAGE_SIZE, which is always "unsigned
> long":
> 
> fs/cifs/file.c: In function ‘cifs_readdata_to_iov’:
> fs/cifs/file.c:2757: warning: comparison of distinct pointer types lacks a cast
> 
> Introduced by commit 7f25bba819a38ab7310024a9350655f374707e20
> ("cifs_iovec_read: keep iov_iter between the calls of
> cifs_readdata_to_iov()"), which changed the signedness of "remaining"
> and the code from min_t() to min().
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> PAGE_SIZE should really be size_t, but that would require lots of changes
> all over the place.
> 
>  fs/cifs/file.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 8807442c94dd..8add25538a3b 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -2754,7 +2754,7 @@ cifs_readdata_to_iov(struct cifs_readdata *rdata, struct iov_iter *iter)
>  
>  	for (i = 0; i < rdata->nr_pages; i++) {
>  		struct page *page = rdata->pages[i];
> -		size_t copy = min(remaining, PAGE_SIZE);
> +		size_t copy = min_t(size_t, remaining, PAGE_SIZE);
>  		size_t written = copy_page_to_iter(page, 0, copy, iter);
>  		remaining -= written;
>  		if (written < copy && iov_iter_count(iter) > 0)

Reviewed-by: Jeff Layton <jlayton@poochiereds.net>