Re: [PATCH] db/gdbm rewrite

[Peter Stephenson <p.stephenson@xxxxxxxxxxx>]

On Thu, 16 Feb 2017 03:46:15 -0800
Sebastian Gniazdowski <psprint2@xxxxxxxxxxxx> wrote:
> – The parameter "testkey" in the hash should be empty as it is its first
> use. It comes from getgdbmnode():
>         val_pm = (Param) zshcalloc( sizeof (*val_pm) );
>         val_pm->node.flags = PM_SCALAR | PM_HASHELEM; /* no PM_UPTODATE
>         */
>         val_pm->gsu.s = (GsuScalar) ht->tmpdata;
>         ht->addnode( ht, ztrdup( name ), val_pm ); // sets pm->node.nam
> 
> – zshcalloc() should result in "pm->u.str" to be set to NULL
> 
> – so the "first zsfree() in gdbmsetfn()" should not run:
>     if (pm->u.str) {
>         zsfree(pm->u.str);
>         ...

The parameter in question at the point of the error contains

$1 = {node = {next = 0x0, nam = 0x8d7d9c8 "testkey", flags = 537395200}, u = {data = 0x8d7d8b8, arr = 0x8d7d8b8, str = 0x8d7d8b8 "testdata", val = 148363448, valptr = 0x8d7d8b8, dval = 7.3301282755354198e-316, hash = 0x8d7d8b8}, gsu = {s = 0x8dfb540, i = 0x8dfb540, f = 0x8dfb540, a = 0x8dfb540, h = 0x8dfb540}, base = 0, width = 0, env = 0x0, ename = 0x0, old = 0x0, level = 0}

I'm guessing this must be a node within the hash, not the hash itself.

I think this is because the file already exists... Yep, I only get the
error the second time I run the code, so my information was incomplete.
The original allocation will have been at whatever point the internal
hash entries were set up.  So you need to check what length that was
(should be 9 for zsfree() to work on "testdata").

In non-zsh malloc, zsfree() simply dispatches to free() and doesn't care
about the size of the string.

However, replacing all the zsfree()s with free gave me an infinite loop
on a free.  This was at free(umkey) inside the braces (the first
of the two) in gdbmgetfn().  So the internal calculation of what needs
freeing is definitely getting confused by something that's going on.

I won't have a lot of time to look at this myself.

Evidence the same effect doesn't happen with a different allocator isn't
useful with memory errors, which are very sensitive to internal layout.

pws