[Freeciv-Dev] Re: (PR#2873) 1.14.0 Xaw client crash

["Stepan Roh via RT" <rt@xxxxxxxxxxxxxx>]

On Sat, 25 Jan 2003, Jason Short via RT wrote:

> > The sad thing is that autoheader is not able to analyze calls to
> > FC_CHECK_X_LIB macro, which calls AC_DEFINE_UNQUOTED. This macro is the
> > only place where AC_DEFINE_* is called on variable and not constant. The
> > proper fix is to add constant AC_DEFINE to every call of FC_CHECK_X_LIB in
> > configure.ac (I wonder it does not crash more often, maybe HAVE_LIBXAW3D
> > is the only one really used, but it should be defined everywhere just to
> > be future-compatible).
>
> Yes.
>
> Per suggested that the problem was that AC_DEFINE_UNQUOTED needs three
> arguments.  This appears not to be the case: even with the additional
> two arguments it doesn't work.
>
> The offender is the following line in m4/x.252:
>
>     AC_DEFINE_UNQUOTED($ac_tr_lib)
>
> $ac_tr_lib gets values like HAVE_LIBX11, HAVE_LIBXEXT, HAVE_LIBXT,
> HAVE_LIBXMU, HAVE_LIBXPM, HAVE_LIBXAW3D, and HAVE_LIBXAW.  None of these
> ever get defined in config.h, although only the HAVE_LIBXAW3D one seems
> fatal (note HAVE_LIBXPM is no longer used; this is a separate issue for
> which I should provide a patch).
>
> According to this page, AC_DEFINE_UNQUOTED should work with variables:
>
> http://www.gnu.org/manual/autoconf-2.53/html_node/Defining-Symbols.html
> http://www.gnu.org/manual/autoconf-2.57/html_chapter/autoconf_7.html#SEC82
>
> and yet it does not, either with my version (2.57) or that which was
> used to generate the distribution's configure script.  This page seems
> to indicate three arguments may be needed:
>
> http://www.gnu.org/manual/autoconf-2.57/html_chapter/autoconf_4.html#SEC29
>
> although it is a bit unspecific about the variable usage.

Quote (important part highlighted with >>><<<):

"In order to do its job, autoheader needs you to document all of the
symbols that you might use; i.e., there must be at least one AC_DEFINE or
one AC_DEFINE_UNQUOTED call with a third argument for each symbol (see
section 7.1 Defining C Preprocessor Symbols). An additional constraint is
that the first argument of AC_DEFINE must be a >>>literal<<<. Note that
all symbols defined by Autoconf's builtin tests are already documented
properly; you only need to document those that you define yourself."

I think it's obvious that you can't call AC_DEFINE with variable and
expect autoheader to trace it. It would be hard work to do it actually and
only with additional constraints (variable content may be based on runtime
state in which case it can't be computed before running) or syntactic
sugar.  AC_DEFINE works with variables (just add manually #undef
HAVE_LIBXAW3D to end of config.h.in and run ./configure
--enable-client=xaw3d and see that config.h contains #define HAVE_LIBXAW3D
1). So it is not bug and it can't be changed.

> > List of all #defines (or strictly speaking #undefs) which are in
> > acconfig.h and are not generated by autoheader (entries not found in
> > configure.ac or m4/* are marked as unused - they may however be used in
> > source; also it looks like acconfig.h is used as template for vms and
> > amiga configs (hand-made of course)):
> >
> > HAVE_LIBICE (unused)
> > HAVE_LIBSM (unused)
> > HAVE_LIBX11
> > HAVE_LIBXAW
> > HAVE_LIBXAW3D
> > HAVE_LIBXEXT
> > HAVE_LIBXMU
> > HAVE_LIBXPM
> > HAVE_LIBXT
> > HAVE_CATGETS (unused)
> > STRICT_WINDOWS (unused)
> > GENERATING_MAC (unused)
> > HAVE_OPENTRANSPORT (unused)
> > PATH_SEPARATOR (unused)
> > OPTION_FILE_NAME (unused)
> >
> > From the list above it is obvious that only calls to FC_CHECK_X_LIB should
> > be altered and everything will work properly.
> >
> > So I summarize my patch-vision:
> >
> > - add AC_DEFINE() to all FC_CHECK_X_LIB calls in configure.ac (maybe also
> > in configure.in?)
> > - add notice to m4/x.252 (and possibly to x.213 also) that AC_DEFINE must
> > be called explicitly or acconfig.h must be altered (for x.213 only)
>
> This seems reasonable.  But it seems likely that AC_DEFINE_UNQUOTED
> *should* work, and we are just using it improperly.

It works. See above.

> The real bug here would then be that autoconf doesn't give us a warning
> that the symbols aren't defined in config.h.in.

Yes.

Maybe we could provide a script which will be called in autogen.sh right
after the autoheader call which will parse calls to FC_CHECK_X_LIB and add
them to config.h.in. Something like this (quick'n'dirty, but works):

#!/bin/sh

input=$1
output=$2

unchanged=1

for lib in `sed -n 's,^.*FC_CHECK_X_LIB(\([^,]*\)\,.*$,\1,p' < $input`; do
  tr_lib=HAVE_LIB`echo $lib | sed -e 's/[^a-zA-Z0-9_]/_/g' \
    -e 'y/abcdefghijklmnopqrstuvwxyz/ABCDEFGHIJKLMNOPQRSTUVWXYZ/'`
  if grep $tr_lib'$' $output >/dev/null 2>/dev/null; then
    true
  else
    echo >> $output
    echo '#undef '$tr_lib >> $output
    unchanged=0
  fi
done

if [ $unchanged -eq 1 ]; then
  echo "fc_config: $output is unchanged"
fi

Stepan Roh