[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cygwin-patches
Subject:    Re: [PATCH] Ensure that a blocking send() on a socket returns (with success) if a signal is handled 
From:       Erik Bray <erik.m.bray () gmail ! com>
Date:       2017-06-07 12:24:52
Message-ID: CAOTD34ayQcO4KXhmAPSJ949JYsYd9neOwtr0LDw=fumniFKXfg () mail ! gmail ! com
[Download RAW message or body]

On Tue, Jun 6, 2017 at 4:23 PM, Corinna Vinschen
<corinna-cygwin@cygwin.com> wrote:
> Hi Erik,
>
> [vacation-induced late reply]

No problem--you had been silent here for a while so I guessed you were
on vacation. Welcome back!

> On May 11 16:05, Erik M. Bray wrote:
>> ---
>>  winsup/cygwin/fhandler_socket.cc | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/winsup/cygwin/fhandler_socket.cc b/winsup/cygwin/fhandler_socket.cc
>> index f3d1d69..c7ed681 100644
>> --- a/winsup/cygwin/fhandler_socket.cc
>> +++ b/winsup/cygwin/fhandler_socket.cc
>> @@ -1851,7 +1851,7 @@ fhandler_socket::send_internal (struct _WSAMSG *wsamsg, int flags)
>>         if (get_socket_type () != SOCK_STREAM || ret < out_len)
>>           break;
>>       }
>> -      else if (is_nonblocking () || err != WSAEWOULDBLOCK)
>> +      else if (is_nonblocking () || WSAGetLastError() != WSAEWOULDBLOCK)
>>       break;
>>      }
>
> Thanks for catching!  Given that the loop isn't guaranteed to set `err'
> correctly all the time, I wonder if we shouldn't get rid of `err'
> completely.  Checking WSAGetLastError is plain user-space memory access
> anyway, and there's no reason the compiler can't optimize this by
> itself.

I agree, it can probably be removed entirely, likely resulting in clearer code.

> Also, I would prefer to have a shorter subject (<=72 chars or so) and
> to describe this in the log message a bit more detailed.
>
> Would you like to provide another patch along these lines?

Sure, no problem.

Erik
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic