bug-texinfo
[Top][All Lists]
Advanced

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

Re: Minor build fixes


From: Ken Brown
Subject: Re: Minor build fixes
Date: Thu, 02 Oct 2014 18:24:49 -0400
User-agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0

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



reply via email to

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