Hi David,

On Fri, Jul 21, 2017 at 07:36:37AM +0100, David CARLIER wrote:
> Hi all,
> 
> hopefully it will be revealed useful, in this patch, we re trying to use
> the MALLOC/FREE macros since pcre* libraries are capable of overriding
> internally memory management.

Given that the MALLOC and FREE macros are only used to allocate/release
pools, and that malloc/calloc/free/strdup/realloc() are used everywhere
else, don't you think that you should simply use the regular ones ? In
fact the initial purpose of the MALLOC macro was to be able to redefine
the allocator but in the end there's so much code using malloc()
(including in libraries we have no control over) that it only became a
way to occasionally debug the pool allocators. I think we could even get
rid of these macros by now.

> I had a little related question, would it be better to provide function
> pointers of this form struct my_allocator { void *(*alloc)(void *, size_t),
> void (*free)(void *, void *)} .... } to allow using hierarical based
> allocators like talloc for example ... just thinking aloud not sure it s
> relevant.

I have no opinion here, I really don't know. However I'm seeing something :

> +#if defined(USE_PCRE) || defined(USE_PCRE_JIT)
> +static inline void *
> +my_pcrealloc(size_t size)
> +{
> +     return MALLOC(size);
> +}
> +
> +static inline void
> +my_pcrefree(void *ptr)
> +{
> +     FREE(ptr);
> +}
> +#elif defined(USE_PCRE2) || defined(USE_PCRE2_JIT)
> +static inline void *
> +my_pcrealloc(PCRE2_SIZE size, void *a)
> +{
> +     (void)a;
> +     return MALLOC(size);
> +}
> +
> +static inline void
> +my_pcrefree(void *ptr, void *a)
> +{
> +     (void)a;
> +     FREE(ptr);
> +}
> +#endif
> +
(...)
>  #ifdef USE_PCRE
> +     pcre_malloc = my_pcrealloc;
> +     pcre_free = my_pcrefree;

These two functions are never used as-is and only as pointers, so the
"inline" type modifier is useless at best and partially wrong (in that
it is confusing), eventhough this is harmless. But I think you should
remove it.

Cheers,
Willy

Reply via email to