Re: Minor build fixes

[Ken Brown]

On 10/2/2014 4:37 PM, Gavin Smith wrote:
> > 1. The compiler produces lots of warnings about setmode being redefined.  It
> > would be simple to silence the warnings, but the real issue in my opinion is
> > that Cygwin should be treated the same as Unix, so that there's no need for
> > setmode at all:
> >
> > Index: system.h
> > ===================================================================
> > --- system.h    (revision 5852)
> > +++ system.h    (working copy)
> > @@ -119,6 +119,13 @@
> >       - text files can have their lines ended either with \n or with \r\n
> > pairs;
> >      These are all parameterized here except the last, which is
> >      handled by the source code as appropriate (mostly, in info/).  */
> > +
> > +/* Cygwin is a Posix system even though it allows the use of O_BINARY.  */
> > +#ifdef __CYGWIN__
> > +# undef O_BINARY
> > +# define O_BINARY 0
> > +#endif
> > +
> >   #ifndef O_BINARY
> >   # ifdef _O_BINARY
> >   #  define O_BINARY _O_BINARY
> > @@ -152,23 +159,13 @@
> >   #  define HAVE_LONG_FILENAMES(dir)   (1)
> >   #  define NULL_DEVICE  "NUL"
> >   # endif  /* O_BINARY && !__MSDOS__ */
> > -# ifdef __CYGWIN__
> > -#  define DEFAULT_TMPDIR       "/tmp/"
> > -#  define PATH_SEP     ":"
> > -#  define STRIP_DOT_EXE        0
> > -#  undef NULL_DEVICE
> > -#  define NULL_DEVICE "/dev/null"
> > -#  define PIPE_USE_FORK        1
> > -# else  /* O_BINARY && !__CYGWIN__ */
> > -#  ifdef __MINGW32__
> > -#   define SET_SCREEN_SIZE_HELPER terminal_prep_terminal()
> > -#  endif  /* _WIN32 */
> > -#  define DEFAULT_TMPDIR       "c:/"
> > -#  define PATH_SEP     ";"
> > -#  define STRIP_DOT_EXE        1
> > -#  define PIPE_USE_FORK        0
> > -# endif /* O_BINARY && !__CYGWIN__ */
> > -  /* Back to any O_BINARY system.  */
> > +# ifdef __MINGW32__
> > +#  define SET_SCREEN_SIZE_HELPER terminal_prep_terminal()
> > +# endif  /* _WIN32 */
> > +# define DEFAULT_TMPDIR        "c:/"
> > +# define PATH_SEP      ";"
> > +# define STRIP_DOT_EXE 1
> > +# define PIPE_USE_FORK 0
> >   # define FILENAME_CMP  mbscasecmp
> >   # define FILENAME_CMPN mbsncasecmp
> >   # define FOPEN_RBIN    "rb"
>
> If I understand this right, O_BINARY is being checked to see if it is
> a Posix-style system, right, but Cygwin has O_BINARY defined? O_BINARY
> is not actually being set in system.h for the benefit of source code
> including it? I noticed in info/filesys.c O_BINARY is used for opening
> a file, so will it matter if this value is 0 under Cygwin?

Cygwin *allows* files to be opened as "text" or "binary", but this is not 
necessary.  Everything works fine if all files are viewed as "binary", as in 
GNU/Linux and other Posix systems.  In particular, it's fine that O_BINARY is 0 
in info/filesys.c.

> The problem with setmode is that it is being redefined as "_setmode",
> correct? Does anyone know what systems need to use _setmode instead of
> setmode?

I don't know about other systems, but on Cygwin, setmode is defined to be 
_setmode, as in system.h.  That's why I was getting (harmless) compiler warnings 
about the redefinition of setmode.

> I see the SET_BINARY macro, whose expansion uses setmode, is used in
> info/makedoc.c. Would it matter if setmode wasn't used here under
> Cygwin?

No.

> Does it matter that the other macros will get the Unix definitions,
> e.g. HAVE_DRIVE will always be false, comparison of filenames will be
> case-sensitive, "\" will not be allowed as a directory separator?

I think it's all OK, but I have to think a little more about comparison of file 
names.  I'll get back to you on that.

> > 2. The following patch provides the prototype for ioctl:
> >
> > Index: info/termdep.h
> > ===================================================================
> > --- info/termdep.h      (revision 5852)
> > +++ info/termdep.h      (working copy)
> > @@ -49,7 +49,7 @@
> >   #  endif /* !HAVE_TERMIO_H */
> >   #endif /* !HAVE_TERMIOS_H */
> >
> > -#ifdef GWINSZ_IN_SYS_IOCTL
> > +#ifndef __MINGW32__
> >   #  include <sys/ioctl.h>
> >   #endif
>
> I see the same thing (conditional on __MINGW32__) is done when
> including sys/ioctl.h in info/ioctl.h and info/man.h. It looks like
> GWINSZ_IN_SYS_IOCTL is set by the AC_HEADER_TIOCGWINSZ macro in
> configure.ac. However, your patch is probably what we want because
> we're not just including <sys/ioctl.h> to get TIOCGWINSZ (although I
> don't understand why you would want the definition of TIOCGWINSZ and
> /not/ have the definition of ioctl, so maybe I'm missing something?)

I don't understand that either.  In any case, I think it's clear that we need to 
include <sys/ioctl.h> on most systems, regardless of the value of 
GWINSZ_IN_SYS_IOCTL.

Ken