Subject: Re: PR/33392 CVS commit: src/dist/nawk
To: Aleksey Cheusov <cheusov@tut.by>
From: Christos Zoulas <christos@zoulas.com>
List: netbsd-bugs
Date: 07/26/2006 16:39:19
On Jul 26,  8:29pm, cheusov@tut.by (Aleksey Cheusov) wrote:
-- Subject: Re: PR/33392 CVS commit: src/dist/nawk

| >  | > | Changes you commited to the HEAD related to this PR seems good to me,
| >  | > | everything works correctly and much faster than gawk (for huge
| >  | > | regexps) that i used for years.
| >  | > | 
| >  | 
| >  | unfortunately, there seem to be some bugs introduced by these commits. 
| >  | I'm getting lots of junk pointer complaints from free() followed by a 
| >  | segfault.  nawk prior to these changes don't have this problem.  The 
| >  | patch Christos attached to PR/3406 makes the segfaults go away, but this 
| >  | new nawk uses more than 10x the RAM on the same script with the same 
| >  | input file than an older (prior to these changes) nawk.
| >  | 
| >  | I can send the awk script and input file if anyone cares to dig around.
| >  
| >  I fixed all of it...
| >  christos
| 
| Are you sure about the following?
| 
| @@ -82,10 +82,10 @@
|         void *p;
|         int i, new_count;
| 
| -       if (state < fa->state_count)
| +       if (++state < fa->state_count)
|              return
| 
| I think ++ is unwanted
| 
|  new_count = state + 10;
|  for (i = fa->state_count; i < new_count; ++i) {
|                            ^^^^^
|     ...
|  }
|  fa->state_count = new_count;

The meaning of state_count has changed from being the largest number
allocated to be the count of the states allocated, so both are correct.
The code now passes all the regression tests. We could change the
++state, if we change the resize_state calls to be 1 greater than now,
but that is more intrusive.


christos