You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nuttx.apache.org by Gregory Nutt <sp...@gmail.com> on 2020/01/11 18:05:24 UTC
PR85 Question for Xiao Xiang
I fixed this section of code because it had ETIMEOUT instead of
ETIMEDOUT and so caused a compilation failure.
Looking more carefully, net_timedwait() will return -ETIMEDOUT on the
timeout. So shouldn't the condition be (ret == -ETIMEDOUT && acked ==
state.snd_acked). I will change the code that way but I really need
your input.
701 for (; ; )
702 {
703 uint32_t acked = state.snd_acked;
704
705 ret = net_timedwait(&state.snd_sem,
706 _SO_TIMEOUT(psock->s_sndtimeo));
707 if (ret != -ETIMEDOUT || acked == state.snd_acked)
708 {
709 break; /* Timeout without any progress */
710 }
711 }
Greg
Re: PR85 Question for Xiao Xiang
Posted by Xiang Xiao <xi...@gmail.com>.
On Sun, Jan 12, 2020 at 2:20 AM Gregory Nutt <sp...@gmail.com> wrote:
>
>
> > I fixed this section of code because it had ETIMEOUT instead of
> > ETIMEDOUT and so caused a compilation failure.
> >
> > Looking more carefully, net_timedwait() will return -ETIMEDOUT on the
> > timeout. So shouldn't the condition be (ret == -ETIMEDOUT && acked ==
> > state.snd_acked). I will change the code that way but I really need
> > your input.
> >
> > 701 for (; ; )
> > 702 {
> > 703 uint32_t acked = state.snd_acked;
> > 704
> > 705 ret = net_timedwait(&state.snd_sem,
> > 706 _SO_TIMEOUT(psock->s_sndtimeo));
> > 707 if (ret != -ETIMEDOUT || acked == state.snd_acked)
> > 708 {
> > 709 break; /* Timeout without any progress */
> > 710 }
> > 711 }
> >
> No, my suggested change is not right. But I don't understand the loop
> termination condition either. There is something wrong. Wouldn't ret
> always be -ETIMEDOUT if acked == state.snd_acked? The comment is wrong
> in any case.
>
The condition is good, but the comment just describe the one exit
criteria which make the confusion:
1.Exit if ret != -ETIMEDOUT which mean that the transfer is either
completed(OK) or interrupted(-EINTR)
2.Exit if ret == -ETIMEDOUT && acked == state.snd_acked which mean the
transfer stuck for a while
The only condition to continue the loop is:
ret == -ETIMEDOUT && acked != state.snd_acked
> We will merge PR85 with only the the change to the spelling of
> ETIMEDOUT, but could you please review the condition for exiting the
> loop (and whatever that should be, make it match the comments?).
>
> Greg
>
>
Re: PR85 Question for Xiao Xiang
Posted by Gregory Nutt <sp...@gmail.com>.
> I fixed this section of code because it had ETIMEOUT instead of
> ETIMEDOUT and so caused a compilation failure.
>
> Looking more carefully, net_timedwait() will return -ETIMEDOUT on the
> timeout. So shouldn't the condition be (ret == -ETIMEDOUT && acked ==
> state.snd_acked). I will change the code that way but I really need
> your input.
>
> 701 for (; ; )
> 702 {
> 703 uint32_t acked = state.snd_acked;
> 704
> 705 ret = net_timedwait(&state.snd_sem,
> 706 _SO_TIMEOUT(psock->s_sndtimeo));
> 707 if (ret != -ETIMEDOUT || acked == state.snd_acked)
> 708 {
> 709 break; /* Timeout without any progress */
> 710 }
> 711 }
>
No, my suggested change is not right. But I don't understand the loop
termination condition either. There is something wrong. Wouldn't ret
always be -ETIMEDOUT if acked == state.snd_acked? The comment is wrong
in any case.
We will merge PR85 with only the the change to the spelling of
ETIMEDOUT, but could you please review the condition for exiting the
loop (and whatever that should be, make it match the comments?).
Greg