Re: getting EBADF on MSVC

[Bastien ROUCARIES]

On Mon, Sep 19, 2011 at 7:34 AM, Paul Eggert <address@hidden> wrote:
> On 09/18/11 01:36, Bruno Haible wrote:
>
>  This patch protects the dup2() calls in gnulib.
>
> This change has some problems with GNU Emacs, and I suspect there will
> be similar problems with printf, close, etc. so let's try to get see
> if we can fix these problems with dup2 first.
>
> Emacs has a separate way of building on Windows, one that doesn't
> involve gnulib's replacement functions and is not likely to involve
> them any time in the near future.  I expect that Emacs developers will
> rather omit the files lib/msvc-inval.c, lib/msvc-inval.h, and
> m4/msvc-inval.m4 as part of the Emacs distribution, as they're useless
> baggage for Emacs.  However, I can't achieve this omission by invoking
> gnulib-tool --avoid=msvc-inval, because dup2.c now includes
> msvc-inval.h unilaterally.
>
> I'd like a solution where --avoid=msvc-inval works.
>
> Here's one way to do it.  In dup2.c:
>
> #if MSVC_HACK_NEEDED
> # include "msvc-inval.h"
> #else
> # define TRY_MSVC_INVAL if (1)
> # define CATCH_MSVC_INVAL else
> # define DONE_MSVC_INVAL
> #endif

why not
 #else
 # define TRY_MSVC_INVAL if (1)
 # define CATCH_MSVC_INVAL else do
 # define DONE_MSVC_INVAL while(0)
 #endif

> but this leads into the second problem, as discussed below.
>
>
>  > @@ -64,7 +78,18 @@
>  >    if (fd == desired_fd)
>  >      return fcntl (fd, F_GETFL) == -1 ? -1 : fd;
>  >  # endif
>  > -  result = dup2 (fd, desired_fd);
>  > +
>  > +  TRY_MSVC_INVAL
>  > +    {
>  > +      result = dup2 (fd, desired_fd);
>  > +    }
>  > +  CATCH_MSVC_INVAL
>  > +    {
>  > +      result = -1;
>  > +      errno = EBADF;
>  > +    }
>  > +  DONE_MSVC_INVAL
>
> This style is unsatisfying, because it propagates MSVC workarounds
> into the rest of gnulib.  I suspect it won't scale well when used
> throughout gnulib for other functions.  I'd rather have a solution
> that puts the MSVC workarounds in a separate place, where MSVC-aware
> programmers can work on them, a place which people who don't know or
> care about MSVC ports can omit and ignore.
>
> I also have some qualms about C macros that expand to series of tokens
> with unbalanced braces.

See my proposition will catch unbalanced brace used in kernel.

> How about something like this instead?  Have dup2.c look
> something like this:
>
>     [existing includes]
>
>     #if HAVE_DUP2
>     # undef dup2
>     # if MSVC_HACK_NEEDED              /* NEW */
>     #  include "msvc-hack.h"           /* NEW */
>     # endif                            /* NEW */
>
>     int rpl_dup2 (int fd, int desired_fd)
>     {
>       ...
>       result = dup2 (fd, desired_fd);
>       ...
>     }
>
> and then "msvc-hack.h" can #define dup2 to do whatever it likes.  This
> way, the source code to lib/dup2.c needs a relatively small change
> (the three lines marked "/* NEW */" above) and can be read by others
> largely without worrying about MSVC hacks.

I really dislike this

why not something like:
#if HAVE_DUP2
# undef dup2
# if MSVC_HACK_NEEDED              /* NEW */
#  include "msvc-hack.h"           /* NEW */
static int dup2_msvcsafe(int fd, int desired_fd)
{

}
# else
static inline dup2_msvcsafe(int fd, int desired_fd)
{
 return dup2();
}
# endif                            /* NEW */

int rpl_dup2 (int fd, int desired_fd)
   {
       ...
       result = dup2 (fd, desired_fd);
       ...
    }

Moreover _get_osfhandle (fd) should be render safe globally. May be we
should create a <io.h> module ?

Bastien
>
> Perhaps you can think of a better approach, but I hope you get
> the idea.
>
>
>