| Message ID | 20190801170946.81221-1-carenas@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] grep: avoid leak of chartables in PCRE2 | expand |
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;
Junio Thanks for fixing the conflicts in pu as well; apologize if I could had made it easier by doings things differently Carlo
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.
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;
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(-)