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

[arnold]

Hi Paul.

Paul Eggert  wrote:

> On 12/08/2017 01:16 AM in 
>  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

Re: [PATCH 06/17] Regex: Use re_malloc / re_free consistently.

[arnold]

Thanks, I have merged this in to gawk's version.

Paul --- I think that  you have permission to push the patches you approve
to glibc. Please do so.

Thanks,

Arnold

Paul Eggert  wrote:

> On 12/08/2017 01:16 AM, Arnold Robbins wrote:
> > This patch changes several calls to malloc/free into re_malloc/re_free,
> > bringing consistency to the code.
>
> Thanks, that patch makes sense, but it misses three opportunities to 
> bring consistency. regcomp.c has one call each to malloc and free, which 
> should be consistent too. Also, regexec.c has a call to realloc that 
> should be be changed to re_realloc. A minor formatting issue: one 
> newly-introduced re_malloc call doesn't need to appear on the next line.
>
> (Possibly we should be adding consistency in the opposite way, by 
> removing the macros re_free, re_malloc, and re_realloc, and simply using 
> the underlying C functions. These macros are tricky since they are 
> function-like but (aside from re_free) cannot be implemented as 
> functions, and they don't buy much. But that'd be a bigger change.)
>
> I installed the attached patch into Gnulib; it contains the originally 
> proposed patch 06/17 along with the abovementioned fixups. Something 
> like this should be easily installable into glibc.
>

Re: [PATCH 01/17] Regex: Fix spelling errors / typos.

[arnold]

Thanks!

One down, 16 to go... :-)

"Carlos O'Donell"  wrote:

> On 12/19/2017 02:03 PM, Paul Eggert wrote:
> > These typo changes are all in Gnulib, and would be fine to install in glibc.
> > 
> > Adding bug-gnulib to the CC list. For reference, the original email is here:
> > https://sourceware.org/ml/libc-alpha/2017-12/msg00243.html
>
> Done. I've pushed these for Arnold.
>
> commit 5069ff32842c60c55f8b573ee66fe43f9ec364af
> Author: Arnold Robbins 
> Date:   Tue Dec 19 19:26:08 2017 -0800
>
> regex: Fix spelling in comments.
> 
> Fix the spelling in various comments throughout the
> regex implementation. These changes are also present
> in gnulib and will be integrated there also, see:
> https://sourceware.org/ml/libc-alpha/2017-12/msg00688.html
>
>
> -- 
> Cheers,
> Carlos.