Re: another dup2 mingw failure

[Simon Josefsson]

Eric Blake <address@hidden> writes:

> According to Simon Josefsson on 1/9/2010 4:06 AM:
>> There is another dup2 failure due to Wine, see:
>> 
>> http://bugs.winehq.org/show_bug.cgi?id=21291
>> 
>> The patch below works around it.  Thoughts?
>
> Hmm.  Repeatedly adding workarounds for wine bugs seems awkward.  If the
> goal is making wine a windows emulator, it seems like we are better off
> giving the wine project a little while to fix their bug, rather than
> making a permanent workaround in gnulib that will be obsoleted as soon as
> wine is updated.  It's bad enough that mingw is already awkward enough to
> port to, without having to also deal with porting to wine bugs.

I understand this, but supporting Wine as a platform makes it easy to
check if GNU packages work under Windows easily (just build them using a
mingw cross-compiler and run self-tests under Wine).

I suspect Wine is a more common (and important) operating environment
compared to some of the more exotic systems that we support in gnulib.
Having support for it results in GNU packages working better on Windows,
and consequently more Windows people will be exposed to free software,
which seems like a good thing.

> That said, here's a review of your proposed patch:
>
>>  
>> +#if !O_BINARY
>> +# define setmode(f,m) zero ()
>> +static int zero (void) { return 0; }
>> +#endif
>
> That snippet is fine for unit tests, but for actual modules, it is better
> to depend on binary-io and use the setmode declaration from "binary-io.h".
>  I'm assuming that the use of zero is a spurious copy-n-paste.  That, and
> since all your uses of setmode() are already guarded by _WIN32, you don't
> really need to worry about #defining setmode on platforms where O_BINARY is 0.

Right.

>> +
>>  int
>>  rpl_dup2 (int fd, int desired_fd)
>>  {
>>    int result;
>>  # if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
>> +  int fd_mode = -1;
>>    /* If fd is closed, mingw hangs on dup2 (fd, fd).  If fd is open,
>>       dup2 (fd, fd) returns 0, but all further attempts to use fd in
>>       future dup2 calls will hang.  */
>> @@ -59,6 +65,14 @@ rpl_dup2 (int fd, int desired_fd)
>>        errno = EBADF;
>>        return -1;
>>      }
>> +  /* Wine 1.0.1 puts desired_fd into binary mode when fd is in text
>> +     mode, so we save the old mode here.
>> +     http://bugs.winehq.org/show_bug.cgi?id=21291 */
>> +  if ((HANDLE) _get_osfhandle (fd) != (HANDLE) -1)
>> +    {
>> +      fd_mode = setmode (fd, O_BINARY);
>> +      setmode (fd, fd_mode);
>> +    }
>>  # endif
>>    result = dup2 (fd, desired_fd);
>>  # ifdef __linux__
>> @@ -80,6 +94,12 @@ rpl_dup2 (int fd, int desired_fd)
>>    if (fd != desired_fd && result != -1)
>>      result = _gl_register_dup (fd, result);
>>  # endif
>> +# if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
>
> Is there a wine-specific preprocessor witness that we can use to
> distinguish compilation for wine vs. native?

That's not possible, the same EXE file should work on both Wine and
native Windows.

>> +  /* Restore text mode if needed.
>> +     http://bugs.winehq.org/show_bug.cgi?id=21291 */
>> +  if (result != -1 && fd_mode != -1)
>> +    setmode (desired_fd, fd_mode);
>
> Yes, this looks like it would resolve the problem, logic wise.  It's now
> just a question of policy, whether we want to apply it.

Right.

/Simon