Subject: Re: [PATCH v3 01/14] mingw: add network-wrappers for daemon

From: Eric Sunshine <[email-masked]>
Date: Sun, 10 Oct 2010 17:19:09 -0400

On 10/10/2010 4:20 PM, Erik Faye-Lund wrote:
> On Sun, Oct 10, 2010 at 9:40 PM, Eric Sunshine<[email-masked]>  wrote:
>> On 10/10/2010 9:20 AM, Erik Faye-Lund wrote:
>>>
>>> From: Mike Pape<[email-masked]>
>>>
>>> git-daemon requires some socket-functionality that is not yet
>>> supported in the Windows-port. This patch adds said functionality,
>>> and makes sure WSAStartup gets called by socket(), since it is the
>>> first network-call in git-daemon. In addition, a check is added to
>>> prevent WSAStartup (and WSACleanup, though atexit) from being
>>> called more than once, since git-daemon calls both socket() and
>>> gethostbyname().
>>>
>>> Signed-off-by: Mike Pape<[email-masked]>
>>> Signed-off-by: Erik Faye-Lund<[email-masked]>
>>> ---
>>> diff --git a/compat/mingw.c b/compat/mingw.c
>>> index 6590f33..563ef1f 100644
>>> --- a/compat/mingw.c
>>> +++ b/compat/mingw.c
>>> +#undef accept
>>> +int mingw_accept(int sockfd1, struct sockaddr *sa, socklen_t *sz)
>>> +{
>>> +       int sockfd2;
>>> +
>>> +       SOCKET s1 = (SOCKET)_get_osfhandle(sockfd1);
>>> +       SOCKET s2 = accept(s1, sa, sz);
>>> +
>>> +       /* convert into a file descriptor */
>>> +       if ((sockfd2 = _open_osfhandle(s2, O_RDWR|O_BINARY))<    0) {
>>> +               closesocket(s2);
>>> +               return error("unable to make a socket file descriptor:
>>> %s",
>>> +                       strerror(errno));
>>
>> Is 'errno' from _open_osfhandle() still valid when handed to strerror() or
>> has it been clobbered by closesocket()?
>>
>> Corollary: Does _open_osfhandle() indeed set 'errno', or is it more
>> appropriate to call WSAGetLastError()? (The documentation I read for
>> _open_osfhandle() did not say anything about how to determine the reason for
>> failure.)
>>
>
> _open_osfhandle seems to set both errno and the winsock-error.
> closesocket() sets the winsock-error but not the CRT. I've just tested
> with a very simple application:
> So, it seems that WSAGetLastError() gets clobbered by closesocket(),
> but not errno.

Thank you for checking. Even if it is not strictly needed in this case, 
for the sake of clarity (and to avoid having this question repeated in 
the future), it might be worthwhile to save 'errno' or the result of 
WSAGetLastError() in a temporary before invoking closesocket(), and then 
pass the temporary to strerror().

-- ES