Re: [PATCH 1/3] grep: make PCRE1 aware of custom allocator

From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: "Carlo Marcelo Arenas Belón" <carenas@gmail.com>
Cc: gitster@pobox.com, git@vger.kernel.org
Subject: Re: [PATCH 1/3] grep: make PCRE1 aware of custom allocator
Date: Thu, 8 Aug 2019 15:54:58 +0200 (CEST)	[thread overview]
Message-ID: <nycvar.QRO.7.76.6.1908081552440.46@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <20190806085014.47776-2-carenas@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2317 bytes --]

Hi Carlo,

On Tue, 6 Aug 2019, Carlo Marcelo Arenas Belón wrote:

> 63e7e9d8b6 ("git-grep: Learn PCRE", 2011-05-09) didn't include a way
> to override the system alocator, and so it is incompatible with
> USE_NED_ALLOCATOR as reported by Dscho[1] (in similar code from PCRE2)
>
> Make the minimum change possible to ensure this combination is supported
>
> [1] https://public-inbox.org/git/pull.306.git.gitgitgadget@gmail.com
>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
>  Makefile |  2 +-
>  grep.c   | 10 ++++++++++
>  2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index bd246f2989..4b384f3759 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1764,7 +1764,7 @@ ifdef NATIVE_CRLF
>  endif
>
>  ifdef USE_NED_ALLOCATOR
> -	COMPAT_CFLAGS += -Icompat/nedmalloc
> +	COMPAT_CFLAGS += -DUSE_NED_ALLOCATOR -Icompat/nedmalloc

This pretends that all custom allocators are selected at build time,
something I tried to stress in my commit message as not true. You can
pre-load not only nedmalloc, but also jemalloc and unless I am mistaken
also tcmalloc. And mi-malloc.

So the premise of this patch, that you can tell at compile time that a
different allocator than the system one will be in use is simply
incorrect.

Ciao,
Dscho

>  	COMPAT_OBJS += compat/nedmalloc/nedmalloc.o
>  	OVERRIDE_STRDUP = YesPlease
>  endif
> diff --git a/grep.c b/grep.c
> index cd952ef5d3..0154998695 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -150,12 +150,22 @@ int grep_config(const char *var, const char *value, void *cb)
>   * Initialize one instance of grep_opt and copy the
>   * default values from the template we read the configuration
>   * information in an earlier call to git_config(grep_config).
> + *
> + * If using PCRE make sure that the library is configured
> + * to use the right allocator (ex: NED)
>   */
>  void grep_init(struct grep_opt *opt, struct repository *repo, const char *prefix)
>  {
>  	struct grep_opt *def = &grep_defaults;
>  	int i;
>
> +#ifdef USE_NED_ALLOCATOR
> +#ifdef USE_LIBPCRE1
> +	pcre_malloc = malloc;
> +	pcre_free = free;
> +#endif
> +#endif
> +
>  	memset(opt, 0, sizeof(*opt));
>  	opt->repo = repo;
>  	opt->prefix = prefix;
> --
> 2.23.0.rc1
>
>