On Sun, Jan 12, 2020 at 2:20 AM Gregory Nutt <spudan...@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
>
>
