[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [cygwin] cwrapper emits wrapper script

From: Eric Blake-1
Subject: Re: [cygwin] cwrapper emits wrapper script
Date: Thu, 7 Jun 2007 10:26:35 -0700 (PDT)

Charles Wilson <libtool <at>> writes:

> Attached.

Some nits that you should fix, now that you have committed this.

> -/* -DDEBUG is fairly common in CFLAGS.  */
> -#undef DEBUG
>  #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 preprocessing __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
#ifndef S_IXGRP
# define S_IXGRP 0


Eric Blake

View this message in context:
Sent from the Gnu - Libtool - Patches mailing list archive at

reply via email to

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