This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 07/17] Regex: Additional memory management checks.
- From: arnold at skeeve dot com
- To: libc-alpha at sourceware dot org, eggert at cs dot ucla dot edu, carlos at redhat dot com, arnold at skeeve dot com
- Cc: bug-gnulib at gnu dot org
- Date: Wed, 20 Dec 2017 11:58:04 -0700
- Subject: Re: [PATCH 07/17] Regex: Additional memory management checks.
- Authentication-results: sourceware.org; auth=none
- References: <201712080916.vB89GxcF005503@skeeve.com> <2409ec89-d2f5-797f-b597-9870c972ba5e@cs.ucla.edu>
Hi Paul.
Paul Eggert <eggert@cs.ucla.edu> wrote:
> On 12/08/2017 01:16 AM in
> <https://sourceware.org/ml/libc-alpha/2017-12/msg00242.html> Arnold
> Robbins wrote:
> > + /* some malloc()-checkers don't like zero allocations */
>
> Which checkers are these?
Lord only knows. That change has been in gawk's regex for years and
years and I don't remember. So:
> Can they be told that 'malloc (0)' is OK?
Practically speaking, no.
> > + * ADR: valgrind says size can be 0, which then doesn't
> > + * free the block of size 0. Harumph. This seems
> > + * to work ok, though.
> > + */
> > + if (size == 0)
> > + {
> > + memset(set, 0, sizeof(*set));
> > + return REG_NOERROR;
> > + }
> > set->alloc = size;
> > set->nelem = 0;
> > set->elems = re_malloc (int, size);
>
> For this, how about if we use the corresponding Gnulib fix instead? An
> advantage of the Gnulib fix is that it doesn't introduce runtime
> overhead when glibc is used. Here is a URL:
I think that the runtime overhead is so small that it cannot be
measured. I don't want to pull in more gnulib m4 goop for this.
The GLIBC guys can do as they wish, of course. :-)
> > diff --git a/posix/regexec.c b/posix/regexec.c
> > index 2d2bc46..8573765 100644
> > --- a/posix/regexec.c
> > +++ b/posix/regexec.c
> > @@ -605,7 +605,7 @@ re_search_internal (const regex_t *preg, const char *string, int length,
> > nmatch -= extra_nmatch;
> >
> > /* Check if the DFA haven't been compiled. */
> > - if (BE (preg->used == 0 || dfa->init_state == NULL
> > + if (BE (preg->used == 0 || dfa == NULL || dfa->init_state == NULL
> > || dfa->init_state_word == NULL || dfa->init_state_nl == NULL
> > || dfa->init_state_begbuf == NULL, 0))
> > return REG_NOMATCH;
>
> Why is this change needed? I couldn't see a code path that would trigger
> it.
I managed once while doing some changes to cause dfa to be NULL. So
I added the check. I don't remember what I did.
Thanks,
Arnold