Re: [cygwin] cwrapper emits wrapper script

[Eric Blake-1]

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 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
#endif
#ifndef S_IXGRP
# define S_IXGRP 0
#endif

int S_XFLAGS = S_IXOTH | S_IXGRP | S_IXUSR;

-- 
Eric Blake


-- 
View this message in context: 
http://www.nabble.com/-cygwin--cwrapper-emits-wrapper-script-tf3621378.html#a11012253
Sent from the Gnu - Libtool - Patches mailing list archive at Nabble.com.