Hi,
> The code itself looks good, with a few comments:
>
> } +#if defined(__GNUC__) && __GNUC__ >= 3
> } +# define PCRE_ATTR_MALLOC __attribute__((__malloc__))
>
> I prefer to check for __attribute__ support in autoconf, and condition
> on #ifdef HAVE___ATTRIBUTE__ rather than checking for a particular
> version of gcc. I guess in practice everyone who supports
> __attribute__ also mimicks being gcc (does icc?) so maybe this is "6
> of one, half-dozen of the other."
The thing is that PCRE is often built along with other projects. The
more autoconf magic we add, the worst, since those options are
unlikely to be picked up by these projects bundling PCRE. (and I
speak for myself; we ship PCRE along with PHP, and we have it
integrated in our build system).
And I think all the other gcc-like compilers advertise themselves as
gcc. ICC does it for sure.
> } +#if (defined(__GNUC__) && (__GNUC__ > 4 || (__GNUC__ == 4 &&
> __GNUC_MINOR__ >= 3))) || __has_attribute(alloc_size)
>
> Doesn't clang support __has_attribute(__alloc_size__) now? It seems
> so from here:
> http://clang.llvm.org/docs/LanguageExtensions.html#feature_check
> Seems slightly better, to avoid the (admittedly small) chance an
> application #defines alloc_size.
>
> I'm not familiar with the alloc_size attribute; from this code it
> sounds like it's fairly new. It would definitely be a good addition
> to gperftools!, but I wonder how much benefit it gives to pcre. It
> sounds like it only affects glibc's alloc_size() (?) which I doubt
> anyone will use with these functions. If there was worry about the
> readability of this header file, as Zoltan's comments indicate, one
> possibility would be to keep attribute(__malloc__) but drop the
> alloc_size stuff.
The alloc_size attribute is fairly recent, yes. It's available since
gcc 4.3, and since clang 3.2 (not released yet; I'm still working on
the last details).
This attribute has nothing to do with glibc. It is used by gcc/clang
to provide useful warnings for, e.g., array out-of-bounds indexing.
For example:
char *a = malloc(2);
a[3] - 0; <-- warning: overflow
If you use pcre_malloc instead, then the compiler has no chance to
know that the function returns a buffer of size X. Unless you tag it
with the alloc_size attribute.
Apart of the warnings, it can be used for optimizations, and run-time
code instrumentation.
Nuno
