Re: [PATCH] db/gdbm rewrite

[Sebastian Gniazdowski <psprint2@xxxxxxxxxxxx>]

On Thu, Feb 16, 2017, at 02:16 AM, Peter Stephenson wrote:
> On Wed, 15 Feb 2017 02:22:43 -0800
> Sebastian Gniazdowski <psprint2@xxxxxxxxxxxx> wrote:
> Thanks.
> 
> I've fixed the following warnings (which are trivial).

Sorry, turns out no -Wall by default

> I've added the stack trace; the key context will be frame 2: it's the
> first zsfree() in gdbmsetfn().  Is it simply counting of terminating
> '\0's again, or should it not actually be treating the data as a
> string at all?

Rejection of facts is IMO always futile in such situation, but just to
state facts on db_gdbm:

– 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);
        ...

Maybe zshcalloc() with enable-zsh-mem somehow doesn't zero the memory?

I cannot reproduce. My `make TESTNUM=V11` ends with other core dump and
backtrace:

* frame #0: 0x000000010bdf7cf1 zsh-5.3.1-dev-0`malloc(size=8) + 321 at
mem.c:1264
frame #1: 0x000000010bdf5b14 zsh-5.3.1-dev-0`zalloc(size=4) + 68 at
mem.c:966
frame #2: 0x000000010be2fc29 zsh-5.3.1-dev-0`ztrdup(s="off") + 57 at
string.c:82
...
frame #9: 0x000000010bdb6bef
zsh-5.3.1-dev-0`runshfunc(prog=0x000000010c0e37c8,
wrap=0x0000000000000000, name="ZTST_execchunk") + 575 at exec.c:5647

So it's test suite code. I think it's cool to test with zsh-mem to
stress test the code and find boundary cases, but maybe, at least on my
machine, zsh-mem seems to be not fully ready and db_gdbm might not have
error.. That's for futile defensive speech..

PS. Had to add ulimit -c unlimited to "%prep" section of V11*,  maybe
doing this by default would cause more with-backtrace reports.

-- 
Sebastian Gniazdowski
psprint2@xxxxxxxxxxxx