Re: [PATCH] net: Fix the displayed value of bytes transferred
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 0x. %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=%2A=both > >> > -- Regards, Siddharth.
Re: [PATCH] net: Fix the displayed value of bytes transferred
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
---
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 0x. %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
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
> >> ---
> >> 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
signature.asc
Description: PGP signature
Re: [PATCH] net: Fix the displayed value of bytes transferred
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
>> ---
>> 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
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
> ---
> 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
[PATCH] net: Fix the displayed value of bytes transferred
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
---
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
