[U-Boot] [RFC PATCH] ext4: Fix comparision of unsigned expression with < 0

In file ext4fs.c funtion ext4fs_read_file() compares an unsigned expression with < 0 like below
lbaint_t blknr; blknr = read_allocated_block(&(node->inode), i); if (blknr < 0) return -1;
blknr is of type ulong/uint64_t. read_allocated_block() returns long int. So comparing blknr with < 0 will always be false. Instead declare blknr as long int.
Reported-by: Sunita Nadampalli sunitan@ti.com Signed-off-by: Lokesh Vutla lokeshvutla@ti.com --- fs/ext4/ext4fs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c index 7187dcfb05..081509dbb4 100644 --- a/fs/ext4/ext4fs.c +++ b/fs/ext4/ext4fs.c @@ -71,7 +71,7 @@ int ext4fs_read_file(struct ext2fs_node *node, loff_t pos, blockcnt = lldiv(((len + pos) + blocksize - 1), blocksize);
for (i = lldiv(pos, blocksize); i < blockcnt; i++) { - lbaint_t blknr; + long int blknr; int blockoff = pos - (blocksize * i); int blockend = blocksize; int skipfirst = 0;

On Tue, Apr 25, 2017 at 10:22:27AM +0530, Lokesh Vutla wrote:
In file ext4fs.c funtion ext4fs_read_file() compares an unsigned expression with < 0 like below
lbaint_t blknr; blknr = read_allocated_block(&(node->inode), i); if (blknr < 0) return -1;
blknr is of type ulong/uint64_t. read_allocated_block() returns long int. So comparing blknr with < 0 will always be false. Instead declare blknr as long int.
Reported-by: Sunita Nadampalli sunitan@ti.com Signed-off-by: Lokesh Vutla lokeshvutla@ti.com
fs/ext4/ext4fs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c index 7187dcfb05..081509dbb4 100644 --- a/fs/ext4/ext4fs.c +++ b/fs/ext4/ext4fs.c @@ -71,7 +71,7 @@ int ext4fs_read_file(struct ext2fs_node *node, loff_t pos, blockcnt = lldiv(((len + pos) + blocksize - 1), blocksize);
for (i = lldiv(pos, blocksize); i < blockcnt; i++) {
lbaint_t blknr;
int blockoff = pos - (blocksize * i); int blockend = blocksize; int skipfirst = 0;long int blknr;
My only question is, did you catch that by inspection, clang, or a newer than gcc-6.3 warning? Also, fs/ext4/dev.c:63 is a similar problem, if you'd like to non-RFC a v2. Thanks!

On Tuesday 25 April 2017 11:34 PM, Tom Rini wrote:
On Tue, Apr 25, 2017 at 10:22:27AM +0530, Lokesh Vutla wrote:
In file ext4fs.c funtion ext4fs_read_file() compares an unsigned expression with < 0 like below
lbaint_t blknr; blknr = read_allocated_block(&(node->inode), i); if (blknr < 0) return -1;
blknr is of type ulong/uint64_t. read_allocated_block() returns long int. So comparing blknr with < 0 will always be false. Instead declare blknr as long int.
Reported-by: Sunita Nadampalli sunitan@ti.com Signed-off-by: Lokesh Vutla lokeshvutla@ti.com
fs/ext4/ext4fs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c index 7187dcfb05..081509dbb4 100644 --- a/fs/ext4/ext4fs.c +++ b/fs/ext4/ext4fs.c @@ -71,7 +71,7 @@ int ext4fs_read_file(struct ext2fs_node *node, loff_t pos, blockcnt = lldiv(((len + pos) + blocksize - 1), blocksize);
for (i = lldiv(pos, blocksize); i < blockcnt; i++) {
lbaint_t blknr;
int blockoff = pos - (blocksize * i); int blockend = blocksize; int skipfirst = 0;long int blknr;
My only question is, did you catch that by inspection, clang, or a newer
This is reported by Sunita and not sure how she caught it.
than gcc-6.3 warning? Also, fs/ext4/dev.c:63 is a similar problem, if you'd like to non-RFC a v2. Thanks!
In this case do you want me to drop the check or change lbaint_t to long int and update the argument type cast where ever ext4fs_devread() is called?
Thanks and regards, Lokesh

On Wed, Apr 26, 2017 at 08:56:49AM +0530, Lokesh Vutla wrote:
On Tuesday 25 April 2017 11:34 PM, Tom Rini wrote:
On Tue, Apr 25, 2017 at 10:22:27AM +0530, Lokesh Vutla wrote:
In file ext4fs.c funtion ext4fs_read_file() compares an unsigned expression with < 0 like below
lbaint_t blknr; blknr = read_allocated_block(&(node->inode), i); if (blknr < 0) return -1;
blknr is of type ulong/uint64_t. read_allocated_block() returns long int. So comparing blknr with < 0 will always be false. Instead declare blknr as long int.
Reported-by: Sunita Nadampalli sunitan@ti.com Signed-off-by: Lokesh Vutla lokeshvutla@ti.com
fs/ext4/ext4fs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c index 7187dcfb05..081509dbb4 100644 --- a/fs/ext4/ext4fs.c +++ b/fs/ext4/ext4fs.c @@ -71,7 +71,7 @@ int ext4fs_read_file(struct ext2fs_node *node, loff_t pos, blockcnt = lldiv(((len + pos) + blocksize - 1), blocksize);
for (i = lldiv(pos, blocksize); i < blockcnt; i++) {
lbaint_t blknr;
int blockoff = pos - (blocksize * i); int blockend = blocksize; int skipfirst = 0;long int blknr;
My only question is, did you catch that by inspection, clang, or a newer
This is reported by Sunita and not sure how she caught it.
OK. Because if she's got clang used on a TI platform and linking and booting, I'd very much like to hear what else is required, or if nothing, what version of the tools she's using, so I can put that into my CI loop. I didn't have luck when I tried last, but it was a while ago.
than gcc-6.3 warning? Also, fs/ext4/dev.c:63 is a similar problem, if you'd like to non-RFC a v2. Thanks!
In this case do you want me to drop the check or change lbaint_t to long int and update the argument type cast where ever ext4fs_devread() is called?
Just drop it I think, thanks.

On Wednesday 26 April 2017 04:54 PM, Tom Rini wrote:
On Wed, Apr 26, 2017 at 08:56:49AM +0530, Lokesh Vutla wrote:
On Tuesday 25 April 2017 11:34 PM, Tom Rini wrote:
On Tue, Apr 25, 2017 at 10:22:27AM +0530, Lokesh Vutla wrote:
In file ext4fs.c funtion ext4fs_read_file() compares an unsigned expression with < 0 like below
lbaint_t blknr; blknr = read_allocated_block(&(node->inode), i); if (blknr < 0) return -1;
blknr is of type ulong/uint64_t. read_allocated_block() returns long int. So comparing blknr with < 0 will always be false. Instead declare blknr as long int.
Reported-by: Sunita Nadampalli sunitan@ti.com Signed-off-by: Lokesh Vutla lokeshvutla@ti.com
fs/ext4/ext4fs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c index 7187dcfb05..081509dbb4 100644 --- a/fs/ext4/ext4fs.c +++ b/fs/ext4/ext4fs.c @@ -71,7 +71,7 @@ int ext4fs_read_file(struct ext2fs_node *node, loff_t pos, blockcnt = lldiv(((len + pos) + blocksize - 1), blocksize);
for (i = lldiv(pos, blocksize); i < blockcnt; i++) {
lbaint_t blknr;
int blockoff = pos - (blocksize * i); int blockend = blocksize; int skipfirst = 0;long int blknr;
My only question is, did you catch that by inspection, clang, or a newer
This is reported by Sunita and not sure how she caught it.
OK. Because if she's got clang used on a TI platform and linking and booting, I'd very much like to hear what else is required, or if nothing, what version of the tools she's using, so I can put that into my CI loop. I didn't have luck when I tried last, but it was a while ago.
Sure, Ill update you on this with more details.
Thanks and regards, Lokesh
than gcc-6.3 warning? Also, fs/ext4/dev.c:63 is a similar problem, if you'd like to non-RFC a v2. Thanks!
In this case do you want me to drop the check or change lbaint_t to long int and update the argument type cast where ever ext4fs_devread() is called?
Just drop it I think, thanks.
participants (2)
-
Lokesh Vutla
-
Tom Rini