[PATCH 2/2] Quickly retry sendmsg or recvmsg if errno is EAGAIN or EINTR.

Max Woodbury mtewoodbury at gmail.com
Wed Feb 26 19:10:31 CST 2014


On 02/26/2014 06:36 PM, Erich E. Hoover wrote:
> On Wed, Feb 26, 2014 at 4:26 PM, Max Woodbury <mtewoodbury at gmail.com> wrote:
>> Please explain in words of one syllable why this has been rejected?
>
> Probably for the reasons Ken already mentioned:
> 1) Strange blank line after "if (n == -1)"
> 2) Unnecessary style change to K&R style ("if (ret >= 0) {")
> 3) "[PATCH 2/2]" with no "[PATCH 1/2]"
> 4) No explanation for his comment that EAGAIN is probably not needed.
> EINTR might want this behavior, but EGAIN should not.
> 5) If you bring this code "down" to where you have then you need to
> remove it from the "higher" level:
> http://source.winehq.org/source/dlls/ws2_32/socket.c#L4494
> http://source.winehq.org/source/dlls/ws2_32/socket.c#L6500
> 6) It should be in a loop
>
> Does that help?
>
> Best,
> Erich
>
>
So it's mainly on style:

-    I've been reading other peoples code for 50 years and that aspect
      of K&R is much easier to handle in my opinion.  Same with the
      extra blank line.  Other parts of wine use the style and are much
      easier to read as a result.

-    And the miscount can't simply be ignored?

If you read kernel code, you'll find that just returning to
application level is sometimes enough to free the missing resource.
That happens often enough that a single quick retry on EAGAIN is
warranted.

Bringing the relevant part of the retry code down and applying it to
only those parts where is has a strong likelihood of doing the most
good is exactly what the patch does.  If one retry is not enough, then
higher level strategy is needed so no changes are called for on the
higher level.  A loop at this level is _not_ a good idea since there
would have to be something to break the loop if it runs too long.

In other words, this fiddle is based on several decades of experience 
with kernel and bottom level application debugging experience.  At
least look at it and test it before rejecting it on superficial grounds.

max




More information about the wine-devel mailing list