Re: [PATCH v2 08/10] grep/pcre2: actually make pcre2 use custom allocator
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 4, 2021, 00:24 UTC
- Message-ID
- <xmqqlfb3hiku.fsf@gitster.c.googlers.com>
- In-Reply-To
- <20210218000728.13995-9-avarab@gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 16 quoted lines
> Continue work started in 513f2b0bbd4 (grep: make PCRE2 aware of custom
> allocator, 2019-10-16) and make PCREv2 use our pcre2_{malloc,free}().
> functions for allocation. We'll now use it for all PCREv2 allocations.
>
> The reason 513f2b0bbd4 worked as a bugfix for the USE_NED_ALLOCATOR
> issue is because it targeted the allocation freed via free(), as
> opposed to by a pcre2_*free() function. I.e. the pcre2_maketables()
> and pcre2_maketables_free() pair.
>
> For most of the rest we continued allocating with stock malloc()
> inside PCREv2 itself, but didn't segfault because we'd use its
> corresponding free().
>
> In a preceding commit of mine I changed the free() to
> pcre2_maketables_free() on versions of PCREv2 10.34 and newer. So as
> far as fixing the segfault goes ...Wait, wait. So, because of the previous step, we would start segfaulting and we need to fix that breakage, which is the reason why this commit exists?
If so, ...
> we could revert 513f2b0bbd4. But then > we wouldn't use the desired allocator, let's just use it instead.
... I agree with the conclusion that both the previous step and this step are needed and better than a reversion of 513f2b0b (grep: make PCRE2 aware of custom allocator, 2019-10-16) and the previou step.
But even then, it feels somewhat backwards. Shouldn't this step come first, so that we would be using a matching alloc/free pair, and then do the previous step?
> Instead we should always create it, and then pass the general context > to those functions that accept it, so that they'll consistently use > our preferred memory allocation functions.
Thanks.