bug-cvs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Derek Robert Price
Subject: Re: [Bug-gnulib] Re: fnmatch.c includes alloca.h conditionally?
Date: Tue, 20 Apr 2004 11:02:38 -0400
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040413

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Bruno Haible wrote:

>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@.


Is there a reason the Autoconf manual doesn't include the _MSC_VER switch?

>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().


Sound reasoning, but please see my problems with the implementation below.

>>>>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).


Does this mean that you think it would be hard to persuade the
libiberty folks to use approach 1b?

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


1.  Ok, vasnprintf.c includes alloca.h almost unconditionally, so it
almost makes sense to include the alloca.a module, but on closer
inspection, it appears broken.  Assuming HAVE_ALLOCA_H is not defined
so that the GNULIB alloca.h header gets built and included, I would
guess that HAVE_ALLOCA will also usually be undefined, as is the case
on Windows.  Including alloca.h can then sometimes create a working
alloca substitute, for instance on Windows where alloca is #defined to
_alloca, and I presume something similar is happening in the AIX & HP
ifdefs.  It then goes unused since HAVE_ALLOCA was never defined,
rendering vasnprintf's dependency on alloca pointless since it is
never used despite the header file's inclusion,

2.  Why should regex depend on the alloca module then?  regex.c only
includes alloca.h when HAVE_ALLOCA_H is defined, which means the
GNULIB alloca.h will never be included.  It also only uses the
alloca() function when HAVE_ALLOCA is defined, which means the gnulib
alloca() function will never be used, despite being compiled, and the
gl_FUNC_ALLOCA adds nothing useful to AC_FUNC_ALLOCA if the alloca.h
header file will not be used.  Why not just have gl_REGEX just require
AC_FUNC_ALLOCA?

>>>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.


Ok.

>>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)?


The fnmatch module doesn't currently work as specified on Windows.  It
skips the _MSC_VER stuff that the alloc.h header does.  Since on
Windows, HAVE_ALLOC_H does not get defined (as it should not),
fnmatch.c also never includes the GNULIB alloca.h (fnmatch.c's
#include <alloca.h> is #ifdef on HAVE_ALLOC_H).  fnmatch() then fails
to link when no alloc() function is found.

>>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.


Ok.

>>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.


Ok.

>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.


Ok, but how about adding the full switch from alloca.h, including the
_MSC_VER portion, to fnmatch.c?

Derek

- --
                *8^)

Email: derek@ximbiot.com

Get CVS support at <http://ximbiot.com>!
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFAhTuNLD1OTBfyMaQRArozAKCANk4VoYZdKbGQW7/yilPzk5ipMQCgnIJc
UrD5yJgJXztO5nQjslYIWbw=
=WvvR
-----END PGP SIGNATURE-----





reply via email to

[Prev in Thread] Current Thread [Next in Thread]