Re: gawk: other double free(_wstr)

[Andrew J. Schorr]

On Mon, Jan 15, 2007 at 01:03:07PM +0100, Karel Zak wrote:
> On Sat, Jan 13, 2007 at 03:17:20PM -0500, Andrew J. Schorr wrote:
> > On Sat, Jan 13, 2007 at 08:52:30PM +0200, Aharon Robbins wrote:
> > > I think I'm going to undo the part of free_wstr that only zeros the
> > > fields if the flag is set.
> > 
> > Hmmm, this code that you plan to restore (zeroing wstptr even if the
> > WSTRCUR flag is not set) seems to conflict with a statement you made
> > back in July:
> 
>  I agree with Aharon. It's more robust. There are places in code which
>  expect this behavior.

Perhaps I'm misunderstanding the logic, but I believe that changing free_wstr
back to the old behavior was completely orthogonal to fixing the bug that you
discovered.  The problem, in the bug that you found, was that the WSTRCUR flag
was in fact set, but free_wstr was being called after the NODE had been copied
to another NODE that had the old values -- so the pointer was being freed
inside unref's call to free_wstr, but the copied NODE still had the freed
pointer with the WSTRCUR flag set.  The fix that Arnold applied was to
call free_wstr before copying the fields_arr[i] node into tmp.  This fixes
the bug regardless of the change to free_wstr.

Just to be sure, I reran your test case with both versions of free_wstr,
and valgrind reports the same double free error in both cases.  So I claim
that the change to free_wstr has nothing to do with fixing the bug.

So are you sure that there are actually places in the code that depend on this
behavior (free_wstr zeroing the wstptr fields even if WSTRCUR is not set)?
Why would wstptr ever be set if the corresponding WSTRCUR flag was not set?
I'm not sure what problem you're trying to solve.  If you claim that there are
some cases where wstptr is set, but WSTRCUR is not set, then I guess this is
some kind of memory corruption or memory leak situation?  And if so, isn't
the proper approach to fix the bug that caused this situation in the
first place?

Regards,
Andy