[debbugs-tracker] bug#17328: closed (dfa.c will fail if used on more than one DFA)

[GNU bug Tracking System]

Your message dated Wed, 23 Apr 2014 12:49:29 -0700
with message-id <address@hidden>
and subject line Re: bug#17328: dfa.c will fail if used on more than one DFA
has caused the debbugs.gnu.org bug report #17328,
regarding dfa.c will fail if used on more than one DFA
to be marked as done.

(If you believe you have received this mail in error, please contact
address@hidden)


-- 
17328: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=17328
GNU Bug Tracking System
Contact address@hidden with problems
> --- Begin Message ---
>
>
>
> Subject: 
>
> dfa.c will fail if used on more than one DFA
>
>
>
>
> Date: 
>
> Wed, 23 Apr 2014 21:36:11 +0300
>
>
>
>
> User-agent: 
>
> Heirloom mailx 12.5 6/20/10
>
>
>
>
> Hello.
>
> There is a built-in assumption that the routines in dfa.c will only
> be used on one struct dfa.  This is true for grep but not true for gawk.
> I found this when trying to update gawk's dfa with all the changes that
> have been coming through.  The patch is below.
>
> Thanks,
>
> Arnold
> -----------------------
> diff --git a/src/dfa.c b/src/dfa.c
> index 65fc03d..85ab9bd 100644
> --- a/src/dfa.c
> +++ b/src/dfa.c
> @@ -3244,15 +3244,10 @@ dfaexec (struct dfa *d, char const *begin, char *end,
>  
>    if (d->mb_cur_max > 1)
>      {
> -      static bool mb_alloc = false;
>        memset (&mbs, 0, sizeof (mbstate_t));
> -      if (!mb_alloc)
> -        {
> -          d->mb_match_lens = xnmalloc (d->nleaves, sizeof *d->mb_match_lens);
> -          d->mb_follows = xmalloc (sizeof *d->mb_follows);
> -          alloc_position_set (d->mb_follows, d->nleaves);
> -          mb_alloc = true;
> -        }
> +      d->mb_match_lens = xnmalloc (d->nleaves, sizeof *d->mb_match_lens);
> +      d->mb_follows = xmalloc (sizeof *d->mb_follows);
> +      alloc_position_set (d->mb_follows, d->nleaves);
>      }
>  
>    for (;;)
> @@ -3434,8 +3429,13 @@ free_mbdata (struct dfa *d)
>      {
>        free (d->mb_follows->elems);
>        free (d->mb_follows);
> +      d->mb_follows = NULL;
> +    }
> +  if (d->mb_match_lens)
> +    {
> +      free (d->mb_match_lens);
> +      d->mb_match_lens = NULL;
>      }
> -  free (d->mb_match_lens);
>  }
>  
>  /* Initialize the components of a dfa that the other routines don't
>
>
>
> --- End Message ---

--- Begin Message --- Subject: Re: bug#17328: dfa.c will fail if used on more than one DFA Date: Wed, 23 Apr 2014 12:49:29 -0700 User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.4.0 Thanks for reporting that. That static variable was the result of a recent optimization. I guess we'll need to optimize in a different way. I noticed another static variable that dfaexec also uses, namely 'mbs'; this needs to be moved into the struct dfa, for the benefit of any applications that really need stateful encodings. A bonus is that I expect this'll make the dfa code run a bit faster. I installed the attached patch, which I hope addresses the issues you raised along with the mbs issue.

The code still needs more work in this area. There shouldn't be any static variables at all, even when parsing, though this would change the API. And the code isn't consistent about referring to dfa->mb_cur_max versus MB_CUR_MAX; not sure why that is.

0001-dfa-omit-static-variables-that-limited-dfaexec-to-on.patch
Description: Text Data

--- End Message ---