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

[Siddharth Vadapalli <s-vadapalli@ti.com>]

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.