[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [cygwin] cwrapper emits wrapper script
From: |
Eric Blake |
Subject: |
Re: [cygwin] cwrapper emits wrapper script |
Date: |
Thu, 7 Jun 2007 17:19:58 +0000 (UTC) |
User-agent: |
Loom/3.14 (http://gmane.org/) |
Charles Wilson <libtool <at> cwilson.fastmail.fm> writes:
> Attached.
Some nits that you should fix, now that you have committed this.
> -/* -DDEBUG is fairly common in CFLAGS. */
> -#undef DEBUG
> +#undef LTWRAPPER_DEBUGPRINTF
> #if defined DEBUGWRAPPER
> -# define DEBUG(format, ...) fprintf(stderr, format, __VA_ARGS__)
> +# define LTWRAPPER_DEBUGPRINTF(format, ...) fprintf(stderr, format,
__VA_ARGS__)
Not your original issue, but __VA_ARGS__ is not C89. Sure, on cygwin, you are
relatively assured of gcc; but what about mingw with Microsofts' compiler? Or
are we assuming that nobody would define DEBUGWRAPPER unless they are
developing with gcc?
> +int
> +make_executable(const char * path)
> +{
> + int rval = 0;
> + struct stat st;
> +
> + /* MinGW & native WIN32 do not support S_IXOTH or S_IXGRP */
> + int S_XFLAGS =
> +#if defined (S_IXOTH)
> + S_IXOTH ||
> +#endif
> +#if defined (S_IXGRP)
> + S_IXGRP ||
> +#endif
> + S_IXUSR;
Code bug. You meant |, not || (but since on cygwin S_IXOTH is 1, and since
world execute privileges are adequate, it happened to work in spite of your
bug). IMHO, it looks nicer like this (note that my rewrite follows the GCS,
while yours left operators on the end of lines - in general, the coding style I
have seen from coreutils and gnulib prefers to factor out preprocesor
conditionals so that they need not appear in the middle of expressions):
#ifndef S_IXOTH
# define S_IXOTH 0
#endif
#ifndef S_IXGRP
# define S_IXGRP 0
#endif
int S_XFLAGS = S_IXOTH | S_IXGRP | S_IXUSR;
--
Eric Blake
- Re: [cygwin] cwrapper emits wrapper script, (continued)