AIX headers define hz, shadowing variable name

[NattyNarwhal]

Classic stupid define from AIX. Ran into this dealing with muon, which vendors samurai.

In file included from /usr/include/sys/context.h:32,
                 from /QOpenSys/pkgs/lib/gcc/powerpc-ibm-aix6.1.0.0/10/include-fixed/sys/signal.h:372,
                 from /QOpenSys/pkgs/lib/gcc/powerpc-ibm-aix6.1.0.0/10/include-fixed/sys/wait.h:59,
                 from /QOpenSys/pkgs/lib/gcc/powerpc-ibm-aix6.1.0.0/10/include-fixed/stdlib.h:386,
                 from /QOpenSys/pkgs/include/stdlib.h:6,
                 from src/cmd_install.c:9,
                 from src/amalgam.c:24:
src/external/samurai/tree.c: In function 'samu_rot':
src/external/samurai/tree.c:35:6: error: expected identifier or '(' before numeric constant
   35 |  int hz = samu_height(z);
      |      ^~

Naive patch looks something like:

--- a/tree.c
+++ b/tree.c
@@ -18,6 +18,11 @@
 
 #include "external/samurai/ctx.h"
 
+#ifdef _AIX
+/* AIX defines hz... */
+#undef hz
+#endif
+
 #define MAXH (sizeof(void *) * 8 * 3 / 2)
 
 static inline int

...but it may be better to rename hz.

[michaelforney]

Does this also occur when building samurai? hz is not a reserved identifier in C99, and the Makefile uses -std=c99, which should set something like __STRICT_ANSI__, which should signal the libc to not use any unreserved identifiers.

It could be that muon is defining something in the amalgam that is causing the AIX libc headers to define hz. For instance, I see that include/compat.h, included by src/amalgam.c defines _POSIX_C_SOURCE 200809L, which is different from how tree.c is built in samurai.

If you see this same issue when building samurai, it sounds like a bug in AIX. Can you show a snippet of the hz definition in the libc headers with some context? I'm open to changing the name if necessary, but I'm a big proponent of fixing bugs at the source rather than working around them. If it is an issue with AIX libc rather than the particular way muon is vendoring samurai, you should report it to AIX.

[NattyNarwhal]

Happens with samurai proper:

gcc-10 -O1 -std=c99 -Wall -Wextra -Wshadow -Wmissing-prototypes -Wpedantic -Wno-unused-parameter -c -o tree.o tree.c
In file included from /usr/include/sys/context.h:32,
                 from /QOpenSys/pkgs/lib/gcc/powerpc-ibm-aix6.1.0.0/10/include-fixed/sys/signal.h:372,
                 from /QOpenSys/pkgs/lib/gcc/powerpc-ibm-aix6.1.0.0/10/include-fixed/sys/wait.h:59,
                 from /QOpenSys/pkgs/lib/gcc/powerpc-ibm-aix6.1.0.0/10/include-fixed/stdlib.h:386,
                 from /QOpenSys/pkgs/include/stdlib.h:6,
                 from tree.c:3:
tree.c: In function 'rot':
tree.c:36:6: error: expected identifier or '(' before numeric constant
   36 |  int hz = height(z);
      |      ^~
make: *** [Makefile:38: tree.o] Error 1

AIX does define hz if _ALL_SOURCE is defined, in sys/m_param.h:

#ifndef _H_M_PARAM
#define _H_M_PARAM

#ifndef _H_STANDARDS
#include <standards.h>
#endif

#define _NSRS  16
#define _NGPRS 32
#define _NFPRS 32

/* routines for loading and storing words in faultable critical sections */
#define _LOADWCS(__x)           (* (volatile tid_t *) (__x))
#define _STORWCS(__x,__y)       (*(__y) = (__x))

#define _HZ     100             /* ticks per second of the clock        */
#define __hz    HZ              /* Berkeley uses lower case hz          */
#define _CLKTICK 20408          /* microseconds in a clock tick (49 HZ) */

#define _MAXSEG (64*1024)       /* max seg size (in clicks)             */
/* default max data seg size is linear fn of avail pg space at boot time*/
/*
 * Virtual memory related constants, all in bytes, on page boundries
 */
#define _MAXTSIZ        (256*256*4096)          /* max text size */
#define _DFLDSIZ        (128*256*4096)          /* initial data size limit */
#define _MAXDSIZ        (256*256*4096)          /* max data size */
#define _DFLSSIZ        ( 16*256*4096)          /* initial stack size limit */
#define _MAXSSIZ        (256*256*4096)          /* max stack size */

#ifdef _ALL_SOURCE
#define NSRS            _NSRS
#define NGPRS           _NGPRS
#define NFPRS           _NFPRS
#define LOADWCS         _LOADWCS
#define STORWCS         _STORWCS
#define HZ              _HZ
#define hz              __hz
// [...]

...which is odd, usually _ALL_SOURCE shouldn't be defined unless being explicitly opted into.

[michaelforney]

Thanks for checking. Are you able to determine where the _ALL_SOURCE is coming from?

[NattyNarwhal]

Looks like it's in standards.h.

The snippet:

#if (!defined (_XOPEN_SOURCE)) &&  (!defined (_POSIX_SOURCE)) && (!defined (_ANSI_C_SOURCE))
#define _XOPEN_SOURCE  700 // line 161
#define _XOPEN_SOURCE_EXTENDED 1                                                  
#define _POSIX_SOURCE                                                             
#ifndef _POSIX_C_SOURCE                                                           
#define _POSIX_C_SOURCE 200809L                                                   
#endif
#define _ANSI_SOURCE
#ifndef _ANSI_C_SOURCE
#define _ANSI_C_SOURCE
#endif
#ifdef _LONG_LONG
#ifndef _LARGE_FILE_API
#define _LARGE_FILE_API
#endif
#endif
#ifndef _ALL_SOURCE
#define _ALL_SOURCE
#endif
#endif

And the matching gcc -E -ggdb3:

# 161 "/usr/include/standards.h" 3 4
#define _XOPEN_SOURCE 700
#define _XOPEN_SOURCE_EXTENDED 1
#define _POSIX_SOURCE

#define _POSIX_C_SOURCE 200809L

#define _ANSI_SOURCE 

#define _ANSI_C_SOURCE



#define _LARGE_FILE_API 



#define _ALL_SOURCE

Seems forcing i.e CFLAGS=-D_XOPEN_SOURCE=700 works around this one. Not sure what should be done on the samurai side. _POSIX_C_SOURCE is defined in the source in some places, so perhaps maybe there?