Re: [Bug-gnulib] Re: fnmatch.c includes alloca.h conditionally?

Hi Derek,

There are three approaches of how to use alloca, and things are so
complex because not everyone is yet using the One True approach.

1) Use alloca() on all platforms, with the alloca.c substitute for those
platforms that don't have it.

1a) As described in the autoconf manual: In every source file that uses
alloca(), add the #if template before including any system header files.
In the Makefile.in, use @ALLOCA@.

1b) Using the gnulib alloca module. Use #include <alloca.h> unconditionally,
because gnulib's Makefile.am snippet creates an alloca.h file on those
platforms that don't have it.

2) Use alloca() only on those platforms that have it. On those that don't
have it, use malloc() instead. Since alloca() is intended to be a faster
malloc(), this approach doesn't use the alloca.c substitute which would
be slower than malloc().

> > >I noticed that the fnmatch.c file is including alloca.h conditionally,
> > >based on HAVE_ALLOCA_H, as is regex.c.  Shouldn't they both be able to
> > >assume that alloca.h can be included

fnmatch.c and regex.c are still also used in libiberty (binutils + gcc).
We cannot easily change them for approach 1b) as long as libiberty uses
approach 1a).

> > Similarly, vasnprintf.c & regex.c are switching on HAVE_ALLOCA, which
> > they should also be able to assume.

vasnprintf.c uses approach 2).

> > In fact, on systems where
> > lib/alloca.c needs to be compiled, I think that HAVE_ALLOCA often
> > isn't being defined and thus the function is compiled and not used by
> > vasnprintf or regex.

Yes, ok. Then alloca.c is compiled but not used. Still faster than compiling
it and using it.

> Very obviously, fnmatch.c duplicates some code in "alloca.h":
>
> #ifdef __GNUC__
> # define alloca __builtin_alloca
> # define HAVE_ALLOCA 1
> #else
> # if defined HAVE_ALLOCA_H || defined _LIBC
> #  include <alloca.h>
> # else
> #  ifdef _AIX
>  #  pragma alloca
> #  else
> #   ifndef alloca
> char *alloca ();
> #   endif
> #  endif
> # endif
> #endif
>
> I worked around this on Windows

And what is the problem with it? This code is listed in the autoconf
manual, for approach 1a). What's wrong with it (except that it doesn't
work with _MSC_VER)?

> I'm not sure myself
> whether it would be better to alter the alloca module to assume that
> its header is always included and let it do the switching on
> HAVE_ALLOCA_H or to include the GNULIB alloca.h conditionally based on
> HAVE_ALLOCA_H and remove its on switch on HAVE_ALLOCA_H.

Its own switch on HAVE_ALLOCA_H is obviously equivalent to a #if 0. It's
there, though, to make copy&paste into 1a) files, like fnmatch.c, easy.

> I've attached a GNULIB patch to remove the switches on HAVE_ALLOCA
> from the allocsa, vasnprintf, and xallocsa modules.  Since these
> modules are all dependent on the alloca module, this should be able to
> be assumed.

The patches to allocsa and xallocsa will not be used, because here I'm
using approach 2), and don't want alloca.c if malloc() is faster.

The patch to vasnprintf will not be used, for the same reason and also
because it is used inside libintl, which cannot rely on alloca.c.

The patch to fnmatch is acceptable, but I would nevertheless not apply
it, because it is part of an idiomatic block of #ifs that is often copied
from one source file to another. The fact that fnmatch.c doesn't use all
of the definitions provided by this idiom is irrelevant.

Bruno