From 3be8e1bbdde909f7432e1223bf3b4ee3c6470402 Mon Sep 17 00:00:00 2001
From: Sebastian Gniazdowski <psprint2@fastmail.com>
Date: Mon, 6 Feb 2017 15:46:11 -0800
Subject: 40507: Remove extraneous null creating GDBM records

---
 Src/Modules/db_gdbm.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

(limited to 'Src/Modules/db_gdbm.c')

diff --git a/Src/Modules/db_gdbm.c b/Src/Modules/db_gdbm.c
index 8dd60fc0d..310e32948 100644
--- a/Src/Modules/db_gdbm.c
+++ b/Src/Modules/db_gdbm.c
@@ -170,7 +170,7 @@ gdbmgetfn(Param pm)
     GDBM_FILE dbf;
 
     key.dptr = pm->node.nam;
-    key.dsize = strlen(key.dptr) + 1;
+    key.dsize = strlen(key.dptr);
 
     dbf = (GDBM_FILE)(pm->u.hash->tmpdata);
     ret = gdbm_exists(dbf, key);
@@ -191,9 +191,9 @@ gdbmsetfn(Param pm, char *val)
     GDBM_FILE dbf;
 
     key.dptr = pm->node.nam;
-    key.dsize = strlen(key.dptr) + 1;
+    key.dsize = strlen(key.dptr);
     content.dptr = val;
-    content.dsize = strlen(content.dptr) + 1;
+    content.dsize = strlen(content.dptr);
 
     dbf = (GDBM_FILE)(pm->u.hash->tmpdata);
     (void)gdbm_store(dbf, key, content, GDBM_REPLACE);
@@ -207,7 +207,7 @@ gdbmunsetfn(Param pm, UNUSED(int um))
     GDBM_FILE dbf;
 
     key.dptr = pm->node.nam;
-    key.dsize = strlen(key.dptr) + 1;
+    key.dsize = strlen(key.dptr);
 
     dbf = (GDBM_FILE)(pm->u.hash->tmpdata);
     (void)gdbm_delete(dbf, key);
@@ -302,12 +302,12 @@ gdbmhashsetfn(Param pm, HashTable ht)
 	    v.pm = (Param) hn;
 
 	    key.dptr = v.pm->node.nam;
-	    key.dsize = strlen(key.dptr) + 1;
+	    key.dsize = strlen(key.dptr);
 
 	    queue_signals();
 
 	    content.dptr = getstrvalue(&v);
-	    content.dsize = strlen(content.dptr) + 1;
+	    content.dsize = strlen(content.dptr);
 
 	    (void)gdbm_store(dbf, key, content, GDBM_REPLACE);	
 
-- 
cgit v1.2.3


From e9ce00174921354099d96813fc73d62243923904 Mon Sep 17 00:00:00 2001
From: Sebastian Gniazdowski <psprint2@fastmail.com>
Date: Thu, 16 Feb 2017 08:03:37 -0800
Subject: 40558, 40562: General improvements to zsh/db/gdbm module

---
 ChangeLog               |   6 +
 Doc/Zsh/mod_db_gdbm.yo  |  11 ++
 Src/Modules/db_gdbm.c   | 507 +++++++++++++++++++++++++++++++++++++++++-------
 Src/Modules/db_gdbm.mdd |   2 +-
 4 files changed, 454 insertions(+), 72 deletions(-)

(limited to 'Src/Modules/db_gdbm.c')

diff --git a/ChangeLog b/ChangeLog
index 1ab392d88..6f1d19475 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2017-02-17  Peter Stephenson  <p.stephenson@samsung.com>
+
+	* Sebastian: 40558, 40562: Doc/Zsh/mod_db_gdbm.yo,
+	Src/Modules/db_gdbm.c, Src/Modules/db_gdbm.mdd: General
+	improvements to zsh/db/gdbm module.
+
 2017-02-13  Barton E. Schaefer  <schaefer@zsh.org>
 
 	* 40539: Fabian Klotzl: Completion/Unix/Command/_gcc: typo in MIPS
diff --git a/Doc/Zsh/mod_db_gdbm.yo b/Doc/Zsh/mod_db_gdbm.yo
index 90974297c..699e9ab93 100644
--- a/Doc/Zsh/mod_db_gdbm.yo
+++ b/Doc/Zsh/mod_db_gdbm.yo
@@ -43,6 +43,17 @@ local scope (function) ends.  Note that a readonly parameter may not be
 explicitly unset, so the only way to unset a global parameter created with
 `tt(ztie -r)' is to use `tt(zuntie -u)'.
 )
+findex(zgdbmpath)
+cindex(database file path, reading)
+item(tt(zgdbmpath) var(parametername))(
+Put path to database file assigned to var(parametername) into tt(REPLY)
+scalar.
+)
+findex(zgdbm_tied)
+cindex(database tied arrays, enumerating)
+item(tt(zgdbm_tied))(
+Array holding names of all tied parameters.
+)
 enditem()
 
 The fields of an associative array tied to GDBM are neither cached nor
diff --git a/Src/Modules/db_gdbm.c b/Src/Modules/db_gdbm.c
index 310e32948..596a8ae24 100644
--- a/Src/Modules/db_gdbm.c
+++ b/Src/Modules/db_gdbm.c
@@ -6,6 +6,9 @@
  * Copyright (c) 2008 Clint Adams
  * All rights reserved.
  *
+ * Modifications copyright (c) 2017 Sebastian Gniazdowski
+ * All rights reserved.
+ *
  * Permission is hereby granted, without written agreement and without
  * license or royalty fees, to use, copy, modify, and distribute this
  * software and to distribute modified versions of this software for any
@@ -31,6 +34,15 @@
 #include "db_gdbm.mdh"
 #include "db_gdbm.pro"
 
+#ifndef PM_UPTODATE
+#define PM_UPTODATE     (1<<19) /* Parameter has up-to-date data (e.g. loaded from DB) */
+#endif
+
+static Param createhash( char *name, int flags );
+static int append_tied_name( const char *name );
+static int remove_tied_name( const char *name );
+char *unmetafy_zalloc(const char *to_copy, int *new_len);
+
 /*
  * Make sure we have all the bits I'm using for memory mapping, otherwise
  * I don't know what I'm doing.
@@ -41,8 +53,34 @@
 
 static char *backtype = "db/gdbm";
 
-static const struct gsu_scalar gdbm_gsu =
-{ gdbmgetfn, gdbmsetfn, gdbmunsetfn };
+/*
+ * Longer GSU structure, to carry GDBM_FILE of owning
+ * database. Every parameter (hash value) receives GSU
+ * pointer and thus also receives GDBM_FILE - this way
+ * parameters can access proper database.
+ *
+ * Main HashTable parameter has the same instance of
+ * the custom GSU struct in u.hash->tmpdata field.
+ * When database is closed, `dbf` field is set to NULL
+ * and hash values know to not access database when
+ * being unset (total purge at zuntie).
+ *
+ * When database closing is ended, custom GSU struct
+ * is freed. Only new ztie creates new custom GSU
+ * struct instance.
+ */
+
+struct gsu_scalar_ext {
+    struct gsu_scalar std;
+    GDBM_FILE dbf;
+    char *dbfile_path;
+};
+
+/* Source structure - will be copied to allocated one,
+ * with `dbf` filled. `dbf` allocation <-> gsu allocation. */
+static const struct gsu_scalar_ext gdbm_gsu_ext =
+{ { gdbmgetfn, gdbmsetfn, gdbmunsetfn }, 0, 0 };
+
 /**/
 static const struct gsu_hash gdbm_hash_gsu =
 { hashgetfn, gdbmhashsetfn, gdbmhashunsetfn };
@@ -50,6 +88,17 @@ static const struct gsu_hash gdbm_hash_gsu =
 static struct builtin bintab[] = {
     BUILTIN("ztie", 0, bin_ztie, 1, -1, 0, "d:f:r", NULL),
     BUILTIN("zuntie", 0, bin_zuntie, 1, -1, 0, "u", NULL),
+    BUILTIN("zgdbmpath", 0, bin_zgdbmpath, 1, -1, 0, "", NULL),
+};
+
+#define ROARRPARAMDEF(name, var) \
+    { name, PM_ARRAY | PM_READONLY, (void *) var, NULL,  NULL, NULL, NULL }
+
+/* Holds names of all tied parameters */
+char **zgdbm_tied;
+
+static struct paramdef patab[] = {
+    ROARRPARAMDEF( "zgdbm_tied", &zgdbm_tied ),
 };
 
 /**/
@@ -77,8 +126,7 @@ bin_ztie(char *nam, char **args, Options ops, UNUSED(int func))
     }
 
     /* Here should be a lookup of the backend type against
-     * a registry.
-     */
+     * a registry, if generam DB mechanism is to be added */
     if (strcmp(OPT_ARG(ops, 'd'), backtype) != 0) {
         zwarnnam(nam, "unsupported backend type `%s'", OPT_ARG(ops, 'd'));
 	return 1;
@@ -92,7 +140,8 @@ bin_ztie(char *nam, char **args, Options ops, UNUSED(int func))
 	/*
 	 * Unset any existing parameter.  Note there's no implicit
 	 * "local" here, but if the existing parameter is local
-	 * that will be reflected in the new one.
+	 * then new parameter will be also local without following
+         * unset.
 	 *
 	 * We need to do this before attempting to open the DB
 	 * in case this variable is already tied to a DB.
@@ -106,15 +155,15 @@ bin_ztie(char *nam, char **args, Options ops, UNUSED(int func))
     }
 
     dbf = gdbm_open(resource_name, 0, read_write, 0666, 0);
-    if(dbf)
-	addmodulefd(gdbm_fdesc(dbf), FDT_INTERNAL);
-    else {
+    if(dbf) {
+	addmodulefd(gdbm_fdesc(dbf), FDT_MODULE);
+        append_tied_name(pmname);
+    } else {
 	zwarnnam(nam, "error opening database file %s", resource_name);
 	return 1;
     }
 
-    if (!(tied_param = createspecialhash(pmname, &getgdbmnode, &scangdbmkeys,
-					 pmflags))) {
+    if (!(tied_param = createhash(pmname, pmflags))) {
         zwarnnam(nam, "cannot create the requested parameter %s", pmname);
 	fdtable[gdbm_fdesc(dbf)] = FDT_UNUSED;
 	gdbm_close(dbf);
@@ -122,8 +171,23 @@ bin_ztie(char *nam, char **args, Options ops, UNUSED(int func))
     }
 
     tied_param->gsu.h = &gdbm_hash_gsu;
-    tied_param->u.hash->tmpdata = (void *)dbf;
 
+    /* Allocate parameter sub-gsu, fill dbf field. 
+     * dbf allocation is 1 to 1 accompanied by
+     * gsu_scalar_ext allocation. */
+
+    struct gsu_scalar_ext *dbf_carrier = (struct gsu_scalar_ext *) zalloc(sizeof(struct gsu_scalar_ext));
+    dbf_carrier->std = gdbm_gsu_ext.std;
+    dbf_carrier->dbf = dbf;
+    tied_param->u.hash->tmpdata = (void *)dbf_carrier;
+
+    /* Fill also file path field */
+    if (*resource_name != '/') {
+        /* Code copied from check_autoload() */
+        resource_name = zhtricat(metafy(zgetcwd(), -1, META_HEAPDUP), "/", resource_name);
+        resource_name = xsymlink(resource_name, 1);
+    }
+    dbf_carrier->dbfile_path = ztrdup(resource_name);
     return 0;
 }
 
@@ -161,6 +225,53 @@ bin_zuntie(char *nam, char **args, Options ops, UNUSED(int func))
     return ret;
 }
 
+/**/
+static int
+bin_zgdbmpath(char *nam, char **args, Options ops, UNUSED(int func))
+{
+    Param pm;
+    char *pmname;
+
+    pmname = *args;
+
+    if (!pmname) {
+        zwarnnam(nam, "parameter name (whose path is to be written to $REPLY) is required");
+        return 1;
+    }
+
+    pm = (Param) paramtab->getnode(paramtab, pmname);
+    if(!pm) {
+        zwarnnam(nam, "no such parameter: %s", pmname);
+        return 1;
+    }
+
+    if (pm->gsu.h != &gdbm_hash_gsu) {
+        zwarnnam(nam, "not a tied gdbm parameter: %s", pmname);
+        return 1;
+    }
+
+    /* Paranoia, it *will* be always set */
+    if (((struct gsu_scalar_ext *)pm->u.hash->tmpdata)->dbfile_path) {
+        setsparam("REPLY", ztrdup(((struct gsu_scalar_ext *)pm->u.hash->tmpdata)->dbfile_path));
+    } else {
+        setsparam("REPLY", ztrdup(""));
+    }
+
+    return 0;
+}
+
+/*
+ * The param is actual param in hash – always, because
+ * getgdbmnode creates every new key seen. However, it
+ * might be not PM_UPTODATE - which means that database
+ * wasn't yet queried.
+ *
+ * It will be left in this state if database doesn't
+ * contain such key. That might be a drawback, maybe
+ * setting to empty value has sense, as no other writer
+ * can exist. This would remove subtle hcalloc(1) leak.
+ */
+
 /**/
 static char *
 gdbmgetfn(Param pm)
@@ -169,18 +280,56 @@ gdbmgetfn(Param pm)
     int ret;
     GDBM_FILE dbf;
 
-    key.dptr = pm->node.nam;
-    key.dsize = strlen(key.dptr);
+    /* Key already retrieved? There is no sense of asking the
+     * database again, because:
+     * - there can be only multiple readers
+     * - so, no writer + reader use is allowed
+     *
+     * Thus:
+     * - if we are writers, we for sure have newest copy of data
+     * - if we are readers, we for sure have newest copy of data
+     */
+    if ( pm->node.flags & PM_UPTODATE ) {
+        return pm->u.str ? pm->u.str : (char *) hcalloc(1);
+    }
+
+    /* Unmetafy key. GDBM fits nice into this
+     * process, as it uses length of data */
+    int umlen = 0;
+    char *umkey = unmetafy_zalloc(pm->node.nam,&umlen);
+
+    key.dptr = umkey;
+    key.dsize = umlen;
+
+    dbf = ((struct gsu_scalar_ext *)pm->gsu.s)->dbf;
+
+    if((ret = gdbm_exists(dbf, key))) {
+        /* We have data – store it, return it */
+        pm->node.flags |= PM_UPTODATE;
 
-    dbf = (GDBM_FILE)(pm->u.hash->tmpdata);
-    ret = gdbm_exists(dbf, key);
-    if(ret) {
         content = gdbm_fetch(dbf, key);
-    } else {
-        content.dptr = dupstring("");
+
+        /* Ensure there's no leak */
+        if (pm->u.str) {
+            zsfree(pm->u.str);
+        }
+
+        /* Metafy returned data. All fits - metafy
+         * can obtain data length to avoid using \0 */
+        pm->u.str = metafy(content.dptr, content.dsize, META_DUP);
+
+        /* Free key, restoring its original length */
+        zsfree(umkey);
+
+        /* Can return pointer, correctly saved inside hash */
+        return pm->u.str;
     }
 
-    return content.dptr;
+    /* Free key */
+    zsfree(umkey);
+
+    /* Can this be "" ? */
+    return (char *) hcalloc(1);
 }
 
 /**/
@@ -190,78 +339,126 @@ gdbmsetfn(Param pm, char *val)
     datum key, content;
     GDBM_FILE dbf;
 
-    key.dptr = pm->node.nam;
-    key.dsize = strlen(key.dptr);
-    content.dptr = val;
-    content.dsize = strlen(content.dptr);
+    /* Set is done on parameter and on database.
+     * See the allowed workers / readers comment
+     * at gdbmgetfn() */
+
+    /* Parameter */
+    if (pm->u.str) {
+        zsfree(pm->u.str);
+        pm->u.str = NULL;
+        pm->node.flags &= ~(PM_UPTODATE);
+    }
+
+    if (val) {
+        pm->u.str = ztrdup(val);
+        pm->node.flags |= PM_UPTODATE;
+    }
+
+    /* Database */
+    dbf = ((struct gsu_scalar_ext *)pm->gsu.s)->dbf;
+    if (dbf) {
+        int umlen = 0;
+        char *umkey = unmetafy_zalloc(pm->node.nam,&umlen);
 
-    dbf = (GDBM_FILE)(pm->u.hash->tmpdata);
-    (void)gdbm_store(dbf, key, content, GDBM_REPLACE);
+        key.dptr = umkey;
+        key.dsize = umlen;
+
+        if (val) {
+            /* Unmetafy with exact zalloc size */
+            char *umval = unmetafy_zalloc(val,&umlen);
+
+            /* Store */
+            content.dptr = umval;
+            content.dsize = umlen;
+            (void)gdbm_store(dbf, key, content, GDBM_REPLACE);
+
+            /* Free */
+            zsfree(umval);
+        } else {
+            (void)gdbm_delete(dbf, key);
+        }
+
+        /* Free key */
+        zsfree(umkey);
+    }
 }
 
 /**/
 static void
 gdbmunsetfn(Param pm, UNUSED(int um))
 {
-    datum key;
-    GDBM_FILE dbf;
-
-    key.dptr = pm->node.nam;
-    key.dsize = strlen(key.dptr);
-
-    dbf = (GDBM_FILE)(pm->u.hash->tmpdata);
-    (void)gdbm_delete(dbf, key);
+    /* Set with NULL */
+    gdbmsetfn(pm, NULL);
 }
 
 /**/
 static HashNode
 getgdbmnode(HashTable ht, const char *name)
 {
-    int len;
-    char *nameu;
-    Param pm = NULL;
-
-    nameu = dupstring(name);
-    unmetafy(nameu, &len);
-
-    pm = (Param) hcalloc(sizeof(struct param));
-    pm->node.nam = nameu;
-    pm->node.flags = PM_SCALAR;
-    pm->gsu.s = &gdbm_gsu;
-    pm->u.hash = ht;
+    HashNode hn = gethashnode2( ht, name );
+    Param val_pm = (Param) hn;
+
+    /* Entry for key doesn't exist? Create it now,
+     * it will be interfacing between the database
+     * and Zsh - through special gdbm_gsu. So, any
+     * seen key results in new interfacing parameter.
+     *
+     * Previous code was returning heap arena Param
+     * that wasn't actually added to the hash. It was
+     * plainly name / database-key holder. Here we
+     * add the Param to its hash, it is not PM_UPTODATE.
+     * It will be loaded from database *and filled*
+     * or left in that state if the database doesn't
+     * contain it.
+     *
+     * No heap arena memory is used, memory usage is
+     * now limited - by number of distinct keys seen,
+     * not by number of key *uses*.
+     * */
+
+    if ( ! val_pm ) {
+        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
+    }
 
-    return &pm->node;
+    return (HashNode) val_pm;
 }
 
 /**/
 static void
 scangdbmkeys(HashTable ht, ScanFunc func, int flags)
 {
-    Param pm = NULL;
-    datum key, content;
-    GDBM_FILE dbf = (GDBM_FILE)(ht->tmpdata);
-
-    pm = (Param) hcalloc(sizeof(struct param));
-
-    pm->node.flags = PM_SCALAR;
-    pm->gsu.s = &nullsetscalar_gsu;
+    datum key;
+    GDBM_FILE dbf = ((struct gsu_scalar_ext *)ht->tmpdata)->dbf;
 
+    /* Iterate keys adding them to hash, so
+     * we have Param to use in `func` */
     key = gdbm_firstkey(dbf);
 
     while(key.dptr) {
-	content = gdbm_fetch(dbf, key);
-
-	pm->node.nam = key.dptr;
-	pm->u.str = content.dptr;
-	pm->gsu.s = &nullsetscalar_gsu;
+        /* This returns database-interfacing Param,
+         * it will return u.str or first fetch data
+         * if not PM_UPTODATE (newly created) */
+        char *zkey = metafy(key.dptr, key.dsize, META_DUP);
+        HashNode hn = getgdbmnode(ht, zkey);
+        zsfree( zkey );
 
-	func(&pm->node, flags);
+	func(hn, flags);
 
+        /* Iterate - no problem as interfacing Param
+         * will do at most only fetches, not stores */
         key = gdbm_nextkey(dbf, key);
     }
 
 }
 
+/*
+ * Replace database with new hash
+ */
+
 /**/
 static void
 gdbmhashsetfn(Param pm, HashTable ht)
@@ -274,7 +471,7 @@ gdbmhashsetfn(Param pm, HashTable ht)
     if (!pm->u.hash || pm->u.hash == ht)
 	return;
 
-    if (!(dbf = (GDBM_FILE)(pm->u.hash->tmpdata)))
+    if (!(dbf = ((struct gsu_scalar_ext *)pm->u.hash->tmpdata)->dbf))
 	return;
 
     key = gdbm_firstkey(dbf);
@@ -292,6 +489,9 @@ gdbmhashsetfn(Param pm, HashTable ht)
     if (!ht)
 	return;
 
+     /* Put new strings into database, waiting
+      * for their interfacing-Params to be created */
+
     for (i = 0; i < ht->hsize; i++)
 	for (hn = ht->nodes[i]; hn; hn = hn->next) {
 	    struct value v;
@@ -301,16 +501,29 @@ gdbmhashsetfn(Param pm, HashTable ht)
 	    v.arr = NULL;
 	    v.pm = (Param) hn;
 
-	    key.dptr = v.pm->node.nam;
-	    key.dsize = strlen(key.dptr);
+            /* Unmetafy key */
+            int umlen = 0;
+            char *umkey = unmetafy_zalloc(v.pm->node.nam,&umlen);
+
+	    key.dptr = umkey;
+	    key.dsize = umlen;
 
 	    queue_signals();
 
-	    content.dptr = getstrvalue(&v);
-	    content.dsize = strlen(content.dptr);
+            /* Unmetafy */
+            char *umval = unmetafy_zalloc(getstrvalue(&v),&umlen);
 
+            /* Store */
+	    content.dptr = umval;
+	    content.dsize = umlen;
 	    (void)gdbm_store(dbf, key, content, GDBM_REPLACE);	
 
+            /* Free - thanks to unmetafy_zalloc size of
+             * the strings is exact zalloc size - can
+             * pass to zsfree */
+            zsfree(umval);
+            zsfree(umkey);
+
 	    unqueue_signals();
 	}
 }
@@ -319,15 +532,19 @@ gdbmhashsetfn(Param pm, HashTable ht)
 static void
 gdbmuntie(Param pm)
 {
-    GDBM_FILE dbf = (GDBM_FILE)(pm->u.hash->tmpdata);
+    GDBM_FILE dbf = ((struct gsu_scalar_ext *)pm->u.hash->tmpdata)->dbf;
     HashTable ht = pm->u.hash;
 
     if (dbf) { /* paranoia */
 	fdtable[gdbm_fdesc(dbf)] = FDT_UNUSED;
-	gdbm_close(dbf);
-    }
+        gdbm_close(dbf);
 
-    ht->tmpdata = NULL;
+        /* Let hash fields know there's no backend */
+        ((struct gsu_scalar_ext *)ht->tmpdata)->dbf = NULL;
+
+        /* Remove from list of tied parameters */
+        remove_tied_name(pm->node.nam);
+    }
 
     /* for completeness ... createspecialhash() should have an inverse */
     ht->getnode = ht->getnode2 = gethashnode2;
@@ -342,8 +559,20 @@ static void
 gdbmhashunsetfn(Param pm, UNUSED(int exp))
 {
     gdbmuntie(pm);
-    /* hash table is now normal, so proceed normally... */
+
+    /* Remember custom GSU structure assigned to
+     * u.hash->tmpdata before hash gets deleted */
+    struct gsu_scalar_ext * gsu_ext = pm->u.hash->tmpdata;
+
+    /* Uses normal unsetter. Will delete all owned
+     * parameters and also hashtable. */
     pm->gsu.h->setfn(pm, NULL);
+
+    /* Don't need custom GSU structure with its
+     * GDBM_FILE pointer anymore */
+    zsfree( gsu_ext->dbfile_path );
+    zfree( gsu_ext, sizeof(struct gsu_scalar_ext));
+
     pm->node.flags |= PM_UNSET;
 }
 
@@ -355,7 +584,7 @@ static struct features module_features = {
     bintab, sizeof(bintab)/sizeof(*bintab),
     NULL, 0,
     NULL, 0,
-    NULL, 0,
+    patab, sizeof(patab)/sizeof(*patab),
     0
 };
 
@@ -385,6 +614,7 @@ enables_(Module m, int **enables)
 int
 boot_(UNUSED(Module m))
 {
+    zgdbm_tied = zshcalloc((1) * sizeof(char *));
     return 0;
 }
 
@@ -392,6 +622,7 @@ boot_(UNUSED(Module m))
 int
 cleanup_(Module m)
 {
+    /* This frees `zgdbm_tied` */
     return setfeatureenables(m, &module_features, NULL);
 }
 
@@ -401,3 +632,137 @@ finish_(UNUSED(Module m))
 {
     return 0;
 }
+
+/*********************
+ * Utility functions *
+ *********************/
+
+static Param createhash( char *name, int flags ) {
+    Param pm;
+    HashTable ht;
+
+    pm = createparam(name, PM_SPECIAL | PM_HASHED);
+    if (!pm) {
+        return NULL;
+    }
+
+    if (pm->old)
+	pm->level = locallevel;
+
+    /* This creates standard hash. */
+    ht = pm->u.hash = newparamtable(32, name);
+    if (!pm->u.hash) {
+        paramtab->removenode(paramtab, name);
+        paramtab->freenode(&pm->node);
+        zwarnnam(name, "Out of memory when allocating hash");
+    }
+
+    /* These provide special features */
+    ht->getnode = ht->getnode2 = getgdbmnode;
+    ht->scantab = scangdbmkeys;
+
+    return pm;
+}
+
+/*
+ * Adds parameter name to `zgdbm_tied`
+ */
+
+static int append_tied_name( const char *name ) {
+    int old_len = arrlen(zgdbm_tied);
+    char **new_zgdbm_tied = zshcalloc( (old_len+2) * sizeof(char *));
+
+    /* Copy */
+    char **p = zgdbm_tied;
+    char **dst = new_zgdbm_tied;
+    while (*p) {
+        *dst++ = *p++;
+    }
+
+    /* Append new one */
+    *dst = ztrdup(name);
+
+    /* Substitute, free old one */
+    zfree(zgdbm_tied, sizeof(char *) * (old_len + 1));
+    zgdbm_tied = new_zgdbm_tied;
+
+    return 0;
+}
+
+/*
+ * Removes parameter name from `zgdbm_tied`
+ */
+
+static int remove_tied_name( const char *name ) {
+    int old_len = arrlen(zgdbm_tied);
+
+    /* Two stage, to always have arrlen() == zfree-size - 1.
+     * Could do allocation and revert when `not found`, but
+     * what would be better about that. */
+
+    /* Find one to remove */
+    char **p = zgdbm_tied;
+    while (*p) {
+        if (0==strcmp(name,*p)) {
+            break;
+        }
+        p++;
+    }
+
+    /* Copy x+1 to x */
+    while (*p) {
+        *p=*(p+1);
+        p++;
+    }
+
+    /* Second stage. Size changed? Only old_size-1
+     * change is possible, but.. paranoia way */
+    int new_len = arrlen(zgdbm_tied);
+    if (new_len != old_len) {
+        char **new_zgdbm_tied = zshcalloc((new_len+1) * sizeof(char *));
+
+        /* Copy */
+        p = zgdbm_tied;
+        char **dst = new_zgdbm_tied;
+        while (*p) {
+            *dst++ = *p++;
+        }
+        *dst = NULL;
+
+        /* Substitute, free old one */
+        zfree(zgdbm_tied, sizeof(char *) * (old_len + 1));
+        zgdbm_tied = new_zgdbm_tied;
+    }
+
+    return 0;
+}
+
+/*
+ * Unmetafy that:
+ * - duplicates bufer to work on it,
+ * - does zalloc of exact size for the new string,
+ * - restores work buffer to original content, to restore strlen
+ *
+ * No zsfree()-confusing string will be produced.
+ */
+char *unmetafy_zalloc(const char *to_copy, int *new_len) {
+    char *work, *to_return;
+    int my_new_len = 0;
+
+    work = ztrdup(to_copy);
+    work = unmetafy(work,&my_new_len);
+
+    if (new_len)
+        *new_len = my_new_len;
+
+    /* This string can be correctly zsfree()-d */
+    to_return = (char *) zalloc((my_new_len+1)*sizeof(char));
+    memcpy(to_return, work, sizeof(char)*my_new_len); // memcpy handles $'\0'
+    to_return[my_new_len]='\0';
+
+    /* Restore original strlen and correctly free */
+    strcpy(work, to_copy);
+    zsfree(work);
+
+    return to_return;
+}
diff --git a/Src/Modules/db_gdbm.mdd b/Src/Modules/db_gdbm.mdd
index ce7926bd9..210c22177 100644
--- a/Src/Modules/db_gdbm.mdd
+++ b/Src/Modules/db_gdbm.mdd
@@ -7,6 +7,6 @@ fi
 '
 load=no
 
-autofeatures="b:ztie b:zuntie"
+autofeatures="b:ztie b:zuntie b:zgdbmpath p:zgdbm_tied"
 
 objects="db_gdbm.o"
-- 
cgit v1.2.3


From 135075e48c6c2789bd8bae0a643961c0c2361dfc Mon Sep 17 00:00:00 2001
From: Sebastian Gniazdowski <psprint@zdharma.org>
Date: Sat, 25 Mar 2017 07:55:54 +0100
Subject: 40898: fix GDBM error handling

---
 ChangeLog             | 4 ++++
 Src/Modules/db_gdbm.c | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

(limited to 'Src/Modules/db_gdbm.c')

diff --git a/ChangeLog b/ChangeLog
index f4b17ea34..a8e6eabb5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2017-05-24  Peter Stephenson  <p.stephenson@samsung.com>
+
+	* Sebastian: 40898: Src/Modules/db_gdbm.c: fix GDBM error handling.
+
 2017-05-23  Peter Stephenson  <p.stephenson@samsung.com>
 
 	* Stephane: 41142: Src/Modules/system.c: ensure close-on-exec is
diff --git a/Src/Modules/db_gdbm.c b/Src/Modules/db_gdbm.c
index 596a8ae24..0a28a0740 100644
--- a/Src/Modules/db_gdbm.c
+++ b/Src/Modules/db_gdbm.c
@@ -154,12 +154,13 @@ bin_ztie(char *nam, char **args, Options ops, UNUSED(int func))
 	    return 1;
     }
 
+    gdbm_errno=0;
     dbf = gdbm_open(resource_name, 0, read_write, 0666, 0);
     if(dbf) {
 	addmodulefd(gdbm_fdesc(dbf), FDT_MODULE);
         append_tied_name(pmname);
     } else {
-	zwarnnam(nam, "error opening database file %s", resource_name);
+	zwarnnam(nam, "error opening database file %s (%s)", resource_name, gdbm_strerror(gdbm_errno));
 	return 1;
     }
 
-- 
cgit v1.2.3


From 62c416915b1bee6d7ef9dc87f6009907748f2087 Mon Sep 17 00:00:00 2001
From: Sebastian Gniazdowski <psprint@zdharma.org>
Date: Wed, 24 May 2017 16:07:23 +0200
Subject: 41146: careul in GDBM freeing strings with embedded nulls

---
 ChangeLog             |  3 +++
 Src/Modules/db_gdbm.c | 39 +++++++++++++++++++++++++++++----------
 2 files changed, 32 insertions(+), 10 deletions(-)

(limited to 'Src/Modules/db_gdbm.c')

diff --git a/ChangeLog b/ChangeLog
index a8e6eabb5..8ce4aecf3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
 2017-05-24  Peter Stephenson  <p.stephenson@samsung.com>
 
+	* Sebastian: 41146: Src/Modules/db_gdbm.c: be more careful about
+	freeing strings with embedded nulls.
+
 	* Sebastian: 40898: Src/Modules/db_gdbm.c: fix GDBM error handling.
 
 2017-05-23  Peter Stephenson  <p.stephenson@samsung.com>
diff --git a/Src/Modules/db_gdbm.c b/Src/Modules/db_gdbm.c
index 0a28a0740..35254b68c 100644
--- a/Src/Modules/db_gdbm.c
+++ b/Src/Modules/db_gdbm.c
@@ -41,7 +41,8 @@
 static Param createhash( char *name, int flags );
 static int append_tied_name( const char *name );
 static int remove_tied_name( const char *name );
-char *unmetafy_zalloc(const char *to_copy, int *new_len);
+static char *unmetafy_zalloc(const char *to_copy, int *new_len);
+static void set_length(char *buf, int size);
 
 /*
  * Make sure we have all the bits I'm using for memory mapping, otherwise
@@ -320,13 +321,15 @@ gdbmgetfn(Param pm)
         pm->u.str = metafy(content.dptr, content.dsize, META_DUP);
 
         /* Free key, restoring its original length */
+        set_length(umkey, umlen);
         zsfree(umkey);
 
         /* Can return pointer, correctly saved inside hash */
         return pm->u.str;
     }
 
-    /* Free key */
+    /* Free key, restoring its original length */
+    set_length(umkey, umlen);
     zsfree(umkey);
 
     /* Can this be "" ? */
@@ -375,12 +378,14 @@ gdbmsetfn(Param pm, char *val)
             (void)gdbm_store(dbf, key, content, GDBM_REPLACE);
 
             /* Free */
+            set_length(umval, umlen);
             zsfree(umval);
         } else {
             (void)gdbm_delete(dbf, key);
         }
 
         /* Free key */
+        set_length(umkey, key.dsize);
         zsfree(umkey);
     }
 }
@@ -519,10 +524,12 @@ gdbmhashsetfn(Param pm, HashTable ht)
 	    content.dsize = umlen;
 	    (void)gdbm_store(dbf, key, content, GDBM_REPLACE);	
 
-            /* Free - thanks to unmetafy_zalloc size of
-             * the strings is exact zalloc size - can
-             * pass to zsfree */
+            /* Free - unmetafy_zalloc allocates exact required
+             * space, however unmetafied string can have zeros
+             * in content, so we must first fill with non-0 bytes */
+            set_length(umval, content.dsize);
             zsfree(umval);
+            set_length(umkey, key.dsize);
             zsfree(umkey);
 
 	    unqueue_signals();
@@ -577,10 +584,6 @@ gdbmhashunsetfn(Param pm, UNUSED(int exp))
     pm->node.flags |= PM_UNSET;
 }
 
-#else
-# error no gdbm
-#endif /* have gdbm */
-
 static struct features module_features = {
     bintab, sizeof(bintab)/sizeof(*bintab),
     NULL, 0,
@@ -746,7 +749,7 @@ static int remove_tied_name( const char *name ) {
  *
  * No zsfree()-confusing string will be produced.
  */
-char *unmetafy_zalloc(const char *to_copy, int *new_len) {
+static char *unmetafy_zalloc(const char *to_copy, int *new_len) {
     char *work, *to_return;
     int my_new_len = 0;
 
@@ -767,3 +770,19 @@ char *unmetafy_zalloc(const char *to_copy, int *new_len) {
 
     return to_return;
 }
+
+/*
+ * For zsh-allocator, rest of Zsh seems to use
+ * free() instead of zsfree(), and such length
+ * restoration causes slowdown, but all is this
+ * way strict - correct */
+static void set_length(char *buf, int size) {
+    buf[size]='\0';
+    while ( -- size >= 0 ) {
+        buf[size]=' ';
+    }
+}
+
+#else
+# error no gdbm
+#endif /* have gdbm */
-- 
cgit v1.2.3


From ca0607c4c219ea1c75673dd2a89831997d07754e Mon Sep 17 00:00:00 2001
From: Sebastian Gniazdowski <psprint@zdharma.org>
Date: Sun, 28 May 2017 13:15:06 -0700
Subject: 41153: finish module setup only after all error conditions have been
 checked

---
 ChangeLog             | 3 +++
 Src/Modules/db_gdbm.c | 8 ++++----
 2 files changed, 7 insertions(+), 4 deletions(-)

(limited to 'Src/Modules/db_gdbm.c')

diff --git a/ChangeLog b/ChangeLog
index 15a02afa1..d97672317 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
 2017-05-28  Barton E. Schaefer  <schaefer@zsh.org>
 
+	* Sebastian: 41153: Src/Modules/db_gdbm.c: finish module setup
+	only after all error conditions have been checked
+
 	* unposted: Doc/Zsh/mod_complist.yo: clarify ZLS_COLORS pattern
 	matching contexts
 
diff --git a/Src/Modules/db_gdbm.c b/Src/Modules/db_gdbm.c
index 35254b68c..c4bb931a3 100644
--- a/Src/Modules/db_gdbm.c
+++ b/Src/Modules/db_gdbm.c
@@ -157,10 +157,7 @@ bin_ztie(char *nam, char **args, Options ops, UNUSED(int func))
 
     gdbm_errno=0;
     dbf = gdbm_open(resource_name, 0, read_write, 0666, 0);
-    if(dbf) {
-	addmodulefd(gdbm_fdesc(dbf), FDT_MODULE);
-        append_tied_name(pmname);
-    } else {
+    if(dbf == NULL) {
 	zwarnnam(nam, "error opening database file %s (%s)", resource_name, gdbm_strerror(gdbm_errno));
 	return 1;
     }
@@ -172,6 +169,9 @@ bin_ztie(char *nam, char **args, Options ops, UNUSED(int func))
 	return 1;
     }
 
+    addmodulefd(gdbm_fdesc(dbf), FDT_MODULE);
+    append_tied_name(pmname);
+
     tied_param->gsu.h = &gdbm_hash_gsu;
 
     /* Allocate parameter sub-gsu, fill dbf field. 
-- 
cgit v1.2.3


From 284b5aaeb08f90520ac7dc385e8921471678e7dd Mon Sep 17 00:00:00 2001
From: Sebastian Gniazdowski <psprint@zdharma.org>
Date: Sun, 28 May 2017 13:17:26 -0700
Subject: 41151: propagate flags when creating new parameter

---
 ChangeLog             | 3 +++
 Src/Modules/db_gdbm.c | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

(limited to 'Src/Modules/db_gdbm.c')

diff --git a/ChangeLog b/ChangeLog
index d97672317..eb1af3488 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -3,6 +3,9 @@
 	* Sebastian: 41153: Src/Modules/db_gdbm.c: finish module setup
 	only after all error conditions have been checked
 
+	* Sebastian: 41151: Src/Modules/db_gdbm.c: propagate flags when
+	creating new parameter
+
 	* unposted: Doc/Zsh/mod_complist.yo: clarify ZLS_COLORS pattern
 	matching contexts
 
diff --git a/Src/Modules/db_gdbm.c b/Src/Modules/db_gdbm.c
index c4bb931a3..0ab0fe725 100644
--- a/Src/Modules/db_gdbm.c
+++ b/Src/Modules/db_gdbm.c
@@ -645,7 +645,7 @@ static Param createhash( char *name, int flags ) {
     Param pm;
     HashTable ht;
 
-    pm = createparam(name, PM_SPECIAL | PM_HASHED);
+    pm = createparam(name, flags | PM_SPECIAL | PM_HASHED);
     if (!pm) {
         return NULL;
     }
-- 
cgit v1.2.3


From 6116fdb2779c7e1a8623d0f34a48c8fed2d144c5 Mon Sep 17 00:00:00 2001
From: Sebastian Gniazdowski <psprint@zdharma.org>
Date: Thu, 29 Jun 2017 11:08:02 +0200
Subject: 41375: GDBM interface bug fixes

---
 ChangeLog             |   4 ++
 Src/Modules/db_gdbm.c | 156 ++++++++++++++++++++++++++++++--------------------
 2 files changed, 97 insertions(+), 63 deletions(-)

(limited to 'Src/Modules/db_gdbm.c')

diff --git a/ChangeLog b/ChangeLog
index 0f0d6d31b..fb79b29f9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2017-07-03  Peter Stephenson  <p.stephenson@samsung.com>
+
+	* Sebastian: 41375: Src/Modules/db_gdbm.c: GDBM interface bug fixes.
+
 2017-07-02  Peter Stephenson  <p.w.stephenson@ntlworld.com>
 
 	* 41386: Src/jobs.c: when backgrounding a STAT_CURSH job, remove
diff --git a/Src/Modules/db_gdbm.c b/Src/Modules/db_gdbm.c
index 0ab0fe725..cf1322459 100644
--- a/Src/Modules/db_gdbm.c
+++ b/Src/Modules/db_gdbm.c
@@ -42,7 +42,9 @@ static Param createhash( char *name, int flags );
 static int append_tied_name( const char *name );
 static int remove_tied_name( const char *name );
 static char *unmetafy_zalloc(const char *to_copy, int *new_len);
-static void set_length(char *buf, int size);
+static void myfreeparamnode(HashNode hn);
+
+static int no_database_action = 0;
 
 /*
  * Make sure we have all the bits I'm using for memory mapping, otherwise
@@ -72,7 +74,7 @@ static char *backtype = "db/gdbm";
  */
 
 struct gsu_scalar_ext {
-    struct gsu_scalar std;
+    struct gsu_scalar std; /* Size of three pointers */
     GDBM_FILE dbf;
     char *dbfile_path;
 };
@@ -106,6 +108,7 @@ static struct paramdef patab[] = {
 static int
 bin_ztie(char *nam, char **args, Options ops, UNUSED(int func))
 {
+    struct gsu_scalar_ext *dbf_carrier;
     char *resource_name, *pmname;
     GDBM_FILE dbf = NULL;
     int read_write = GDBM_SYNC, pmflags = PM_REMOVABLE;
@@ -164,21 +167,17 @@ bin_ztie(char *nam, char **args, Options ops, UNUSED(int func))
 
     if (!(tied_param = createhash(pmname, pmflags))) {
         zwarnnam(nam, "cannot create the requested parameter %s", pmname);
-	fdtable[gdbm_fdesc(dbf)] = FDT_UNUSED;
 	gdbm_close(dbf);
 	return 1;
     }
 
-    addmodulefd(gdbm_fdesc(dbf), FDT_MODULE);
-    append_tied_name(pmname);
-
     tied_param->gsu.h = &gdbm_hash_gsu;
 
     /* Allocate parameter sub-gsu, fill dbf field. 
      * dbf allocation is 1 to 1 accompanied by
      * gsu_scalar_ext allocation. */
 
-    struct gsu_scalar_ext *dbf_carrier = (struct gsu_scalar_ext *) zalloc(sizeof(struct gsu_scalar_ext));
+    dbf_carrier = (struct gsu_scalar_ext *) zalloc(sizeof(struct gsu_scalar_ext));
     dbf_carrier->std = gdbm_gsu_ext.std;
     dbf_carrier->dbf = dbf;
     tied_param->u.hash->tmpdata = (void *)dbf_carrier;
@@ -190,6 +189,10 @@ bin_ztie(char *nam, char **args, Options ops, UNUSED(int func))
         resource_name = xsymlink(resource_name, 1);
     }
     dbf_carrier->dbfile_path = ztrdup(resource_name);
+
+    addmodulefd(gdbm_fdesc(dbf), FDT_INTERNAL);
+    append_tied_name(pmname);
+
     return 0;
 }
 
@@ -215,8 +218,9 @@ bin_zuntie(char *nam, char **args, Options ops, UNUSED(int func))
 	}
 
 	queue_signals();
-	if (OPT_ISSET(ops,'u'))
-	    gdbmuntie(pm);	/* clear read-only-ness */
+	if (OPT_ISSET(ops,'u')) {
+            pm->node.flags &= ~PM_READONLY;
+        }
 	if (unsetparam_pm(pm, 0, 1)) {
 	    /* assume already reported */
 	    ret = 1;
@@ -270,8 +274,7 @@ bin_zgdbmpath(char *nam, char **args, Options ops, UNUSED(int func))
  *
  * It will be left in this state if database doesn't
  * contain such key. That might be a drawback, maybe
- * setting to empty value has sense, as no other writer
- * can exist. This would remove subtle hcalloc(1) leak.
+ * setting to empty value has sense.
  */
 
 /**/
@@ -279,7 +282,8 @@ static char *
 gdbmgetfn(Param pm)
 {
     datum key, content;
-    int ret;
+    int ret, umlen;
+    char *umkey;
     GDBM_FILE dbf;
 
     /* Key already retrieved? There is no sense of asking the
@@ -292,13 +296,13 @@ gdbmgetfn(Param pm)
      * - if we are readers, we for sure have newest copy of data
      */
     if ( pm->node.flags & PM_UPTODATE ) {
-        return pm->u.str ? pm->u.str : (char *) hcalloc(1);
+        return pm->u.str ? pm->u.str : "";
     }
 
     /* Unmetafy key. GDBM fits nice into this
      * process, as it uses length of data */
-    int umlen = 0;
-    char *umkey = unmetafy_zalloc(pm->node.nam,&umlen);
+    umlen = 0;
+    umkey = unmetafy_zalloc(pm->node.nam,&umlen);
 
     key.dptr = umkey;
     key.dsize = umlen;
@@ -314,26 +318,26 @@ gdbmgetfn(Param pm)
         /* Ensure there's no leak */
         if (pm->u.str) {
             zsfree(pm->u.str);
+            pm->u.str = NULL;
         }
 
         /* Metafy returned data. All fits - metafy
          * can obtain data length to avoid using \0 */
         pm->u.str = metafy(content.dptr, content.dsize, META_DUP);
+        /* gdbm allocates with malloc */
+        free(content.dptr);
 
-        /* Free key, restoring its original length */
-        set_length(umkey, umlen);
-        zsfree(umkey);
+        /* Free key */
+        zfree(umkey, umlen+1);
 
         /* Can return pointer, correctly saved inside hash */
         return pm->u.str;
     }
 
-    /* Free key, restoring its original length */
-    set_length(umkey, umlen);
-    zsfree(umkey);
+    /* Free key */
+    zfree(umkey, umlen+1);
 
-    /* Can this be "" ? */
-    return (char *) hcalloc(1);
+    return "";
 }
 
 /**/
@@ -361,7 +365,7 @@ gdbmsetfn(Param pm, char *val)
 
     /* Database */
     dbf = ((struct gsu_scalar_ext *)pm->gsu.s)->dbf;
-    if (dbf) {
+    if (dbf && no_database_action == 0) {
         int umlen = 0;
         char *umkey = unmetafy_zalloc(pm->node.nam,&umlen);
 
@@ -378,15 +382,13 @@ gdbmsetfn(Param pm, char *val)
             (void)gdbm_store(dbf, key, content, GDBM_REPLACE);
 
             /* Free */
-            set_length(umval, umlen);
-            zsfree(umval);
+            zfree(umval, umlen+1);
         } else {
             (void)gdbm_delete(dbf, key);
         }
 
         /* Free key */
-        set_length(umkey, key.dsize);
-        zsfree(umkey);
+        zfree(umkey, key.dsize+1);
     }
 }
 
@@ -427,7 +429,7 @@ getgdbmnode(HashTable ht, const char *name)
         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
+        ht->addnode( ht, ztrdup( name ), val_pm ); /* sets pm->node.nam */
     }
 
     return (HashNode) val_pm;
@@ -437,7 +439,7 @@ getgdbmnode(HashTable ht, const char *name)
 static void
 scangdbmkeys(HashTable ht, ScanFunc func, int flags)
 {
-    datum key;
+    datum key, prev_key;
     GDBM_FILE dbf = ((struct gsu_scalar_ext *)ht->tmpdata)->dbf;
 
     /* Iterate keys adding them to hash, so
@@ -456,7 +458,9 @@ scangdbmkeys(HashTable ht, ScanFunc func, int flags)
 
         /* Iterate - no problem as interfacing Param
          * will do at most only fetches, not stores */
+        prev_key = key;
         key = gdbm_nextkey(dbf, key);
+        free(prev_key.dptr);
     }
 
 }
@@ -489,8 +493,15 @@ gdbmhashsetfn(Param pm, HashTable ht)
 	key = gdbm_firstkey(dbf);
     }
 
-    /* just deleted everything, clean up */
-    (void)gdbm_reorganize(dbf);
+    /* Just deleted everything, clean up if no new data.
+     * User can also reorganize via gdbmtool. */
+    if (!ht || ht->hsize == 0) {
+        (void)gdbm_reorganize(dbf);
+    }
+
+    no_database_action = 1;
+    emptyhashtable(pm->u.hash);
+    no_database_action = 0;
 
     if (!ht)
 	return;
@@ -498,9 +509,11 @@ gdbmhashsetfn(Param pm, HashTable ht)
      /* Put new strings into database, waiting
       * for their interfacing-Params to be created */
 
-    for (i = 0; i < ht->hsize; i++)
+    for (i = 0; i < ht->hsize; i++) {
 	for (hn = ht->nodes[i]; hn; hn = hn->next) {
 	    struct value v;
+            int umlen = 0;
+            char *umkey, *umval;
 
 	    v.isarr = v.flags = v.start = 0;
 	    v.end = -1;
@@ -508,8 +521,7 @@ gdbmhashsetfn(Param pm, HashTable ht)
 	    v.pm = (Param) hn;
 
             /* Unmetafy key */
-            int umlen = 0;
-            char *umkey = unmetafy_zalloc(v.pm->node.nam,&umlen);
+            umkey = unmetafy_zalloc(v.pm->node.nam,&umlen);
 
 	    key.dptr = umkey;
 	    key.dsize = umlen;
@@ -517,23 +529,23 @@ gdbmhashsetfn(Param pm, HashTable ht)
 	    queue_signals();
 
             /* Unmetafy */
-            char *umval = unmetafy_zalloc(getstrvalue(&v),&umlen);
+            umval = unmetafy_zalloc(getstrvalue(&v),&umlen);
 
             /* Store */
 	    content.dptr = umval;
 	    content.dsize = umlen;
 	    (void)gdbm_store(dbf, key, content, GDBM_REPLACE);	
 
-            /* Free - unmetafy_zalloc allocates exact required
-             * space, however unmetafied string can have zeros
-             * in content, so we must first fill with non-0 bytes */
-            set_length(umval, content.dsize);
-            zsfree(umval);
-            set_length(umkey, key.dsize);
-            zsfree(umkey);
+            /* Free - unmetafy_zalloc allocates
+             * exact required space + 1 null byte */
+            zfree(umval, content.dsize+1);
+            zfree(umkey, key.dsize+1);
 
 	    unqueue_signals();
 	}
+    }
+    /* We reuse our hash, the input is to be deleted */
+    deleteparamtable(ht);
 }
 
 /**/
@@ -566,14 +578,16 @@ gdbmuntie(Param pm)
 static void
 gdbmhashunsetfn(Param pm, UNUSED(int exp))
 {
+    struct gsu_scalar_ext * gsu_ext;
+
     gdbmuntie(pm);
 
     /* Remember custom GSU structure assigned to
      * u.hash->tmpdata before hash gets deleted */
-    struct gsu_scalar_ext * gsu_ext = pm->u.hash->tmpdata;
+    gsu_ext = pm->u.hash->tmpdata;
 
-    /* Uses normal unsetter. Will delete all owned
-     * parameters and also hashtable. */
+    /* Uses normal unsetter (because gdbmuntie is called above).
+     * Will delete all owned field-parameters and also hashtable. */
     pm->gsu.h->setfn(pm, NULL);
 
     /* Don't need custom GSU structure with its
@@ -654,13 +668,17 @@ static Param createhash( char *name, int flags ) {
 	pm->level = locallevel;
 
     /* This creates standard hash. */
-    ht = pm->u.hash = newparamtable(32, name);
+    ht = pm->u.hash = newparamtable(17, name);
     if (!pm->u.hash) {
         paramtab->removenode(paramtab, name);
         paramtab->freenode(&pm->node);
-        zwarnnam(name, "Out of memory when allocating hash");
+        zwarnnam(name, "out of memory when allocating hash");
+        return NULL;
     }
 
+    /* Does free Param (unsetfn is called) */
+    ht->freenode = myfreeparamnode;
+
     /* These provide special features */
     ht->getnode = ht->getnode2 = getgdbmnode;
     ht->scantab = scangdbmkeys;
@@ -699,6 +717,7 @@ static int append_tied_name( const char *name ) {
 
 static int remove_tied_name( const char *name ) {
     int old_len = arrlen(zgdbm_tied);
+    int new_len;
 
     /* Two stage, to always have arrlen() == zfree-size - 1.
      * Could do allocation and revert when `not found`, but
@@ -721,13 +740,14 @@ static int remove_tied_name( const char *name ) {
 
     /* Second stage. Size changed? Only old_size-1
      * change is possible, but.. paranoia way */
-    int new_len = arrlen(zgdbm_tied);
+    new_len = arrlen(zgdbm_tied);
     if (new_len != old_len) {
+        char **dst;
         char **new_zgdbm_tied = zshcalloc((new_len+1) * sizeof(char *));
 
         /* Copy */
         p = zgdbm_tied;
-        char **dst = new_zgdbm_tied;
+        dst = new_zgdbm_tied;
         while (*p) {
             *dst++ = *p++;
         }
@@ -746,10 +766,9 @@ static int remove_tied_name( const char *name ) {
  * - duplicates bufer to work on it,
  * - does zalloc of exact size for the new string,
  * - restores work buffer to original content, to restore strlen
- *
- * No zsfree()-confusing string will be produced.
  */
-static char *unmetafy_zalloc(const char *to_copy, int *new_len) {
+static char *
+unmetafy_zalloc(const char *to_copy, int *new_len) {
     char *work, *to_return;
     int my_new_len = 0;
 
@@ -761,7 +780,7 @@ static char *unmetafy_zalloc(const char *to_copy, int *new_len) {
 
     /* This string can be correctly zsfree()-d */
     to_return = (char *) zalloc((my_new_len+1)*sizeof(char));
-    memcpy(to_return, work, sizeof(char)*my_new_len); // memcpy handles $'\0'
+    memcpy(to_return, work, sizeof(char)*my_new_len); /* memcpy handles $'\0' */
     to_return[my_new_len]='\0';
 
     /* Restore original strlen and correctly free */
@@ -771,16 +790,27 @@ static char *unmetafy_zalloc(const char *to_copy, int *new_len) {
     return to_return;
 }
 
-/*
- * For zsh-allocator, rest of Zsh seems to use
- * free() instead of zsfree(), and such length
- * restoration causes slowdown, but all is this
- * way strict - correct */
-static void set_length(char *buf, int size) {
-    buf[size]='\0';
-    while ( -- size >= 0 ) {
-        buf[size]=' ';
+static void
+myfreeparamnode(HashNode hn)
+{
+    Param pm = (Param) hn;
+
+    /* Upstream: The second argument of unsetfn() is used by modules to
+     * differentiate "exp"licit unset from implicit unset, as when
+     * a parameter is going out of scope.  It's not clear which
+     * of these applies here, but passing 1 has always worked.
+     */
+
+    /* if (delunset) */
+    pm->gsu.s->unsetfn(pm, 1);
+
+    zsfree(pm->node.nam);
+    /* If this variable was tied by the user, ename was ztrdup'd */
+    if (pm->node.flags & PM_TIED && pm->ename) {
+        zsfree(pm->ename);
+        pm->ename = NULL;
     }
+    zfree(pm, sizeof(struct param));
 }
 
 #else
-- 
cgit v1.2.3