Kirkwood: Fix tv sec/usec normalization in kwboot

Kirkwood: Fix tv sec/usec normalization in kwboot

[Stefan Roese]

On 27.04.21 21:57, Dagan Martinez wrote:
>  From 328c559acf6872bf0cbafe7fbb881748d5c753fb Mon Sep 17 00:00:00 2001
> From: Property404 <dmartinez@starry.com>
> Date: Tue, 27 Apr 2021 15:48:31 -0400
> Subject: [PATCH] Kirkwood: Fix tv sec/usec normalization in kwboot
> 
> `kwboot.c` had an issue where it failed to normalize the `tv` struct in
> the case where the `tv_usec` field was 1000000, ie one second.
> 
> This caused issues on Fedora Linux 34, where `select` would return
> `EINVAL`, preventing kwboot from communicating with the board.
> 
> Signed-off-by: Property404 <dmartinez@starry.com>
> ---
>   tools/kwboot.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index 4be094c9c8..5d5d501d36 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -167,7 +167,7 @@ kwboot_tty_recv(int fd, void *buf, size_t len, int timeo)
> 
>       tv.tv_sec = 0;
>       tv.tv_usec = timeo * 1000;
> -    if (tv.tv_usec > 1000000) {
> +    if (tv.tv_usec >= 1000000) {
>           tv.tv_sec += tv.tv_usec / 1000000;
>           tv.tv_usec %= 1000000;
>       }
> 

Reviewed-by: Stefan Roese <sr@denx.de>

BTW: I'm not 100% sure, but my understanding is that it is preferred (or
even required) to provide a "real name" in the tags. So it might be that
"Property404" is not enough here.

Thanks,
Stefan

Kirkwood: Fix tv sec/usec normalization in kwboot

[Dagan Martinez]

Kirkwood: Fix tv sec/usec normalization in kwboot

[Stefan Roese]

On 28.04.21 13:24, Dagan Martinez wrote:
>  From a45340719110b8a8b5292f6353fda7509be81417 Mon Sep 17 00:00:00 2001
> From: Property404 <dmartinez@starry.com>
> Date: Tue, 27 Apr 2021 15:48:31 -0400
> Subject: [PATCH] Kirkwood: Fix tv sec/usec normalization in kwboot
> 
> `kwboot.c` had an issue where it failed to normalize the `tv` struct in
> the case where the `tv_usec` field was 1000000, ie one second.
> 
> This caused issues on Fedora Linux 34, where `select` would return
> `EINVAL`, preventing kwboot from communicating with the board.
> 
> Signed-off-by: Dagan Martinez <dmartinez@starry.com>

Thanks for re-submitting. I do have some more change requests - sorry
for being a bit strict here.

First, please change the subject to not reply on a message but also
mention the patch version in the subject (v2). And also please add
the patch history below the commit text. See here for more details
on submitting patch versions:

http://www.denx.de/wiki/view/U-Boot/Patches#Sending_updated_patch_versions

And also please change your "From:" line, so that it does not mention
"Property404" any more.

BTW: "git send-email" is very helpful for sending patches.

Thanks,
Stefan

> ---
>   tools/kwboot.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index 4be094c9c8..5d5d501d36 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -167,7 +167,7 @@ kwboot_tty_recv(int fd, void *buf, size_t len, int timeo)
> 
>       tv.tv_sec = 0;
>       tv.tv_usec = timeo * 1000;
> -    if (tv.tv_usec > 1000000) {
> +    if (tv.tv_usec >= 1000000) {
>           tv.tv_sec += tv.tv_usec / 1000000;
>           tv.tv_usec %= 1000000;
>       }
> 

Kirkwood: Fix tv sec/usec normalization in kwboot

[Dagan Martinez]

Kirkwood: Fix tv sec/usec normalization in kwboot

[Stefan Roese]

On 29.04.21 14:03, Dagan Martinez wrote:
>  From c200095d4136a071dd9526a48be642ce58fae8c9 Mon Sep 17 00:00:00 2001
> From: Dagan Martinez <dmartinez@starry.com>
> Date: Tue, 27 Apr 2021 15:48:31 -0400
> Subject: [PATCH v2] Kirkwood: Fix tv sec/usec normalization in kwboot
> 
> `kwboot.c` had an issue where it failed to normalize the `tv` struct in
> the case where the `tv_usec` field was 1000000, ie one second.
> 
> This caused issues on Fedora Linux 34, where `select` would return
> `EINVAL`, preventing kwboot from communicating with the board.
> 
> Signed-off-by: Dagan Martinez <dmartinez@starry.com>
> ---
> Changes for v2:
>     - Use real author name in patch

Thanks. Still this patch does not apply. How did you generate and send
it? It's somehow mangled. I suggest to use git send-email.

And another issue: The subject of this original mail still has "Re: " in
it. Please don't reply in some mail client but either use git send-email
or write a completely new message. Some mail clients tend to clutter
the whitespace etc.

Thanks,
Stefan

> 
>   tools/kwboot.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index 4be094c9c8..5d5d501d36 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -167,7 +167,7 @@ kwboot_tty_recv(int fd, void *buf, size_t len, int timeo)
> 
>       tv.tv_sec = 0;
>       tv.tv_usec = timeo * 1000;
> -    if (tv.tv_usec > 1000000) {
> +    if (tv.tv_usec >= 1000000) {
>           tv.tv_sec += tv.tv_usec / 1000000;
>           tv.tv_usec %= 1000000;
>       }
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de