Re: another dup2 mingw failure

2010-01-11 Thread Simon Josefsson
Eric Blake e...@byu.net 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




Re: another dup2 mingw failure

2010-01-09 Thread Eric Blake
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.

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.

 +
  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?

 +  /* 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.

-- 
Don't work too hard, make some time for fun as well!

Eric Blake e...@byu.net



signature.asc
Description: OpenPGP digital signature