bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 07/17] Regex: Additional memory management checks.


From: arnold
Subject: Re: [PATCH 07/17] Regex: Additional memory management checks.
Date: Wed, 20 Dec 2017 11:58:04 -0700
User-agent: Heirloom mailx 12.4 7/29/08

Hi Paul.

Paul Eggert <address@hidden> 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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]