Re: AC_C_BIGENDIAN answers "universal" on powerpc-aix

[Stepan Kasal]

Hello,

I was thinking about this patch and I think I have noticed some more
nits.  (I would like to apologize that I have not posted earlier.)

I think that comments 1) and 2) needs to be resolved before 2.63.

1)
It should be documented that the default action-if-universal works only
with autoheader.  So, instead of

| And finally, the default for @var{action-if-universal} is to define
| @samp{WORDS_BIGENDIAN} or not, depending on the architecture that the
| code is being generated for.

I propose:

``And finally, the default for @var{action-if-universal} is to ensure
that `config header' defines @samp{WORDS_BIGENDIAN), if and only if
the code being generated is destined for big-endian architecture.
This feature works only if autoheader is used.''

2)
The code contains check m4_ifdef([AH_HEADER], ...)
I'm afraid this check is not correct, because the config header may
be declared after AC_C_BIGENDIAN.  (For example, all config foos may
be declared just before AC_OUTPUT.)

So instead of

if AH_HEADER defined || ACTION_IF_UNIVERSAL given
then
   set ac_cv_c_bigendian=universal if universal detected
else
   AC_DIAGNOSE([obsolete], [AC_C_BIGENDIAN suggests AC_CONFIG_HEADERS])
fi

I propose to perform to do (unconditionally):
    set ac_cv_c_bigendian=universal if universal detected

3)
Moreover, I would move the universal-detection code to a separate
macro, perhaps _AC_C_BIGENDIAN_UNIVERSAL.  The comment above its
definition should mention prominently that the failure wegths are
very asymmetrical: false positives brek "normal" builds and are
much bigger shame that "incomplete universal binaries support."

4)
(The warning could be implmented in a macro called at AC_OUTPUT time,
but I do not think it is _needed_ for 2.63.  The warning cannot catch
all situations, anyway: AC_CONFIG_HEADERS does not necessarily mean
autoheader is used in this project.)


5)
It is important that "AC_APPLE_UNIVERSAL_BUILD" < "WORDS_BIGENDIAN"
so that the macros fall into the config header in the right order.
Perhaps this should be mentioned in a comment above 
AC_DEFINE([AC_APPLE_UNIVERSAL_BUILD],...)

6)
The template contains (in the branch when AC_APPLE_UNIVERSAL_BUILD is
not defined):

# ifndef WORDS_BIGENDIAN
#  undef WORDS_BIGENDIAN
# endif

But I see no need for this "#ifndef"; the #undef line is equivalent
to all the other #undef lines in the header template.
WORDS_BIGENDIAN is not a builtin macro and config header is not
allowed to be included twice.
So I propose to replace the above three lined by a mere #undef.

7)
A cheeky proposal:
What about changing the specification this way:
 -- snip --
"AC_C_BIGENDIAN ensures that WORDS_BIGENDIAN is defined if and only if
the building binary for big endian architecture.
With "universal binaries," this means that WORDS_BIGENDIAN is set
independently for each part of the compilation; this works only when
autoheader is used.

"If the endianity of the platform cannot be detected
@var{action-if-unknown} is executed; the default is to abort configure
and tell the installer how to bypass the test.

"Additionally, if any of the other three parameters is executed if
the corresponding endianity is detected."
 -- snip --

Yes, this does not preserve backward compatibility, because it
defines WORDS_BIGENDIAN even when action-if-bigendian is specified.
But does that really matter?

(The nasty bug was that action-if-bigendian was skipped, because of
the falsely detected universal.)

With that change of the specification, we could drop
AC_APPLE_UNIVERSAL_BUILD and simplify the template back to:

#ifdef __BIG_ENDIAN__
# define WORDS_BIGENDIAN 1
#else
# undef WORDS_BIGENDIAN
#endif

(I think we can suppose that if __BIG_ENDIAN__ is predefined, then
the platform really is big endian.)

What would you think about this?

Have a nice day,
        Stepan