[v2] grep: avoid leak of chartables in PCRE2
diff mbox series

Message ID 20190801170946.81221-1-carenas@gmail.com
State New
Headers show
Series
  • [v2] grep: avoid leak of chartables in PCRE2
Related show

Commit Message

Carlo Marcelo Arenas Belón Aug. 1, 2019, 5:09 p.m. UTC
94da9193a6 ("grep: add support for PCRE v2", 2017-06-01) introduced
a small memory leak visible with valgrind in t7813.

Complete the creation of a PCRE2 specific variable that was missing from
the original change and free the generated table just like it is done
for PCRE1.

The table cleanup use free as there is no global context defined when
it was created (pcre2_maketables is passed a NULL pointer) but if that
gets ever changed will need to be updated in tandem.

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
V2:
* better document why free is used as suggested by René
* avoid reusing PCRE1 variable for easy of maintenance (per Ævar)

 grep.c | 7 ++++---
 grep.h | 1 +
 2 files changed, 5 insertions(+), 3 deletions(-)

Comments

Junio C Hamano Aug. 2, 2019, 4:19 p.m. UTC | #1
Carlo Marcelo Arenas Belón  <carenas@gmail.com> writes:

> 94da9193a6 ("grep: add support for PCRE v2", 2017-06-01) introduced
> a small memory leak visible with valgrind in t7813.
>
> Complete the creation of a PCRE2 specific variable that was missing from
> the original change and free the generated table just like it is done
> for PCRE1.
>
> The table cleanup use free as there is no global context defined when
> it was created (pcre2_maketables is passed a NULL pointer) but if that
> gets ever changed will need to be updated in tandem.
>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
> V2:
> * better document why free is used as suggested by René
> * avoid reusing PCRE1 variable for easy of maintenance (per Ævar)

Thanks; will queue.

>
>  grep.c | 7 ++++---
>  grep.h | 1 +
>  2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/grep.c b/grep.c
> index f7c3a5803e..fbd3f3757c 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -488,7 +488,6 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt
>  	PCRE2_UCHAR errbuf[256];
>  	PCRE2_SIZE erroffset;
>  	int options = PCRE2_MULTILINE;
> -	const uint8_t *character_tables = NULL;
>  	int jitret;
>  	int patinforet;
>  	size_t jitsizearg;
> @@ -499,9 +498,10 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt
>  
>  	if (opt->ignore_case) {
>  		if (has_non_ascii(p->pattern)) {
> -			character_tables = pcre2_maketables(NULL);
> +			p->pcre2_tables = pcre2_maketables(NULL);
>  			p->pcre2_compile_context = pcre2_compile_context_create(NULL);
> -			pcre2_set_character_tables(p->pcre2_compile_context, character_tables);
> +			pcre2_set_character_tables(p->pcre2_compile_context,
> +							p->pcre2_tables);
>  		}
>  		options |= PCRE2_CASELESS;
>  	}
> @@ -605,6 +605,7 @@ static void free_pcre2_pattern(struct grep_pat *p)
>  	pcre2_match_data_free(p->pcre2_match_data);
>  	pcre2_jit_stack_free(p->pcre2_jit_stack);
>  	pcre2_match_context_free(p->pcre2_match_context);
> +	free((void *)p->pcre2_tables);
>  }
>  #else /* !USE_LIBPCRE2 */
>  static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt)
> diff --git a/grep.h b/grep.h
> index 1875880f37..26d21a3433 100644
> --- a/grep.h
> +++ b/grep.h
> @@ -96,6 +96,7 @@ struct grep_pat {
>  	pcre2_compile_context *pcre2_compile_context;
>  	pcre2_match_context *pcre2_match_context;
>  	pcre2_jit_stack *pcre2_jit_stack;
> +	const uint8_t *pcre2_tables;
>  	uint32_t pcre2_jit_on;
>  	kwset_t kws;
>  	unsigned fixed:1;
Carlo Marcelo Arenas Belón Aug. 3, 2019, 6:50 p.m. UTC | #2
Junio

Thanks for fixing the conflicts in pu as well; apologize if I could
had made it easier by doings things differently

Carlo
Junio C Hamano Aug. 5, 2019, 7:34 p.m. UTC | #3
Carlo Arenas <carenas@gmail.com> writes:

> Thanks for fixing the conflicts in pu as well; apologize if I could
> had made it easier by doings things differently

With many topics in flight, conflicts between topics happen all the
time.  Thanks for checking to catch a mismerge.

Patch
diff mbox series

diff --git a/grep.c b/grep.c
index f7c3a5803e..fbd3f3757c 100644
--- a/grep.c
+++ b/grep.c
@@ -488,7 +488,6 @@  static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt
 	PCRE2_UCHAR errbuf[256];
 	PCRE2_SIZE erroffset;
 	int options = PCRE2_MULTILINE;
-	const uint8_t *character_tables = NULL;
 	int jitret;
 	int patinforet;
 	size_t jitsizearg;
@@ -499,9 +498,10 @@  static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt
 
 	if (opt->ignore_case) {
 		if (has_non_ascii(p->pattern)) {
-			character_tables = pcre2_maketables(NULL);
+			p->pcre2_tables = pcre2_maketables(NULL);
 			p->pcre2_compile_context = pcre2_compile_context_create(NULL);
-			pcre2_set_character_tables(p->pcre2_compile_context, character_tables);
+			pcre2_set_character_tables(p->pcre2_compile_context,
+							p->pcre2_tables);
 		}
 		options |= PCRE2_CASELESS;
 	}
@@ -605,6 +605,7 @@  static void free_pcre2_pattern(struct grep_pat *p)
 	pcre2_match_data_free(p->pcre2_match_data);
 	pcre2_jit_stack_free(p->pcre2_jit_stack);
 	pcre2_match_context_free(p->pcre2_match_context);
+	free((void *)p->pcre2_tables);
 }
 #else /* !USE_LIBPCRE2 */
 static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt)
diff --git a/grep.h b/grep.h
index 1875880f37..26d21a3433 100644
--- a/grep.h
+++ b/grep.h
@@ -96,6 +96,7 @@  struct grep_pat {
 	pcre2_compile_context *pcre2_compile_context;
 	pcre2_match_context *pcre2_match_context;
 	pcre2_jit_stack *pcre2_jit_stack;
+	const uint8_t *pcre2_tables;
 	uint32_t pcre2_jit_on;
 	kwset_t kws;
 	unsigned fixed:1;