On Mon, Sep 19, 2011 at 7:34 AM, Paul Eggert <egg...@cs.ucla.edu> 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.
>
>
>
