libtool-patches
[Top][All Lists]
Advanced

[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







reply via email to

[Prev in Thread] Current Thread [Next in Thread]