[PATCH] net: Fix the displayed value of bytes transferred

[PATCH] net: Fix the displayed value of bytes transferred

[Siddharth Vadapalli]

In the case of NETLOOP_SUCCESS, the decimal value of the u32 variable
"net_boot_file_size" is printed using "%d", resulting in negative values
being reported for large file sizes. Fix this by using "%lu" to print
the decimal value corresponding to the bytes transferred.

Fixes: 1411157d8578 ("net: cosmetic: Fixup var names related to boot file")
Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 net/net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/net.c b/net/net.c
index 43abbac7c3..7aaeafc247 100644
--- a/net/net.c
+++ b/net/net.c
@@ -716,7 +716,7 @@ restart:
 		case NETLOOP_SUCCESS:
 			net_cleanup_loop();
 			if (net_boot_file_size > 0) {
-				printf("Bytes transferred = %d (%x hex)\n",
+				printf("Bytes transferred = %lu (%x hex)\n",
 				       net_boot_file_size, net_boot_file_size);
 				env_set_hex("filesize", net_boot_file_size);
 				env_set_hex("fileaddr", image_load_addr);
-- 
2.34.1

Re: [PATCH] net: Fix the displayed value of bytes transferred

[Ravi Gunasekaran]

Siddharth,

On 8/10/23 2:45 PM, Siddharth Vadapalli wrote:
> In the case of NETLOOP_SUCCESS, the decimal value of the u32 variable
> "net_boot_file_size" is printed using "%d", resulting in negative values
> being reported for large file sizes. Fix this by using "%lu" to print
> the decimal value corresponding to the bytes transferred.
> 
> Fixes: 1411157d8578 ("net: cosmetic: Fixup var names related to boot file")
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> ---
>  net/net.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/net.c b/net/net.c
> index 43abbac7c3..7aaeafc247 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -716,7 +716,7 @@ restart:
>  		case NETLOOP_SUCCESS:
>  			net_cleanup_loop();
>  			if (net_boot_file_size > 0) {
> -				printf("Bytes transferred = %d (%x hex)\n",
> +				printf("Bytes transferred = %lu (%x hex)\n",

'net_boot_file_size' is of type u32. Using "%lu" will throw a warning for this.
As per [0], format specifier for 'unsigned int' is "%d, %x'.

You could perhaps change the data type of 'net_boot_file_size' to 'ulong' as well.

[0] - https://u-boot.readthedocs.io/en/latest/develop/printf.html

>  				       net_boot_file_size, net_boot_file_size);
>  				env_set_hex("filesize", net_boot_file_size);
>  				env_set_hex("fileaddr", image_load_addr);

-- 
Regards,
Ravi

Re: [PATCH] net: Fix the displayed value of bytes transferred

[Siddharth Vadapalli]

Ravi,

On 10/08/23 17:00, Ravi Gunasekaran wrote:
> Siddharth,
> 
> On 8/10/23 2:45 PM, Siddharth Vadapalli wrote:
>> In the case of NETLOOP_SUCCESS, the decimal value of the u32 variable
>> "net_boot_file_size" is printed using "%d", resulting in negative values
>> being reported for large file sizes. Fix this by using "%lu" to print
>> the decimal value corresponding to the bytes transferred.
>>
>> Fixes: 1411157d8578 ("net: cosmetic: Fixup var names related to boot file")
>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>> ---
>>  net/net.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/net.c b/net/net.c
>> index 43abbac7c3..7aaeafc247 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -716,7 +716,7 @@ restart:
>>  		case NETLOOP_SUCCESS:
>>  			net_cleanup_loop();
>>  			if (net_boot_file_size > 0) {
>> -				printf("Bytes transferred = %d (%x hex)\n",
>> +				printf("Bytes transferred = %lu (%x hex)\n",
> 
> 'net_boot_file_size' is of type u32. Using "%lu" will throw a warning for this.
> As per [0], format specifier for 'unsigned int' is "%d, %x'.
> 
> You could perhaps change the data type of 'net_boot_file_size' to 'ulong' as well.

The issue here isn't the size of the variable itself, but the format specifier.
For large file sizes, the hex value printed for the variable is correct, but the
decimal value is negative.

> 
> [0] - https://u-boot.readthedocs.io/en/latest/develop/printf.html
> 
>>  				       net_boot_file_size, net_boot_file_size);
>>  				env_set_hex("filesize", net_boot_file_size);
>>  				env_set_hex("fileaddr", image_load_addr);
> 

-- 
Regards,
Siddharth.

Re: [PATCH] net: Fix the displayed value of bytes transferred

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1992 bytes --]

On Fri, Aug 11, 2023 at 10:49:23AM +0530, Siddharth Vadapalli wrote:
> Ravi,
> 
> On 10/08/23 17:00, Ravi Gunasekaran wrote:
> > Siddharth,
> > 
> > On 8/10/23 2:45 PM, Siddharth Vadapalli wrote:
> >> In the case of NETLOOP_SUCCESS, the decimal value of the u32 variable
> >> "net_boot_file_size" is printed using "%d", resulting in negative values
> >> being reported for large file sizes. Fix this by using "%lu" to print
> >> the decimal value corresponding to the bytes transferred.
> >>
> >> Fixes: 1411157d8578 ("net: cosmetic: Fixup var names related to boot file")
> >> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> >> ---
> >>  net/net.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/net/net.c b/net/net.c
> >> index 43abbac7c3..7aaeafc247 100644
> >> --- a/net/net.c
> >> +++ b/net/net.c
> >> @@ -716,7 +716,7 @@ restart:
> >>  		case NETLOOP_SUCCESS:
> >>  			net_cleanup_loop();
> >>  			if (net_boot_file_size > 0) {
> >> -				printf("Bytes transferred = %d (%x hex)\n",
> >> +				printf("Bytes transferred = %lu (%x hex)\n",
> > 
> > 'net_boot_file_size' is of type u32. Using "%lu" will throw a warning for this.
> > As per [0], format specifier for 'unsigned int' is "%d, %x'.
> > 
> > You could perhaps change the data type of 'net_boot_file_size' to 'ulong' as well.
> 
> The issue here isn't the size of the variable itself, but the format specifier.
> For large file sizes, the hex value printed for the variable is correct, but the
> decimal value is negative.
> 
> > 
> > [0] - https://u-boot.readthedocs.io/en/latest/develop/printf.html

Uh, maybe I'm just missing something, but I think there's two things.
First, this should be "%u" for "unsigned decimal".  Second,
doc/develop/printf.rst needs to be fixed since:
int                 %d, %x
unsigned int        %d, %x

Should is wrong and should say %u, %x, because, well, that's what would
be correct, yes?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] net: Fix the displayed value of bytes transferred

[Siddharth Vadapalli]

Hello Tom,

On 11/08/23 21:45, Tom Rini wrote:
> On Fri, Aug 11, 2023 at 10:49:23AM +0530, Siddharth Vadapalli wrote:
>> Ravi,
>>
>> On 10/08/23 17:00, Ravi Gunasekaran wrote:
>>> Siddharth,
>>>
>>> On 8/10/23 2:45 PM, Siddharth Vadapalli wrote:
>>>> In the case of NETLOOP_SUCCESS, the decimal value of the u32 variable
>>>> "net_boot_file_size" is printed using "%d", resulting in negative values
>>>> being reported for large file sizes. Fix this by using "%lu" to print
>>>> the decimal value corresponding to the bytes transferred.
>>>>
>>>> Fixes: 1411157d8578 ("net: cosmetic: Fixup var names related to boot file")
>>>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>>>> ---
>>>>  net/net.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/net/net.c b/net/net.c
>>>> index 43abbac7c3..7aaeafc247 100644
>>>> --- a/net/net.c
>>>> +++ b/net/net.c
>>>> @@ -716,7 +716,7 @@ restart:
>>>>  		case NETLOOP_SUCCESS:
>>>>  			net_cleanup_loop();
>>>>  			if (net_boot_file_size > 0) {
>>>> -				printf("Bytes transferred = %d (%x hex)\n",
>>>> +				printf("Bytes transferred = %lu (%x hex)\n",
>>>
>>> 'net_boot_file_size' is of type u32. Using "%lu" will throw a warning for this.
>>> As per [0], format specifier for 'unsigned int' is "%d, %x'.
>>>
>>> You could perhaps change the data type of 'net_boot_file_size' to 'ulong' as well.
>>
>> The issue here isn't the size of the variable itself, but the format specifier.
>> For large file sizes, the hex value printed for the variable is correct, but the
>> decimal value is negative.
>>
>>>
>>> [0] - https://u-boot.readthedocs.io/en/latest/develop/printf.html
> 
> Uh, maybe I'm just missing something, but I think there's two things.
> First, this should be "%u" for "unsigned decimal".  Second,
> doc/develop/printf.rst needs to be fixed since:
> int                 %d, %x
> unsigned int        %d, %x
> 
> Should is wrong and should say %u, %x, because, well, that's what would
> be correct, yes?

Thank you for reviewing the patch. Yes, %u works well and can print u32 variable
accurately. I tested it for 0xffffffff. %d prints -1 for the same. So, %lu isn't
necessary and %u is sufficient. I will replace %lu with %u and post the v2
patch. Additionally, I will include a patch in the v2 series to update the
Documentation as pointed out by you.

> 

-- 
Regards,
Siddharth.

Re: [PATCH] net: Fix the displayed value of bytes transferred

[Siddharth Vadapalli]

On 14/08/23 10:06, Siddharth Vadapalli wrote:
> Hello Tom,
> 
> On 11/08/23 21:45, Tom Rini wrote:
>> On Fri, Aug 11, 2023 at 10:49:23AM +0530, Siddharth Vadapalli wrote:
>>> Ravi,
>>>
>>> On 10/08/23 17:00, Ravi Gunasekaran wrote:

...

>>
>> Uh, maybe I'm just missing something, but I think there's two things.
>> First, this should be "%u" for "unsigned decimal".  Second,
>> doc/develop/printf.rst needs to be fixed since:
>> int                 %d, %x
>> unsigned int        %d, %x
>>
>> Should is wrong and should say %u, %x, because, well, that's what would
>> be correct, yes?
> 
> Thank you for reviewing the patch. Yes, %u works well and can print u32 variable
> accurately. I tested it for 0xffffffff. %d prints -1 for the same. So, %lu isn't
> necessary and %u is sufficient. I will replace %lu with %u and post the v2
> patch. Additionally, I will include a patch in the v2 series to update the
> Documentation as pointed out by you.

I have posted the v2 series at:
https://patchwork.ozlabs.org/project/uboot/list/?series=368661&state=%2A&archive=both

> 
>>
> 

-- 
Regards,
Siddharth.