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

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

[Lokesh Vutla]

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

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

[Tom Rini]

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;
> +		long int blknr;
>  		int blockoff = pos - (blocksize * i);
>  		int blockend = blocksize;
>  		int skipfirst = 0;

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170425/ab14bb56/attachment.sig>

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

[Lokesh Vutla]

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;
>> +		long int blknr;
>>  		int blockoff = pos - (blocksize * i);
>>  		int blockend = blocksize;
>>  		int skipfirst = 0;
> 
> 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

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

[Tom Rini]

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;
> >> +		long int blknr;
> >>  		int blockoff = pos - (blocksize * i);
> >>  		int blockend = blocksize;
> >>  		int skipfirst = 0;
> > 
> > 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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170426/aaed3226/attachment.sig>

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

[Lokesh Vutla]

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;
>>>> +		long int blknr;
>>>>  		int blockoff = pos - (blocksize * i);
>>>>  		int blockend = blocksize;
>>>>  		int skipfirst = 0;
>>>
>>> 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.
>