Discussion:
[PATCH] stat: port to xlc 12.01
Paul Eggert
2017-06-25 07:08:09 UTC
Permalink
* lib/stat-w32.c: Always include <sys/types.h>. Otherwise, xlc
12.01 complains "Compilation unit is empty."
---
ChangeLog | 6 ++++++
lib/stat-w32.c | 4 +++-
2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index 26875e7..78c8257 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2017-06-25 Paul Eggert <***@cs.ucla.edu>
+
+ stat: port to xlc 12.01
+ * lib/stat-w32.c: Always include <sys/types.h>. Otherwise, xlc
+ 12.01 complains "Compilation unit is empty."
+
2017-06-24 Paul Eggert <***@cs.ucla.edu>

xalloc-oversized: port to icc
diff --git a/lib/stat-w32.c b/lib/stat-w32.c
index b4c762c..022d01c 100644
--- a/lib/stat-w32.c
+++ b/lib/stat-w32.c
@@ -18,13 +18,15 @@

#include <config.h>

+/* Include this on all platforms, so that the compilation unit is nonempty. */
+#include <sys/types.h>
+
#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__

/* Ensure that <windows.h> defines FILE_ID_INFO. */
#undef _WIN32_WINNT
#define _WIN32_WINNT _WIN32_WINNT_WIN8

-#include <sys/types.h>
#include <sys/stat.h>
#include <errno.h>
#include <limits.h>
--
2.9.4
Bruno Haible
2017-06-25 09:14:14 UTC
Permalink
Hi Paul,
Post by Paul Eggert
diff --git a/lib/stat-w32.c b/lib/stat-w32.c
index b4c762c..022d01c 100644
--- a/lib/stat-w32.c
+++ b/lib/stat-w32.c
@@ -18,13 +18,15 @@
#include <config.h>
+/* Include this on all platforms, so that the compilation unit is nonempty. */
+#include <sys/types.h>
+
#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
/* Ensure that <windows.h> defines FILE_ID_INFO. */
#undef _WIN32_WINNT
#define _WIN32_WINNT _WIN32_WINNT_WIN8
-#include <sys/types.h>
#include <sys/stat.h>
#include <errno.h>
#include <limits.h>
I prefer the generic idiom 'typedef int dummy;' instead, because:
1) It's generic (i.e. looks the same in all files that need it),
and we're already using it in
lib/argp-pin.c
lib/canonicalize-lgpl.c
lib/dummy.c
lib/float.c
lib/getcwd-lgpl.c
lib/glthread/threadlib.c
lib/lstat.c
lib/math.c
lib/pthread.c
lib/sys_socket.c
lib/u64.c
lib/unistd.c
2) The <sys/types.h> include here really belongs together with
<sys/stat.h>.


2017-06-25 Bruno Haible <***@clisp.org>

stat: Improve last change.
* lib/stat-w32.c: Revert last change. Use generic idiom instead.


diff --git a/lib/stat-w32.c b/lib/stat-w32.c
index 022d01c..237e2aa 100644
--- a/lib/stat-w32.c
+++ b/lib/stat-w32.c
@@ -18,15 +18,13 @@

#include <config.h>

-/* Include this on all platforms, so that the compilation unit is nonempty. */
-#include <sys/types.h>
-
#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__

/* Ensure that <windows.h> defines FILE_ID_INFO. */
#undef _WIN32_WINNT
#define _WIN32_WINNT _WIN32_WINNT_WIN8

+#include <sys/types.h>
#include <sys/stat.h>
#include <errno.h>
#include <limits.h>
@@ -414,4 +412,10 @@ _gl_fstat_by_handle (HANDLE h, const char *path, struct stat *buf)
}
}

+#else
+
+/* This declaration is solely to ensure that after preprocessing
+ this file is never empty. */
+typedef int dummy;
+
#endif
Paul Eggert
2017-06-25 17:56:07 UTC
Permalink
Post by Bruno Haible
I prefer the generic idiom 'typedef int dummy;' instead
Sure. But come to think of it, wouldn't it be better if we compiled stat-w32.c
only on w32 platforms? As things stand, we're likely to run into other porting
glitches on non-w32 platforms, due to compilers that complain about dummy
declarations or whatever. Plus, people auditing builds have to deal with these
compilations that do nothing.
Bruno Haible
2017-06-25 18:58:04 UTC
Permalink
Hi Paul,
Post by Paul Eggert
As things stand, we're likely to run into other porting
glitches on non-w32 platforms, due to compilers that complain about dummy
declarations or whatever.
Such hassles have not occurred with the other files that I mentioned
lib/argp-pin.c
lib/canonicalize-lgpl.c
lib/dummy.c
lib/float.c
lib/getcwd-lgpl.c
lib/glthread/threadlib.c
lib/lstat.c
lib/math.c
lib/pthread.c
lib/sys_socket.c
lib/u64.c
lib/unistd.c
Therefore I think it's very improbable that we run into a problem with
"typedef int dummy;". Unused typedefs are very frequent in C.
Post by Paul Eggert
wouldn't it be better if we compiled stat-w32.c only on w32 platforms?
This is certainly possible (patch below). What we then have is a platform
condition coded at the configure level (based on $host_os from
AC_CANONICAL_HOST) and a platform condition coded in C (based on preprocessor
defines). The experience has taught me that it is a maintainability problem
to keep them in sync.
- At some point Mac OS would be recognized through
case "$host_os" in macos*
Later one had to write
case "$host_os" in darwin*
- At some point native Windows would be recognized through
case "$host_os" in mingw* | pw32*
Now it seems to be only
case "$host_os" in mingw*
When they are not in sync, the typical outcome is a build failure (link error).
So, now, I prefer to do these platform conditions at one level OR the other -
not both.
Post by Paul Eggert
people auditing builds have to deal with these compilations that do nothing.
Who is doing that? Which rules are these people applying to decide whether
something is problematic? I would guess that empty object files are perfectly
OK.

Bruno


diff --git a/modules/fstat b/modules/fstat
index 7a62c6a..e498941 100644
--- a/modules/fstat
+++ b/modules/fstat
@@ -19,7 +19,11 @@ configure.ac:
gl_FUNC_FSTAT
if test $REPLACE_FSTAT = 1; then
AC_LIBOBJ([fstat])
- AC_LIBOBJ([stat-w32])
+ case "$host_os" in
+ mingw*)
+ AC_LIBOBJ([stat-w32])
+ ;;
+ esac
gl_PREREQ_FSTAT
fi
gl_SYS_STAT_MODULE_INDICATOR([fstat])
diff --git a/modules/stat b/modules/stat
index de9e7cd..8d9dfd0 100644
--- a/modules/stat
+++ b/modules/stat
@@ -20,7 +20,11 @@ configure.ac:
gl_FUNC_STAT
if test $REPLACE_STAT = 1; then
AC_LIBOBJ([stat])
- AC_LIBOBJ([stat-w32])
+ case "$host_os" in
+ mingw*)
+ AC_LIBOBJ([stat-w32])
+ ;;
+ esac
gl_PREREQ_STAT
fi
gl_SYS_STAT_MODULE_INDICATOR([stat])
Paul Eggert
2017-06-25 19:31:32 UTC
Permalink
Post by Bruno Haible
So, now, I prefer to do these platform conditions at one level OR the other -
not both.
Makes sense.
Post by Bruno Haible
Post by Paul Eggert
people auditing builds have to deal with these compilations that do nothing.
Who is doing that? Which rules are these people applying to decide whether
something is problematic? I would guess that empty object files are perfectly
OK.
Yes they're OK, but they're one more thing. Compiling these .o files slows down
the build and lengthens the build log slightly. People who read the build logs
will reasonably wonder "why is a w32 file being compiled on a non-w32 platform"?

So if it's a choice of level and either possibility is easy, I'd rather see this
done at the "should I compile this .c file?" level, rather than at the "should
this .c file be empty?" level.
Bruno Haible
2017-06-29 15:37:59 UTC
Permalink
Post by Paul Eggert
Compiling these .o files slows down
the build and lengthens the build log slightly. People who read the build logs
will reasonably wonder "why is a w32 file being compiled on a non-w32 platform"?
OK, I give in. I've pushed the change.

Bruno

Loading...