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

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

Paul Eggert wrote:

>Derek Robert Price <[email protected]> writes:
>
>>Is there a reason the Autoconf manual doesn't include the _MSC_VER
switch?
>
>
>Most likely it's because we didn't know about the problem when that
>text was written.  Also, there's understandable reluctance to
>complicate a common idiom for every little compiler idiosyncrasy that
>we run into.  Bruno's (1b) suggestion (of arranging things so that
>"#include <alloca.h>" always works) is the best way I know to get out
>of this mess in the long run.


Ok.

>>Does this mean that you think it would be hard to persuade the
>>libiberty folks to use approach 1b?
>
>
>It might take some time, but I don't think we need to wait for that.
>Please see the last two lines of this message for a suggestion that
>should work regardless of whether the libiberty folks adopt 1b.
>
>>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,
>
>
>It's not pointless, since the maintainer of vasprintf can include
><alloca.h> unconditionally without worrying about whether there's a
>system alloca.h; this simplifies the code a bit.  The maintainer can
>also use HAVE_ALLOCA to determine whether to use an efficient alloca.


Ok, that makes sense.

>However, I admit that it's a bit confusing for those who don't know
>all the twists and turns here.  Perhaps it would be slightly clearer
>to replace this:
>
>   #ifndef IN_LIBINTL
>   # include <alloca.h>
>   #endif
>
>by this:
>
>   #ifdef HAVE_ALLOCA
>   # include <alloca.h>
>   #endif
>
>in vasnprintf.  That IN_LIBINTL is a bit of a mystery anyway -- at
>least to me -- as the code later uses alloca if HAVE_ALLOCA, not if
>IN_LIBINTL.


I agree.

>While we're on the subject, the following code in lib/alloca_.h
>is a bit weird:
>
>#   if HAVE_ALLOCA_H
>#    include <alloca.h>
>
>This "#include" cannot possibly be reached (which is a good thing as
>the file would be including itself if it did).  Wouldn't it clarify
>things a bit to remove that "#if" and "#include"?


Bruno said he liked this because it meant that portions of the code
such as fnmatch.c which wanted to use the block themselves without
necessarily including alloca_.h could just copy and paste.

Personally, I think I would like to see the alloca module use
standardized to include the unconditional #include of <alloca.h>, if
possible.  Why should any piece of code prefer to cut and paste the
block instead?

>>2.  Why should regex depend on the alloca module then?
>
>
>I don't know.  It doesn't seem to be necessary as things stand now.
>argp and fnmatch are similar: they don't seem to need the alloca
>module now.


Actually, fnmatch.c still calls alloca() unconditionally, but it does
look to me like the dependency reference in modules/argp can be removed.

>>The fnmatch module doesn't currently work as specified on Windows.  It
>>skips the _MSC_VER stuff that the alloc.h header does.
>
>
>I assume you mean gnulib's "alloca_.h" here, not "alloc.h".


Yes.

>>Ok, but how about adding the full switch from alloca.h, including the
>>_MSC_VER portion, to fnmatch.c?
>
>
>Wouldn't it be simpler to change gl_FUNC_ALLOCA to always define
HAVE_ALLOCA_H?


This would work if you removed the #include <alloca.h> from
alloca_.h.  In this case, I should think alloca_.h should #define
HAVE_ALLOCA on platforms where it is deemed faster than malloc, such
as when #ifdef _MSC_VER, as well.

Derek

- --
                *8^)

Email: [email protected]

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

iD8DBQFAhXaFLD1OTBfyMaQRAoMGAKCkGPF6NUtlwy0rZ3JqskOOybNB5ACguh+b
bZevap6fYB4UQH5YBpubTjY=
=h/1B
-----END PGP SIGNATURE-----