* [PATCH] db/gdbm rewrite
@ 2017-02-14 12:20 Sebastian Gniazdowski
2017-02-15 10:22 ` Sebastian Gniazdowski
0 siblings, 1 reply; 15+ messages in thread
From: Sebastian Gniazdowski @ 2017-02-14 12:20 UTC (permalink / raw)
To: zsh-workers
[-- Attachment #1: Type: text/plain, Size: 1682 bytes --]
Hello,
gdbm module has specific design:
– hashtable is always empty
– getnode() returns heap-arena allocated (hcalloc) Param each time a key
is referenced
– the Param is PM_SCALAR, but it has u.hash set – this is to pass aroud
GDBM_FILE in u.hash.tmpdata
– the Param interfaces to database, never creates u.str (it cannot,
would overwrite u.hash), returns GDBM internal strings
I've rewritten the module:
– hashtable is filled normally, initially with ~PM_UPTODATE
– Param "dereference" detects ~PM_UPTODATE, uses database, sets u.str,
sets PM_UPTODATE
– GDBM_FILE is passed around in gsu_scalar extension:
struct gsu_scalar_ext {
struct gsu_scalar std;
GDBM_FILE dbf;
};
– normal ztrdup() strings are returned from Param
This way there is no subtle memory leak when doing:
while (( 1 )); do val=tied_param[key]; done
Tested this and top values were 100 MiB and rising.
Created also extensive tests (following V10private.ztst), doing things
like +=( ), unset on not existing (in Zsh) key, but existing in
database, scoping.
I develop this as pluggable module psprint/zgdbm, here I submit a few
days of work. I might further expand and submit to upstream, but what's
there now is a finished work, confirmed by complete testing.
Sending as patch and alternatively 2 separate files.
PS. Maybe this:
#define PM_UPTODATE (1<<19) /* Parameter has up-to-date data (e.g.
loaded from DB) */
could be moved to zsh.h. It can be fairly common for every frontend-like
module to use parameters that are not "up to date".
--
Sebastian Gniazdowski
psprint2@fastmail.com
[-- Attachment #2: db_gdbm_rewrite.diff --]
[-- Type: text/plain, Size: 17266 bytes --]
diff --git a/Src/Modules/db_gdbm.c b/Src/Modules/db_gdbm.c
index 310e329..eb83e7d 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,12 @@
#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 );
+
/*
* Make sure we have all the bits I'm using for memory mapping, otherwise
* I don't know what I'm doing.
@@ -41,8 +50,33 @@
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;
+};
+
+/* 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 };
+
/**/
static const struct gsu_hash gdbm_hash_gsu =
{ hashgetfn, gdbmhashsetfn, gdbmhashunsetfn };
@@ -77,8 +111,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 +125,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.
@@ -107,14 +141,13 @@ 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);
+ addmodulefd(gdbm_fdesc(dbf), FDT_MODULE);
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,7 +155,15 @@ 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;
return 0;
}
@@ -161,6 +202,18 @@ bin_zuntie(char *nam, char **args, Options ops, UNUSED(int func))
return ret;
}
+/*
+ * 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 +222,43 @@ gdbmgetfn(Param pm)
int ret;
GDBM_FILE dbf;
+ /* 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);
+ }
+
key.dptr = pm->node.nam;
key.dsize = strlen(key.dptr);
- dbf = (GDBM_FILE)(pm->u.hash->tmpdata);
- ret = gdbm_exists(dbf, key);
- if(ret) {
+ 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;
+
content = gdbm_fetch(dbf, key);
- } else {
- content.dptr = dupstring("");
+
+ /* Ensure there's no leak */
+ if (pm->u.str) {
+ zsfree(pm->u.str);
+ }
+
+ pm->u.str = ztrduppfx( content.dptr, content.dsize );
+
+ /* Can return pointer, correctly saved inside hash */
+ return pm->u.str;
}
- return content.dptr;
+ /* Can this be "" ? */
+ return (char *) hcalloc(1);
}
/**/
@@ -190,47 +268,81 @@ gdbmsetfn(Param pm, char *val)
datum key, content;
GDBM_FILE dbf;
+ /* 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 */
key.dptr = pm->node.nam;
key.dsize = strlen(key.dptr);
- content.dptr = val;
- content.dsize = strlen(content.dptr);
-
- dbf = (GDBM_FILE)(pm->u.hash->tmpdata);
- (void)gdbm_store(dbf, key, content, GDBM_REPLACE);
+ dbf = ((struct gsu_scalar_ext *)pm->gsu.s)->dbf;
+
+ if (val) {
+ if (dbf) {
+ content.dptr = val;
+ content.dsize = strlen(content.dptr);
+ (void)gdbm_store(dbf, key, content, GDBM_REPLACE);
+ }
+ } else {
+ if (dbf) {
+ (void)gdbm_delete(dbf, key);
+ }
+ }
}
/**/
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;
}
/**/
@@ -239,29 +351,33 @@ 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;
+ 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 = ztrduppfx(key.dptr, key.dsize);
+ 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 +390,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 +408,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;
@@ -319,15 +438,16 @@ 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;
+ }
/* for completeness ... createspecialhash() should have an inverse */
ht->getnode = ht->getnode2 = gethashnode2;
@@ -342,8 +462,19 @@ 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 */
+ void * 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 */
+ zfree( gsu_ext, sizeof(struct gsu_scalar_ext));
+
pm->node.flags |= PM_UNSET;
}
@@ -401,3 +532,30 @@ finish_(UNUSED(Module m))
{
return 0;
}
+
+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;
+}
diff --git a/Test/V11db_gdbm.ztst b/Test/V11db_gdbm.ztst
new file mode 100644
index 0000000..fcea55f
--- /dev/null
+++ b/Test/V11db_gdbm.ztst
@@ -0,0 +1,215 @@
+# Tests for the zsh/param/private module
+
+%prep
+
+ #module_path+=( ~/github/zgdbm/module/Src )
+ #modname="psprint/zgdbm"
+ modname="zsh/db/gdbm"
+ dbfile=db.gdbm
+ if ! zmodload $modname ; then
+ ZTST_unimplemented="can't load $modname module for testing"
+ fi
+ rm -f db.gdbm
+
+%test
+
+ (zmodload -u $modname && zmodload $modname)
+0:unload and reload the module without crashing
+
+ ztie -d db/gdbm -f $dbfile dbase
+ zuntie dbase
+0:create the database
+
+ ztie -r -d db/gdbm -f $dbfile dbase
+ zuntie -u dbase
+0:open the database read-only
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase[testkey]=testdata
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[testkey]
+ zuntie -u dbase
+0:store key in database
+>testdata
+
+ ztie -d db/gdbm -f $dbfile dbase2
+ unset 'dbase2[testkey]'
+ zuntie dbase2
+ ztie -d db/gdbm -f $dbfile dbase
+ echo $dbase[testkey]
+ zuntie dbase
+0:remove key from database (different variables)
+>
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase[testkey]=testdata
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[testkey]
+ zuntie -u dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ unset 'dbase[testkey]'
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[testkey]
+ zuntie -u dbase
+0:store & remove key from database (the same variables)
+>testdata
+>
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase[testkey]=testdata
+ dbase[testkey2]=$dbase[testkey]
+ dbase[testkey3]=$dbase[testkey]x$dbase[testkey2]
+ zuntie dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ echo $dbase[testkey]
+ echo $dbase[testkey2]
+ echo $dbase[testkey3]
+ zuntie dbase
+0:store 2 keys fetching 1st
+>testdata
+>testdata
+>testdataxtestdata
+
+ ztie -d db/gdbm -f $dbfile dbase
+ val=$dbase[testkey2]
+ unset 'dbase[testkey2]'
+ echo $val
+ zuntie dbase
+0:unset key that was fetched
+>testdata
+
+ ztie -r -d db/gdbm -f $dbfile dbase
+ print -rl -- "${(kv)dbase[@]}"
+ zuntie -u dbase
+0:scan read-only tied hash
+>testkey
+>testdata
+>testkey3
+>testdataxtestdata
+
+ ztie -d db/gdbm -f $dbfile dbase
+ local -a arr
+ arr=( "${(kv)dbase[@]}" )
+ print -rl -- "${arr[@]}"
+ zuntie dbase
+0:different scan, also read-write mode
+>testkey
+>testdata
+>testkey3
+>testdataxtestdata
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase=( a b c d )
+ zuntie dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ print -rl -- "${(kv)dbase[@]}"
+ zuntie dbase
+0:replace hash / database, scan
+>c
+>d
+>a
+>b
+
+ ztie -d db/gdbm -f $dbfile dbase
+ local -a arr
+ arr=( "${dbase[@]}" )
+ print -rl -- "${arr[@]}"
+ zuntie dbase
+0:scan with no (kv)
+>d
+>b
+
+ ztie -d db/gdbm -f $dbfile dbase
+ print -rl -- "${(k)dbase[@]}"
+ zuntie dbase
+0:scan with keys only (k)
+>c
+>a
+
+ ztie -d db/gdbm -f $dbfile dbase
+ print -rl -- "${(v)dbase[@]}"
+ zuntie dbase
+0:scan with keys only explicit (v)
+>d
+>b
+
+ rm -f $dbfile
+ ztie -r -d db/gdbm -f $dbfile dbase 2>/dev/null
+1:read-only open non-existent database
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase+=( a b )
+ echo $dbase[a]
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[a]
+ print -rl -- "${(kv)dbase[@]}"
+ zuntie -u dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase+=( c d )
+ echo $dbase[a]
+ echo $dbase[c]
+ print -rl -- "${(kv)dbase[@]}"
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[a]
+ echo $dbase[c]
+ print -rl -- "${(kv)dbase[@]}"
+ zuntie -u dbase
+0:Append with +=( ), also with existing data, also (kv) scan
+>b
+>b
+>a
+>b
+>b
+>d
+>c
+>d
+>a
+>b
+>b
+>d
+>c
+>d
+>a
+>b
+
+ ztie -d db/gdbm -f $dbfile dbase
+ echo ${(t)dbase}
+ zuntie dbase
+0:Type of tied parameter
+>association-special
+
+ typeset -ga dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ echo ${(t)dbase}
+ zuntie dbase
+0:Type of tied parameter, with preceding unset
+>association-special
+
+ local -a dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ echo ${(t)dbase}
+ zuntie dbase
+0:Type of tied parameter, with local parameter already existing
+>association-local-special
+
+ local -a dbase
+ dbase=( fromarray )
+ () {
+ local -a dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ echo ${(t)dbase}
+ zuntie dbase
+ }
+ echo $dbase[1]
+ ztie -d db/gdbm -f $dbfile dbase2
+ echo "Can connect, so untie happened:" $dbase2[a]
+ zuntie dbase2
+0:Test of automatic untie (use of local scope) and of scoping
+>association-local-special
+>fromarray
+>Can connect, so untie happened: b
[-- Attachment #3: V11db_gdbm.ztst.txt --]
[-- Type: text/plain, Size: 4170 bytes --]
# Tests for the zsh/param/private module
%prep
#module_path+=( ~/github/zgdbm/module/Src )
#modname="psprint/zgdbm"
modname="zsh/db/gdbm"
dbfile=db.gdbm
if ! zmodload $modname ; then
ZTST_unimplemented="can't load $modname module for testing"
fi
rm -f db.gdbm
%test
(zmodload -u $modname && zmodload $modname)
0:unload and reload the module without crashing
ztie -d db/gdbm -f $dbfile dbase
zuntie dbase
0:create the database
ztie -r -d db/gdbm -f $dbfile dbase
zuntie -u dbase
0:open the database read-only
ztie -d db/gdbm -f $dbfile dbase
dbase[testkey]=testdata
zuntie dbase
ztie -r -d db/gdbm -f $dbfile dbase
echo $dbase[testkey]
zuntie -u dbase
0:store key in database
>testdata
ztie -d db/gdbm -f $dbfile dbase2
unset 'dbase2[testkey]'
zuntie dbase2
ztie -d db/gdbm -f $dbfile dbase
echo $dbase[testkey]
zuntie dbase
0:remove key from database (different variables)
>
ztie -d db/gdbm -f $dbfile dbase
dbase[testkey]=testdata
zuntie dbase
ztie -r -d db/gdbm -f $dbfile dbase
echo $dbase[testkey]
zuntie -u dbase
ztie -d db/gdbm -f $dbfile dbase
unset 'dbase[testkey]'
zuntie dbase
ztie -r -d db/gdbm -f $dbfile dbase
echo $dbase[testkey]
zuntie -u dbase
0:store & remove key from database (the same variables)
>testdata
>
ztie -d db/gdbm -f $dbfile dbase
dbase[testkey]=testdata
dbase[testkey2]=$dbase[testkey]
dbase[testkey3]=$dbase[testkey]x$dbase[testkey2]
zuntie dbase
ztie -d db/gdbm -f $dbfile dbase
echo $dbase[testkey]
echo $dbase[testkey2]
echo $dbase[testkey3]
zuntie dbase
0:store 2 keys fetching 1st
>testdata
>testdata
>testdataxtestdata
ztie -d db/gdbm -f $dbfile dbase
val=$dbase[testkey2]
unset 'dbase[testkey2]'
echo $val
zuntie dbase
0:unset key that was fetched
>testdata
ztie -r -d db/gdbm -f $dbfile dbase
print -rl -- "${(kv)dbase[@]}"
zuntie -u dbase
0:scan read-only tied hash
>testkey
>testdata
>testkey3
>testdataxtestdata
ztie -d db/gdbm -f $dbfile dbase
local -a arr
arr=( "${(kv)dbase[@]}" )
print -rl -- "${arr[@]}"
zuntie dbase
0:different scan, also read-write mode
>testkey
>testdata
>testkey3
>testdataxtestdata
ztie -d db/gdbm -f $dbfile dbase
dbase=( a b c d )
zuntie dbase
ztie -d db/gdbm -f $dbfile dbase
print -rl -- "${(kv)dbase[@]}"
zuntie dbase
0:replace hash / database, scan
>c
>d
>a
>b
ztie -d db/gdbm -f $dbfile dbase
local -a arr
arr=( "${dbase[@]}" )
print -rl -- "${arr[@]}"
zuntie dbase
0:scan with no (kv)
>d
>b
ztie -d db/gdbm -f $dbfile dbase
print -rl -- "${(k)dbase[@]}"
zuntie dbase
0:scan with keys only (k)
>c
>a
ztie -d db/gdbm -f $dbfile dbase
print -rl -- "${(v)dbase[@]}"
zuntie dbase
0:scan with keys only explicit (v)
>d
>b
rm -f $dbfile
ztie -r -d db/gdbm -f $dbfile dbase 2>/dev/null
1:read-only open non-existent database
ztie -d db/gdbm -f $dbfile dbase
dbase+=( a b )
echo $dbase[a]
zuntie dbase
ztie -r -d db/gdbm -f $dbfile dbase
echo $dbase[a]
print -rl -- "${(kv)dbase[@]}"
zuntie -u dbase
ztie -d db/gdbm -f $dbfile dbase
dbase+=( c d )
echo $dbase[a]
echo $dbase[c]
print -rl -- "${(kv)dbase[@]}"
zuntie dbase
ztie -r -d db/gdbm -f $dbfile dbase
echo $dbase[a]
echo $dbase[c]
print -rl -- "${(kv)dbase[@]}"
zuntie -u dbase
0:Append with +=( ), also with existing data, also (kv) scan
>b
>b
>a
>b
>b
>d
>c
>d
>a
>b
>b
>d
>c
>d
>a
>b
ztie -d db/gdbm -f $dbfile dbase
echo ${(t)dbase}
zuntie dbase
0:Type of tied parameter
>association-special
typeset -ga dbase
ztie -d db/gdbm -f $dbfile dbase
echo ${(t)dbase}
zuntie dbase
0:Type of tied parameter, with preceding unset
>association-special
local -a dbase
ztie -d db/gdbm -f $dbfile dbase
echo ${(t)dbase}
zuntie dbase
0:Type of tied parameter, with local parameter already existing
>association-local-special
local -a dbase
dbase=( fromarray )
() {
local -a dbase
ztie -d db/gdbm -f $dbfile dbase
echo ${(t)dbase}
zuntie dbase
}
echo $dbase[1]
ztie -d db/gdbm -f $dbfile dbase2
echo "Can connect, so untie happened:" $dbase2[a]
zuntie dbase2
0:Test of automatic untie (use of local scope) and of scoping
>association-local-special
>fromarray
>Can connect, so untie happened: b
[-- Attachment #4: db_gdbm.c.txt --]
[-- Type: text/plain, Size: 14020 bytes --]
/*
* db_gdbm.c - bindings for gdbm
*
* This file is part of zsh, the Z shell.
*
* 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
* purpose, provided that the above copyright notice and the following
* two paragraphs appear in all copies of this software.
*
* In no event shall Clint Adams or the Zsh Development
* Group be liable to any party for direct, indirect, special, incidental, or
* consequential damages arising out of the use of this software and its
* documentation, even if Peter Stephenson, Sven Wischnowsky and the Zsh
* Development Group have been advised of the possibility of such damage.
*
* Clint Adams and the Zsh Development Group
* specifically disclaim any warranties, including, but not limited to, the
* implied warranties of merchantability and fitness for a particular purpose.
* The software provided hereunder is on an "as is" basis, and Peter
* Stephenson, Sven Wischnowsky and the Zsh Development Group have no
* obligation to provide maintenance, support, updates, enhancements, or
* modifications.
*
*/
#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 );
/*
* Make sure we have all the bits I'm using for memory mapping, otherwise
* I don't know what I'm doing.
*/
#if defined(HAVE_GDBM_H) && defined(HAVE_GDBM_OPEN)
#include <gdbm.h>
static char *backtype = "db/gdbm";
/*
* 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;
};
/* 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 };
/**/
static const struct gsu_hash gdbm_hash_gsu =
{ hashgetfn, gdbmhashsetfn, gdbmhashunsetfn };
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),
};
/**/
static int
bin_ztie(char *nam, char **args, Options ops, UNUSED(int func))
{
char *resource_name, *pmname;
GDBM_FILE dbf = NULL;
int read_write = GDBM_SYNC, pmflags = PM_REMOVABLE;
Param tied_param;
if(!OPT_ISSET(ops,'d')) {
zwarnnam(nam, "you must pass `-d %s'", backtype);
return 1;
}
if(!OPT_ISSET(ops,'f')) {
zwarnnam(nam, "you must pass `-f' with a filename", NULL);
return 1;
}
if (OPT_ISSET(ops,'r')) {
read_write |= GDBM_READER;
pmflags |= PM_READONLY;
} else {
read_write |= GDBM_WRCREAT;
}
/* Here should be a lookup of the backend type against
* 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;
}
resource_name = OPT_ARG(ops, 'f');
pmname = *args;
if ((tied_param = (Param)paramtab->getnode(paramtab, pmname)) &&
!(tied_param->node.flags & PM_UNSET)) {
/*
* Unset any existing parameter. Note there's no implicit
* "local" here, but if the existing parameter is local
* 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.
*
* This can fail if the variable is readonly or restricted.
* We could call unsetparam() and check errflag instead
* of the return status.
*/
if (unsetparam_pm(tied_param, 0, 1))
return 1;
}
dbf = gdbm_open(resource_name, 0, read_write, 0666, 0);
if(dbf)
addmodulefd(gdbm_fdesc(dbf), FDT_MODULE);
else {
zwarnnam(nam, "error opening database file %s", resource_name);
return 1;
}
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;
}
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->std = gdbm_gsu_ext.std;
dbf_carrier->dbf = dbf;
tied_param->u.hash->tmpdata = (void *)dbf_carrier;
return 0;
}
/**/
static int
bin_zuntie(char *nam, char **args, Options ops, UNUSED(int func))
{
Param pm;
char *pmname;
int ret = 0;
for (pmname = *args; *args++; pmname = *args) {
pm = (Param) paramtab->getnode(paramtab, pmname);
if(!pm) {
zwarnnam(nam, "cannot untie %s", pmname);
ret = 1;
continue;
}
if (pm->gsu.h != &gdbm_hash_gsu) {
zwarnnam(nam, "not a tied gdbm hash: %s", pmname);
ret = 1;
continue;
}
queue_signals();
if (OPT_ISSET(ops,'u'))
gdbmuntie(pm); /* clear read-only-ness */
if (unsetparam_pm(pm, 0, 1)) {
/* assume already reported */
ret = 1;
}
unqueue_signals();
}
return ret;
}
/*
* 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)
{
datum key, content;
int ret;
GDBM_FILE dbf;
/* 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);
}
key.dptr = pm->node.nam;
key.dsize = strlen(key.dptr);
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;
content = gdbm_fetch(dbf, key);
/* Ensure there's no leak */
if (pm->u.str) {
zsfree(pm->u.str);
}
pm->u.str = ztrduppfx( content.dptr, content.dsize );
/* Can return pointer, correctly saved inside hash */
return pm->u.str;
}
/* Can this be "" ? */
return (char *) hcalloc(1);
}
/**/
static void
gdbmsetfn(Param pm, char *val)
{
datum key, content;
GDBM_FILE dbf;
/* 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 */
key.dptr = pm->node.nam;
key.dsize = strlen(key.dptr);
dbf = ((struct gsu_scalar_ext *)pm->gsu.s)->dbf;
if (val) {
if (dbf) {
content.dptr = val;
content.dsize = strlen(content.dptr);
(void)gdbm_store(dbf, key, content, GDBM_REPLACE);
}
} else {
if (dbf) {
(void)gdbm_delete(dbf, key);
}
}
}
/**/
static void
gdbmunsetfn(Param pm, UNUSED(int um))
{
/* Set with NULL */
gdbmsetfn(pm, NULL);
}
/**/
static HashNode
getgdbmnode(HashTable ht, const char *name)
{
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 (HashNode) val_pm;
}
/**/
static void
scangdbmkeys(HashTable ht, ScanFunc func, int flags)
{
Param pm = NULL;
datum key, content;
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) {
/* This returns database-interfacing Param,
* it will return u.str or first fetch data
* if not PM_UPTODATE (newly created) */
char *zkey = ztrduppfx(key.dptr, key.dsize);
HashNode hn = getgdbmnode(ht, zkey);
zsfree( zkey );
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)
{
int i;
HashNode hn;
GDBM_FILE dbf;
datum key, content;
if (!pm->u.hash || pm->u.hash == ht)
return;
if (!(dbf = ((struct gsu_scalar_ext *)pm->u.hash->tmpdata)->dbf))
return;
key = gdbm_firstkey(dbf);
while (key.dptr) {
queue_signals();
(void)gdbm_delete(dbf, key);
free(key.dptr);
unqueue_signals();
key = gdbm_firstkey(dbf);
}
/* just deleted everything, clean up */
(void)gdbm_reorganize(dbf);
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;
v.isarr = v.flags = v.start = 0;
v.end = -1;
v.arr = NULL;
v.pm = (Param) hn;
key.dptr = v.pm->node.nam;
key.dsize = strlen(key.dptr);
queue_signals();
content.dptr = getstrvalue(&v);
content.dsize = strlen(content.dptr);
(void)gdbm_store(dbf, key, content, GDBM_REPLACE);
unqueue_signals();
}
}
/**/
static void
gdbmuntie(Param pm)
{
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);
/* Let hash fields know there's no backend */
((struct gsu_scalar_ext *)ht->tmpdata)->dbf = NULL;
}
/* for completeness ... createspecialhash() should have an inverse */
ht->getnode = ht->getnode2 = gethashnode2;
ht->scantab = NULL;
pm->node.flags &= ~(PM_SPECIAL|PM_READONLY);
pm->gsu.h = &stdhash_gsu;
}
/**/
static void
gdbmhashunsetfn(Param pm, UNUSED(int exp))
{
gdbmuntie(pm);
/* Remember custom GSU structure assigned to
* u.hash->tmpdata before hash gets deleted */
void * 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 */
zfree( gsu_ext, sizeof(struct gsu_scalar_ext));
pm->node.flags |= PM_UNSET;
}
#else
# error no gdbm
#endif /* have gdbm */
static struct features module_features = {
bintab, sizeof(bintab)/sizeof(*bintab),
NULL, 0,
NULL, 0,
NULL, 0,
0
};
/**/
int
setup_(UNUSED(Module m))
{
return 0;
}
/**/
int
features_(Module m, char ***features)
{
*features = featuresarray(m, &module_features);
return 0;
}
/**/
int
enables_(Module m, int **enables)
{
return handlefeatures(m, &module_features, enables);
}
/**/
int
boot_(UNUSED(Module m))
{
return 0;
}
/**/
int
cleanup_(Module m)
{
return setfeatureenables(m, &module_features, NULL);
}
/**/
int
finish_(UNUSED(Module m))
{
return 0;
}
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;
}
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] db/gdbm rewrite
2017-02-14 12:20 [PATCH] db/gdbm rewrite Sebastian Gniazdowski
@ 2017-02-15 10:22 ` Sebastian Gniazdowski
2017-02-16 10:16 ` Peter Stephenson
0 siblings, 1 reply; 15+ messages in thread
From: Sebastian Gniazdowski @ 2017-02-15 10:22 UTC (permalink / raw)
To: zsh-workers
[-- Attachment #1: Type: text/plain, Size: 744 bytes --]
Changes:
– data is (un)metafied – compatibility with normal database usage
– new parameter "zgdbm_tied", array holding tied parameters' names
– new builtin "zgdmpath", returning ($REPLY) path to database file, of
given tied parameter
– new tests (Unicode, metafication, new parameter)
– updated documentation
Original db_gdbm was a no-go because it sucked widget names into
database:
https://asciinema.org/a/12ffm5ltpj9udnmradtat17am
Current db_gdbm is a no-go because GDBM doesn't always return \0
terminated strings.. Sorry for that, but the update solves this.
I sent is as patch, plus the new file (as option, patch includes)
Test/V11db_gdbm.ztst.
--
Sebastian Gniazdowski
psprint2@fastmail.com
[-- Attachment #2: db_gdbm_rewrite2.diff --]
[-- Type: text/plain, Size: 25893 bytes --]
diff --git a/Doc/Zsh/mod_db_gdbm.yo b/Doc/Zsh/mod_db_gdbm.yo
index 9097429..699e9ab 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 310e329..3eef4c6 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,14 @@
#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 );
+
/*
* Make sure we have all the bits I'm using for memory mapping, otherwise
* I don't know what I'm doing.
@@ -41,8 +52,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 +87,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 +125,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 +139,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 +154,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 +170,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;
}
@@ -162,6 +225,53 @@ bin_zuntie(char *nam, char **args, Options ops, UNUSED(int func))
}
/**/
+static int
+bin_zgdbmpath(char *nam, char **args, Options ops, UNUSED(int func))
+{
+ Param pm, reply_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 +279,57 @@ 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 */
+ char *umkey = ztrdup(pm->node.nam);
+ int umlen = 0;
+ umkey = unmetafy(umkey,¨en);
+
+ 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_ALLOC);
+
+ /* Free key */
+ 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,47 +339,94 @@ 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);
+ }
- dbf = (GDBM_FILE)(pm->u.hash->tmpdata);
- (void)gdbm_store(dbf, key, content, GDBM_REPLACE);
+ if (val) {
+ pm->u.str = ztrdup(val);
+ pm->node.flags |= PM_UPTODATE;
+ }
+
+ /* Database */
+ dbf = ((struct gsu_scalar_ext *)pm->gsu.s)->dbf;
+ if (dbf) {
+ char *umkey = ztrdup(pm->node.nam);
+ int umlen = 0;
+ umkey = unmetafy(umkey,¨en);
+
+ key.dptr = umkey;
+ key.dsize = umlen;
+
+ if (val) {
+ /* Unmetafy */
+ char *umval = ztrdup(val);
+ umval = unmetafy(umval,¨en);
+
+ /* 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;
}
/**/
@@ -239,29 +435,33 @@ 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;
+ 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 = ztrduppfx(key.dptr, key.dsize);
+ 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 +474,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 +492,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 +504,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 */
+ char *umkey = ztrdup(v.pm->node.nam);
+ int umlen = 0;
+ umkey = unmetafy(umkey,¨en);
+
+ key.dptr = umkey;
+ key.dsize = umlen;
queue_signals();
- content.dptr = getstrvalue(&v);
- content.dsize = strlen(content.dptr);
+ /* Unmetafy */
+ char *umval = ztrdup(getstrvalue(&v));
+ umval = unmetafy(umval,¨en);
+ /* Store */
+ content.dptr = umval;
+ content.dsize = umlen;
(void)gdbm_store(dbf, key, content, GDBM_REPLACE);
+ /* Free */
+ zsfree(umval);
+ zsfree(umkey);
+
unqueue_signals();
}
}
@@ -319,15 +535,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 +562,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 +587,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 +617,7 @@ enables_(Module m, int **enables)
int
boot_(UNUSED(Module m))
{
+ zgdbm_tied = zshcalloc((1) * sizeof(char *));
return 0;
}
@@ -392,6 +625,7 @@ boot_(UNUSED(Module m))
int
cleanup_(Module m)
{
+ /* This frees `zgdbm_tied` */
return setfeatureenables(m, &module_features, NULL);
}
@@ -401,3 +635,107 @@ 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;
+}
diff --git a/Src/Modules/db_gdbm.mdd b/Src/Modules/db_gdbm.mdd
index ce7926b..210c221 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"
diff --git a/Test/V11db_gdbm.ztst b/Test/V11db_gdbm.ztst
new file mode 100644
index 0000000..a1076dc
--- /dev/null
+++ b/Test/V11db_gdbm.ztst
@@ -0,0 +1,285 @@
+# Tests for the zsh/param/private module
+
+%prep
+
+ modname="zsh/db/gdbm"
+ dbfile=db.gdbm
+ if ! zmodload $modname ; then
+ ZTST_unimplemented="can't load $modname module for testing"
+ fi
+ rm -f db.gdbm
+
+%test
+
+ (zmodload -u $modname && zmodload $modname)
+0:unload and reload the module without crashing
+
+ ztie -d db/gdbm -f $dbfile dbase
+ zuntie dbase
+0:create the database
+
+ ztie -r -d db/gdbm -f $dbfile dbase
+ zuntie -u dbase
+0:open the database read-only
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase[testkey]=testdata
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[testkey]
+ zuntie -u dbase
+0:store key in database
+>testdata
+
+ ztie -d db/gdbm -f $dbfile dbase2
+ unset 'dbase2[testkey]'
+ zuntie dbase2
+ ztie -d db/gdbm -f $dbfile dbase
+ echo $dbase[testkey]
+ zuntie dbase
+0:remove key from database (different variables)
+>
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase[testkey]=testdata
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[testkey]
+ zuntie -u dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ unset 'dbase[testkey]'
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[testkey]
+ zuntie -u dbase
+0:store & remove key from database (the same variables)
+>testdata
+>
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase[testkey]=testdata
+ dbase[testkey2]=$dbase[testkey]
+ dbase[testkey3]=$dbase[testkey]x$dbase[testkey2]
+ zuntie dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ echo $dbase[testkey]
+ echo $dbase[testkey2]
+ echo $dbase[testkey3]
+ zuntie dbase
+0:store 2 keys fetching 1st
+>testdata
+>testdata
+>testdataxtestdata
+
+ ztie -d db/gdbm -f $dbfile dbase
+ val=$dbase[testkey2]
+ unset 'dbase[testkey2]'
+ echo $val
+ zuntie dbase
+0:unset key that was fetched
+>testdata
+
+ ztie -r -d db/gdbm -f $dbfile dbase
+ print -rl -- "${(kv)dbase[@]}"
+ zuntie -u dbase
+0:scan read-only tied hash
+>testkey
+>testdata
+>testkey3
+>testdataxtestdata
+
+ ztie -d db/gdbm -f $dbfile dbase
+ local -a arr
+ arr=( "${(kv)dbase[@]}" )
+ print -rl -- "${arr[@]}"
+ zuntie dbase
+0:different scan, also read-write mode
+>testkey
+>testdata
+>testkey3
+>testdataxtestdata
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase=( a b c d )
+ zuntie dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ print -rl -- "${(kv)dbase[@]}"
+ zuntie dbase
+0:replace hash / database, scan
+>c
+>d
+>a
+>b
+
+ ztie -d db/gdbm -f $dbfile dbase
+ local -a arr
+ arr=( "${dbase[@]}" )
+ print -rl -- "${arr[@]}"
+ zuntie dbase
+0:scan with no (kv)
+>d
+>b
+
+ ztie -d db/gdbm -f $dbfile dbase
+ print -rl -- "${(k)dbase[@]}"
+ zuntie dbase
+0:scan with keys only (k)
+>c
+>a
+
+ ztie -d db/gdbm -f $dbfile dbase
+ print -rl -- "${(v)dbase[@]}"
+ zuntie dbase
+0:scan with keys only explicit (v)
+>d
+>b
+
+ rm -f $dbfile
+ ztie -r -d db/gdbm -f $dbfile dbase 2>/dev/null
+1:read-only open non-existent database
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase+=( a b )
+ echo $dbase[a]
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[a]
+ print -rl -- "${(kv)dbase[@]}"
+ zuntie -u dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase+=( c d )
+ echo $dbase[a]
+ echo $dbase[c]
+ print -rl -- "${(kv)dbase[@]}"
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[a]
+ echo $dbase[c]
+ print -rl -- "${(kv)dbase[@]}"
+ zuntie -u dbase
+0:Append with +=( ), also with existing data, also (kv) scan
+>b
+>b
+>a
+>b
+>b
+>d
+>c
+>d
+>a
+>b
+>b
+>d
+>c
+>d
+>a
+>b
+
+ ztie -d db/gdbm -f $dbfile dbase
+ echo ${(t)dbase}
+ zuntie dbase
+0:Type of tied parameter
+>association-special
+
+ typeset -ga dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ echo ${(t)dbase}
+ zuntie dbase
+0:Type of tied parameter, with preceding unset
+>association-special
+
+ local -a dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ echo ${(t)dbase}
+ zuntie dbase
+0:Type of tied parameter, with local parameter already existing
+>association-local-special
+
+ local -a dbase
+ dbase=( fromarray )
+ () {
+ local -a dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ echo ${(t)dbase}
+ zuntie dbase
+ }
+ echo $dbase[1]
+ ztie -d db/gdbm -f $dbfile dbase2
+ echo "Can connect, so untie happened:" $dbase2[a]
+ zuntie dbase2
+0:Test of automatic untie (use of local scope) and of scoping
+>association-local-special
+>fromarray
+>Can connect, so untie happened: b
+
+ echo $zgdbm_tied ${#zgdbm_tied}
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $zgdbm_tied ${#zgdbm_tied}
+ ztie -d db/gdbm -f ${dbfile}2 dbase2
+ echo $zgdbm_tied ${#zgdbm_tied}
+ zuntie -u dbase
+ echo $zgdbm_tied ${#zgdbm_tied}
+ zuntie dbase2
+ echo $zgdbm_tied ${#zgdbm_tied}
+0:zgdbm_tied parameter
+>0
+>dbase 1
+>dbase dbase2 2
+>dbase2 1
+>0
+
+ unset zgdbm_tied 2>/dev/null
+1:unset of read-only zgdbm_tied parameter
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase[æ¼¢å—]=æ¼¢å—
+ echo $dbase[æ¼¢å—]
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[æ¼¢å—]
+ zuntie -u dbase
+0:Unicode test
+>æ¼¢å—
+>æ¼¢å—
+
+ key="ab"$'\0'"ef"
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase[$key]=value
+ echo $dbase[$key]
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[$key]
+ zuntie -u dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase[$key]=$key
+ zuntie dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ [[ "$dbase[$key]" = "$key" ]] && echo correct
+ zuntie dbase
+0:Metafication of $'\0'
+>value
+>value
+>correct
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase=( æ¼¢å— æ¼¢å— )
+ echo $dbase[æ¼¢å—]
+ zuntie dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ echo $dbase[æ¼¢å—]
+ zuntie dbase
+ key="ab"$'\0'"ef"
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase+=( $key $key )
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ [[ "$dbase[$key]" = "$key" ]] && echo correct
+ zuntie -u dbase
+0:Unicode & metafication test, different hash access
+>æ¼¢å—
+>æ¼¢å—
+>correct
+
+%clean
+
+ rm -f ${dbfile}*
[-- Attachment #3: V11db_gdbm.ztst.txt --]
[-- Type: text/plain, Size: 5554 bytes --]
# Tests for the zsh/param/private module
%prep
modname="zsh/db/gdbm"
dbfile=db.gdbm
if ! zmodload $modname ; then
ZTST_unimplemented="can't load $modname module for testing"
fi
rm -f db.gdbm
%test
(zmodload -u $modname && zmodload $modname)
0:unload and reload the module without crashing
ztie -d db/gdbm -f $dbfile dbase
zuntie dbase
0:create the database
ztie -r -d db/gdbm -f $dbfile dbase
zuntie -u dbase
0:open the database read-only
ztie -d db/gdbm -f $dbfile dbase
dbase[testkey]=testdata
zuntie dbase
ztie -r -d db/gdbm -f $dbfile dbase
echo $dbase[testkey]
zuntie -u dbase
0:store key in database
>testdata
ztie -d db/gdbm -f $dbfile dbase2
unset 'dbase2[testkey]'
zuntie dbase2
ztie -d db/gdbm -f $dbfile dbase
echo $dbase[testkey]
zuntie dbase
0:remove key from database (different variables)
>
ztie -d db/gdbm -f $dbfile dbase
dbase[testkey]=testdata
zuntie dbase
ztie -r -d db/gdbm -f $dbfile dbase
echo $dbase[testkey]
zuntie -u dbase
ztie -d db/gdbm -f $dbfile dbase
unset 'dbase[testkey]'
zuntie dbase
ztie -r -d db/gdbm -f $dbfile dbase
echo $dbase[testkey]
zuntie -u dbase
0:store & remove key from database (the same variables)
>testdata
>
ztie -d db/gdbm -f $dbfile dbase
dbase[testkey]=testdata
dbase[testkey2]=$dbase[testkey]
dbase[testkey3]=$dbase[testkey]x$dbase[testkey2]
zuntie dbase
ztie -d db/gdbm -f $dbfile dbase
echo $dbase[testkey]
echo $dbase[testkey2]
echo $dbase[testkey3]
zuntie dbase
0:store 2 keys fetching 1st
>testdata
>testdata
>testdataxtestdata
ztie -d db/gdbm -f $dbfile dbase
val=$dbase[testkey2]
unset 'dbase[testkey2]'
echo $val
zuntie dbase
0:unset key that was fetched
>testdata
ztie -r -d db/gdbm -f $dbfile dbase
print -rl -- "${(kv)dbase[@]}"
zuntie -u dbase
0:scan read-only tied hash
>testkey
>testdata
>testkey3
>testdataxtestdata
ztie -d db/gdbm -f $dbfile dbase
local -a arr
arr=( "${(kv)dbase[@]}" )
print -rl -- "${arr[@]}"
zuntie dbase
0:different scan, also read-write mode
>testkey
>testdata
>testkey3
>testdataxtestdata
ztie -d db/gdbm -f $dbfile dbase
dbase=( a b c d )
zuntie dbase
ztie -d db/gdbm -f $dbfile dbase
print -rl -- "${(kv)dbase[@]}"
zuntie dbase
0:replace hash / database, scan
>c
>d
>a
>b
ztie -d db/gdbm -f $dbfile dbase
local -a arr
arr=( "${dbase[@]}" )
print -rl -- "${arr[@]}"
zuntie dbase
0:scan with no (kv)
>d
>b
ztie -d db/gdbm -f $dbfile dbase
print -rl -- "${(k)dbase[@]}"
zuntie dbase
0:scan with keys only (k)
>c
>a
ztie -d db/gdbm -f $dbfile dbase
print -rl -- "${(v)dbase[@]}"
zuntie dbase
0:scan with keys only explicit (v)
>d
>b
rm -f $dbfile
ztie -r -d db/gdbm -f $dbfile dbase 2>/dev/null
1:read-only open non-existent database
ztie -d db/gdbm -f $dbfile dbase
dbase+=( a b )
echo $dbase[a]
zuntie dbase
ztie -r -d db/gdbm -f $dbfile dbase
echo $dbase[a]
print -rl -- "${(kv)dbase[@]}"
zuntie -u dbase
ztie -d db/gdbm -f $dbfile dbase
dbase+=( c d )
echo $dbase[a]
echo $dbase[c]
print -rl -- "${(kv)dbase[@]}"
zuntie dbase
ztie -r -d db/gdbm -f $dbfile dbase
echo $dbase[a]
echo $dbase[c]
print -rl -- "${(kv)dbase[@]}"
zuntie -u dbase
0:Append with +=( ), also with existing data, also (kv) scan
>b
>b
>a
>b
>b
>d
>c
>d
>a
>b
>b
>d
>c
>d
>a
>b
ztie -d db/gdbm -f $dbfile dbase
echo ${(t)dbase}
zuntie dbase
0:Type of tied parameter
>association-special
typeset -ga dbase
ztie -d db/gdbm -f $dbfile dbase
echo ${(t)dbase}
zuntie dbase
0:Type of tied parameter, with preceding unset
>association-special
local -a dbase
ztie -d db/gdbm -f $dbfile dbase
echo ${(t)dbase}
zuntie dbase
0:Type of tied parameter, with local parameter already existing
>association-local-special
local -a dbase
dbase=( fromarray )
() {
local -a dbase
ztie -d db/gdbm -f $dbfile dbase
echo ${(t)dbase}
zuntie dbase
}
echo $dbase[1]
ztie -d db/gdbm -f $dbfile dbase2
echo "Can connect, so untie happened:" $dbase2[a]
zuntie dbase2
0:Test of automatic untie (use of local scope) and of scoping
>association-local-special
>fromarray
>Can connect, so untie happened: b
echo $zgdbm_tied ${#zgdbm_tied}
ztie -r -d db/gdbm -f $dbfile dbase
echo $zgdbm_tied ${#zgdbm_tied}
ztie -d db/gdbm -f ${dbfile}2 dbase2
echo $zgdbm_tied ${#zgdbm_tied}
zuntie -u dbase
echo $zgdbm_tied ${#zgdbm_tied}
zuntie dbase2
echo $zgdbm_tied ${#zgdbm_tied}
0:zgdbm_tied parameter
>0
>dbase 1
>dbase dbase2 2
>dbase2 1
>0
unset zgdbm_tied 2>/dev/null
1:unset of read-only zgdbm_tied parameter
ztie -d db/gdbm -f $dbfile dbase
dbase[æ¼¢å—]=æ¼¢å—
echo $dbase[æ¼¢å—]
zuntie dbase
ztie -r -d db/gdbm -f $dbfile dbase
echo $dbase[æ¼¢å—]
zuntie -u dbase
0:Unicode test
>æ¼¢å—
>æ¼¢å—
key="ab"$'\0'"ef"
ztie -d db/gdbm -f $dbfile dbase
dbase[$key]=value
echo $dbase[$key]
zuntie dbase
ztie -r -d db/gdbm -f $dbfile dbase
echo $dbase[$key]
zuntie -u dbase
ztie -d db/gdbm -f $dbfile dbase
dbase[$key]=$key
zuntie dbase
ztie -d db/gdbm -f $dbfile dbase
[[ "$dbase[$key]" = "$key" ]] && echo correct
zuntie dbase
0:Metafication of $'\0'
>value
>value
>correct
ztie -d db/gdbm -f $dbfile dbase
dbase=( æ¼¢å— æ¼¢å— )
echo $dbase[æ¼¢å—]
zuntie dbase
ztie -d db/gdbm -f $dbfile dbase
echo $dbase[æ¼¢å—]
zuntie dbase
key="ab"$'\0'"ef"
ztie -d db/gdbm -f $dbfile dbase
dbase+=( $key $key )
zuntie dbase
ztie -r -d db/gdbm -f $dbfile dbase
[[ "$dbase[$key]" = "$key" ]] && echo correct
zuntie -u dbase
0:Unicode & metafication test, different hash access
>æ¼¢å—
>æ¼¢å—
>correct
%clean
rm -f ${dbfile}*
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] db/gdbm rewrite
2017-02-15 10:22 ` Sebastian Gniazdowski
@ 2017-02-16 10:16 ` Peter Stephenson
2017-02-16 11:46 ` Sebastian Gniazdowski
0 siblings, 1 reply; 15+ messages in thread
From: Peter Stephenson @ 2017-02-16 10:16 UTC (permalink / raw)
To: zsh-workers
On Wed, 15 Feb 2017 02:22:43 -0800
Sebastian Gniazdowski <psprint2@fastmail.com> wrote:
> Changes:
> – data is (un)metafied – compatibility with normal database usage
> – new parameter "zgdbm_tied", array holding tied parameters' names
> – new builtin "zgdmpath", returning ($REPLY) path to database file, of
> given tied parameter
> – new tests (Unicode, metafication, new parameter)
> – updated documentation
>
> Original db_gdbm was a no-go because it sucked widget names into
> database:
>
> https://asciinema.org/a/12ffm5ltpj9udnmradtat17am
>
> Current db_gdbm is a no-go because GDBM doesn't always return \0
> terminated strings.. Sorry for that, but the update solves this.
>
> I sent is as patch, plus the new file (as option, patch includes)
> Test/V11db_gdbm.ztst.
Thanks.
I've fixed the following warnings (which are trivial).
I'm also getting a test failure from allocation. My gdbm is quite old,
1.8.3, but it also looks like I have --enable-zsh-mem, which you don't,
as well as --enable-zsh-debug, so the difference is more likely to be
that. Just running the test code on its own is enough to show it up.
The error from the debug is on the line
dbase[testkey]=testdata
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?
pws
db_gdbm.c: In function ‘bin_zgdbmpath’:
db_gdbm.c:231:15: warning: unused variable ‘reply_pm’ [-Wunused-variable]
Param pm, reply_pm;
^~~~~~~~
db_gdbm.c: In function ‘scangdbmkeys’:
db_gdbm.c:437:16: warning: unused variable ‘content’ [-Wunused-variable]
datum key, content;
^~~~~~~
db_gdbm.c:436:11: warning: unused variable ‘pm’ [-Wunused-variable]
Param pm = NULL;
^~
% make TESTNUM=V11 ZTST_verbose=1
if test -n "gcc"; then \
cd .. && DESTDIR= \
make MODDIR=`pwd`/Test/Modules install.modules > /dev/null; \
fi
if ZTST_testlist="`for f in ./V11*.ztst; \
do echo $f; done`" \
ZTST_srcdir="." \
ZTST_exe=../Src/zsh \
../Src/zsh +Z -f ./runtests.zsh; then \
stat=0; \
else \
stat=1; \
fi; \
sleep 1; \
rm -rf Modules .zcompdump; \
exit $stat
./V11db_gdbm.ztst: starting.
Running test: unload and reload the module without crashing
Test successful.
Running test: create the database
Test successful.
Running test: open the database read-only
Test successful.
Running test: store key in database
--- /tmp/zsh.ztst.19816/ztst.err 2017-02-16 09:58:41.220556989 +0000
+++ /tmp/zsh.ztst.19816/ztst.terr 2017-02-16 09:58:41.258558381 +0000
@@ -0,0 +1 @@
+6: mem.c:1512: BUG: attempt to free more than allocated.
Test ./V11db_gdbm.ztst failed: error output differs from expected as shown above for:
ztie -d db/gdbm -f $dbfile dbase
dbase[testkey]=testdata
zuntie dbase
ztie -r -d db/gdbm -f $dbfile dbase
echo $dbase[testkey]
zuntie -u dbase
Was testing: store key in database
./V11db_gdbm.ztst: test failed.
**************************************
0 successful test scripts, 1 failure, 0 skipped
**************************************
make: *** [check] Error 1
#0 zfree (p=0x8733c30, sz=0) at mem.c:1512
#1 0x080a4ce4 in zsfree (p=0x8733c30 "testdata\030<s\b\377\377\377\377H<s\b\377\377\377\377P<s\b\377\377\377\377h<s\b\377\377\377\377dbase") at mem.c:1662
#2 0x00668a15 in gdbmsetfn (pm=0x8737a90, val=0x8731998 "testdata") at db_gdbm.c:348
#3 0x080b1c1c in assignstrvalue (v=0xbfbd4cdc, val=0x8731998 "testdata", flags=0) at params.c:2456
#4 0x080b3629 in assignsparam (s=0xb775abad "[testkey]", val=0x8731998 "testdata", flags=0) at params.c:3035
#5 0x08070c6c in addvars (state=0xbfbd50a0, pc=0xb775ab30, addflags=0) at exec.c:2462
#6 0x0806d5a4 in execsimple (state=0xbfbd50a0) at exec.c:1173
#7 0x0806daa6 in execlist (state=0xbfbd50a0, dont_change_job=0, exiting=0) at exec.c:1312
#8 0x0806d473 in execode (p=0xb775ab00, dont_change_job=0, exiting=0, context=0x80edc37 "toplevel") at exec.c:1130
#9 0x0808ccdd in loop (toplevel=1, justonce=0) at init.c:208
#10 0x080901d8 in zsh_main (argc=1, argv=0xbfbd5344) at init.c:1692
#11 0x0805433c in main (argc=1, argv=0xbfbd5344) at ./main.c:93
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] db/gdbm rewrite
2017-02-16 10:16 ` Peter Stephenson
@ 2017-02-16 11:46 ` Sebastian Gniazdowski
2017-02-16 12:52 ` Peter Stephenson
0 siblings, 1 reply; 15+ messages in thread
From: Sebastian Gniazdowski @ 2017-02-16 11:46 UTC (permalink / raw)
To: zsh-workers
On Thu, Feb 16, 2017, at 02:16 AM, Peter Stephenson wrote:
> On Wed, 15 Feb 2017 02:22:43 -0800
> Sebastian Gniazdowski <psprint2@fastmail.com> 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@fastmail.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] db/gdbm rewrite
2017-02-16 11:46 ` Sebastian Gniazdowski
@ 2017-02-16 12:52 ` Peter Stephenson
2017-02-16 14:25 ` Sebastian Gniazdowski
0 siblings, 1 reply; 15+ messages in thread
From: Peter Stephenson @ 2017-02-16 12:52 UTC (permalink / raw)
To: zsh-workers
On Thu, 16 Feb 2017 03:46:15 -0800
Sebastian Gniazdowski <psprint2@fastmail.com> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] db/gdbm rewrite
2017-02-16 12:52 ` Peter Stephenson
@ 2017-02-16 14:25 ` Sebastian Gniazdowski
2017-02-16 14:30 ` Sebastian Gniazdowski
0 siblings, 1 reply; 15+ messages in thread
From: Sebastian Gniazdowski @ 2017-02-16 14:25 UTC (permalink / raw)
To: zsh-workers
[-- Attachment #1: Type: text/plain, Size: 1769 bytes --]
On Thu, Feb 16, 2017, at 04:52 AM, Peter Stephenson wrote:
> 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.
This leads to something. The zsfree() backtrace might be late effect of
previous incorrect zsfree(). I did following change:
- /* Unmetafy */
- char *umval = ztrdup(val);
- umval = unmetafy(umval,¨en);
+ /* Unmetafy with exact zalloc size */
+ char *umval = unmetafy_zalloc(val,¨en);
in multiple unmetafy-places. Otherwise strlen() is shorter than actual
buffer size. Sending as incremental patch including your warnings catch,
also as complete patch, and as db_gdbm.c file.
The added function:
/*
* 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;
}
--
Sebastian Gniazdowski
psprint2@fastmail.com
[-- Attachment #2: db_gdbm_incremental.diff --]
[-- Type: text/plain, Size: 4304 bytes --]
diff --git a/Src/Modules/db_gdbm.c b/Src/Modules/db_gdbm.c
index 3eef4c6..0a16540 100644
--- a/Src/Modules/db_gdbm.c
+++ b/Src/Modules/db_gdbm.c
@@ -41,6 +41,7 @@
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
@@ -228,7 +229,7 @@ bin_zuntie(char *nam, char **args, Options ops, UNUSED(int func))
static int
bin_zgdbmpath(char *nam, char **args, Options ops, UNUSED(int func))
{
- Param pm, reply_pm;
+ Param pm;
char *pmname;
pmname = *args;
@@ -294,9 +295,8 @@ gdbmgetfn(Param pm)
/* Unmetafy key. GDBM fits nice into this
* process, as it uses length of data */
- char *umkey = ztrdup(pm->node.nam);
int umlen = 0;
- umkey = unmetafy(umkey,¨en);
+ char *umkey = unmetafy_zalloc(pm->node.nam,¨en);
key.dptr = umkey;
key.dsize = umlen;
@@ -318,7 +318,7 @@ gdbmgetfn(Param pm)
* can obtain data length to avoid using \0 */
pm->u.str = metafy(content.dptr, content.dsize, META_ALLOC);
- /* Free key */
+ /* Free key, restoring its original length */
zsfree(umkey);
/* Can return pointer, correctly saved inside hash */
@@ -358,17 +358,15 @@ gdbmsetfn(Param pm, char *val)
/* Database */
dbf = ((struct gsu_scalar_ext *)pm->gsu.s)->dbf;
if (dbf) {
- char *umkey = ztrdup(pm->node.nam);
int umlen = 0;
- umkey = unmetafy(umkey,¨en);
+ char *umkey = unmetafy_zalloc(pm->node.nam,¨en);
key.dptr = umkey;
key.dsize = umlen;
if (val) {
- /* Unmetafy */
- char *umval = ztrdup(val);
- umval = unmetafy(umval,¨en);
+ /* Unmetafy with exact zalloc size */
+ char *umval = unmetafy_zalloc(val,¨en);
/* Store */
content.dptr = umval;
@@ -433,8 +431,7 @@ getgdbmnode(HashTable ht, const char *name)
static void
scangdbmkeys(HashTable ht, ScanFunc func, int flags)
{
- Param pm = NULL;
- datum key, content;
+ datum key;
GDBM_FILE dbf = ((struct gsu_scalar_ext *)ht->tmpdata)->dbf;
/* Iterate keys adding them to hash, so
@@ -505,9 +502,8 @@ gdbmhashsetfn(Param pm, HashTable ht)
v.pm = (Param) hn;
/* Unmetafy key */
- char *umkey = ztrdup(v.pm->node.nam);
int umlen = 0;
- umkey = unmetafy(umkey,¨en);
+ char *umkey = unmetafy_zalloc(v.pm->node.nam,¨en);
key.dptr = umkey;
key.dsize = umlen;
@@ -515,15 +511,16 @@ gdbmhashsetfn(Param pm, HashTable ht)
queue_signals();
/* Unmetafy */
- char *umval = ztrdup(getstrvalue(&v));
- umval = unmetafy(umval,¨en);
+ char *umval = unmetafy_zalloc(getstrvalue(&v),¨en);
/* Store */
content.dptr = umval;
content.dsize = umlen;
(void)gdbm_store(dbf, key, content, GDBM_REPLACE);
- /* Free */
+ /* Free - thanks to unmetafy_zalloc size of
+ * the strings is exact zalloc size - can
+ * pass to zsfree */
zsfree(umval);
zsfree(umkey);
@@ -739,3 +736,33 @@ static int remove_tied_name( const char *name ) {
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;
+}
[-- Attachment #3: db_gdbm_rewrite3.diff --]
[-- Type: text/plain, Size: 26831 bytes --]
diff --git a/Doc/Zsh/mod_db_gdbm.yo b/Doc/Zsh/mod_db_gdbm.yo
index 9097429..699e9ab 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 310e329..0a16540 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;
}
@@ -162,6 +226,53 @@ bin_zuntie(char *nam, char **args, Options ops, UNUSED(int func))
}
/**/
+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,¨en);
+
+ 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_ALLOC);
+
+ /* 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,¨en);
- 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,¨en);
+
+ /* 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 = ztrduppfx(key.dptr, key.dsize);
+ 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,¨en);
+
+ key.dptr = umkey;
+ key.dsize = umlen;
queue_signals();
- content.dptr = getstrvalue(&v);
- content.dsize = strlen(content.dptr);
+ /* Unmetafy */
+ char *umval = unmetafy_zalloc(getstrvalue(&v),¨en);
+ /* 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 ce7926b..210c221 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"
diff --git a/Test/V11db_gdbm.ztst b/Test/V11db_gdbm.ztst
new file mode 100644
index 0000000..a1076dc
--- /dev/null
+++ b/Test/V11db_gdbm.ztst
@@ -0,0 +1,285 @@
+# Tests for the zsh/param/private module
+
+%prep
+
+ modname="zsh/db/gdbm"
+ dbfile=db.gdbm
+ if ! zmodload $modname ; then
+ ZTST_unimplemented="can't load $modname module for testing"
+ fi
+ rm -f db.gdbm
+
+%test
+
+ (zmodload -u $modname && zmodload $modname)
+0:unload and reload the module without crashing
+
+ ztie -d db/gdbm -f $dbfile dbase
+ zuntie dbase
+0:create the database
+
+ ztie -r -d db/gdbm -f $dbfile dbase
+ zuntie -u dbase
+0:open the database read-only
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase[testkey]=testdata
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[testkey]
+ zuntie -u dbase
+0:store key in database
+>testdata
+
+ ztie -d db/gdbm -f $dbfile dbase2
+ unset 'dbase2[testkey]'
+ zuntie dbase2
+ ztie -d db/gdbm -f $dbfile dbase
+ echo $dbase[testkey]
+ zuntie dbase
+0:remove key from database (different variables)
+>
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase[testkey]=testdata
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[testkey]
+ zuntie -u dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ unset 'dbase[testkey]'
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[testkey]
+ zuntie -u dbase
+0:store & remove key from database (the same variables)
+>testdata
+>
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase[testkey]=testdata
+ dbase[testkey2]=$dbase[testkey]
+ dbase[testkey3]=$dbase[testkey]x$dbase[testkey2]
+ zuntie dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ echo $dbase[testkey]
+ echo $dbase[testkey2]
+ echo $dbase[testkey3]
+ zuntie dbase
+0:store 2 keys fetching 1st
+>testdata
+>testdata
+>testdataxtestdata
+
+ ztie -d db/gdbm -f $dbfile dbase
+ val=$dbase[testkey2]
+ unset 'dbase[testkey2]'
+ echo $val
+ zuntie dbase
+0:unset key that was fetched
+>testdata
+
+ ztie -r -d db/gdbm -f $dbfile dbase
+ print -rl -- "${(kv)dbase[@]}"
+ zuntie -u dbase
+0:scan read-only tied hash
+>testkey
+>testdata
+>testkey3
+>testdataxtestdata
+
+ ztie -d db/gdbm -f $dbfile dbase
+ local -a arr
+ arr=( "${(kv)dbase[@]}" )
+ print -rl -- "${arr[@]}"
+ zuntie dbase
+0:different scan, also read-write mode
+>testkey
+>testdata
+>testkey3
+>testdataxtestdata
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase=( a b c d )
+ zuntie dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ print -rl -- "${(kv)dbase[@]}"
+ zuntie dbase
+0:replace hash / database, scan
+>c
+>d
+>a
+>b
+
+ ztie -d db/gdbm -f $dbfile dbase
+ local -a arr
+ arr=( "${dbase[@]}" )
+ print -rl -- "${arr[@]}"
+ zuntie dbase
+0:scan with no (kv)
+>d
+>b
+
+ ztie -d db/gdbm -f $dbfile dbase
+ print -rl -- "${(k)dbase[@]}"
+ zuntie dbase
+0:scan with keys only (k)
+>c
+>a
+
+ ztie -d db/gdbm -f $dbfile dbase
+ print -rl -- "${(v)dbase[@]}"
+ zuntie dbase
+0:scan with keys only explicit (v)
+>d
+>b
+
+ rm -f $dbfile
+ ztie -r -d db/gdbm -f $dbfile dbase 2>/dev/null
+1:read-only open non-existent database
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase+=( a b )
+ echo $dbase[a]
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[a]
+ print -rl -- "${(kv)dbase[@]}"
+ zuntie -u dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase+=( c d )
+ echo $dbase[a]
+ echo $dbase[c]
+ print -rl -- "${(kv)dbase[@]}"
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[a]
+ echo $dbase[c]
+ print -rl -- "${(kv)dbase[@]}"
+ zuntie -u dbase
+0:Append with +=( ), also with existing data, also (kv) scan
+>b
+>b
+>a
+>b
+>b
+>d
+>c
+>d
+>a
+>b
+>b
+>d
+>c
+>d
+>a
+>b
+
+ ztie -d db/gdbm -f $dbfile dbase
+ echo ${(t)dbase}
+ zuntie dbase
+0:Type of tied parameter
+>association-special
+
+ typeset -ga dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ echo ${(t)dbase}
+ zuntie dbase
+0:Type of tied parameter, with preceding unset
+>association-special
+
+ local -a dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ echo ${(t)dbase}
+ zuntie dbase
+0:Type of tied parameter, with local parameter already existing
+>association-local-special
+
+ local -a dbase
+ dbase=( fromarray )
+ () {
+ local -a dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ echo ${(t)dbase}
+ zuntie dbase
+ }
+ echo $dbase[1]
+ ztie -d db/gdbm -f $dbfile dbase2
+ echo "Can connect, so untie happened:" $dbase2[a]
+ zuntie dbase2
+0:Test of automatic untie (use of local scope) and of scoping
+>association-local-special
+>fromarray
+>Can connect, so untie happened: b
+
+ echo $zgdbm_tied ${#zgdbm_tied}
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $zgdbm_tied ${#zgdbm_tied}
+ ztie -d db/gdbm -f ${dbfile}2 dbase2
+ echo $zgdbm_tied ${#zgdbm_tied}
+ zuntie -u dbase
+ echo $zgdbm_tied ${#zgdbm_tied}
+ zuntie dbase2
+ echo $zgdbm_tied ${#zgdbm_tied}
+0:zgdbm_tied parameter
+>0
+>dbase 1
+>dbase dbase2 2
+>dbase2 1
+>0
+
+ unset zgdbm_tied 2>/dev/null
+1:unset of read-only zgdbm_tied parameter
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase[æ¼¢å—]=æ¼¢å—
+ echo $dbase[æ¼¢å—]
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[æ¼¢å—]
+ zuntie -u dbase
+0:Unicode test
+>æ¼¢å—
+>æ¼¢å—
+
+ key="ab"$'\0'"ef"
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase[$key]=value
+ echo $dbase[$key]
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[$key]
+ zuntie -u dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase[$key]=$key
+ zuntie dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ [[ "$dbase[$key]" = "$key" ]] && echo correct
+ zuntie dbase
+0:Metafication of $'\0'
+>value
+>value
+>correct
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase=( æ¼¢å— æ¼¢å— )
+ echo $dbase[æ¼¢å—]
+ zuntie dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ echo $dbase[æ¼¢å—]
+ zuntie dbase
+ key="ab"$'\0'"ef"
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase+=( $key $key )
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ [[ "$dbase[$key]" = "$key" ]] && echo correct
+ zuntie -u dbase
+0:Unicode & metafication test, different hash access
+>æ¼¢å—
+>æ¼¢å—
+>correct
+
+%clean
+
+ rm -f ${dbfile}*
[-- Attachment #4: db_gdbm.c.txt --]
[-- Type: text/plain, Size: 19494 bytes --]
/*
* db_gdbm.c - bindings for gdbm
*
* This file is part of zsh, the Z shell.
*
* 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
* purpose, provided that the above copyright notice and the following
* two paragraphs appear in all copies of this software.
*
* In no event shall Clint Adams or the Zsh Development
* Group be liable to any party for direct, indirect, special, incidental, or
* consequential damages arising out of the use of this software and its
* documentation, even if Peter Stephenson, Sven Wischnowsky and the Zsh
* Development Group have been advised of the possibility of such damage.
*
* Clint Adams and the Zsh Development Group
* specifically disclaim any warranties, including, but not limited to, the
* implied warranties of merchantability and fitness for a particular purpose.
* The software provided hereunder is on an "as is" basis, and Peter
* Stephenson, Sven Wischnowsky and the Zsh Development Group have no
* obligation to provide maintenance, support, updates, enhancements, or
* modifications.
*
*/
#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.
*/
#if defined(HAVE_GDBM_H) && defined(HAVE_GDBM_OPEN)
#include <gdbm.h>
static char *backtype = "db/gdbm";
/*
* 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 };
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 ),
};
/**/
static int
bin_ztie(char *nam, char **args, Options ops, UNUSED(int func))
{
char *resource_name, *pmname;
GDBM_FILE dbf = NULL;
int read_write = GDBM_SYNC, pmflags = PM_REMOVABLE;
Param tied_param;
if(!OPT_ISSET(ops,'d')) {
zwarnnam(nam, "you must pass `-d %s'", backtype);
return 1;
}
if(!OPT_ISSET(ops,'f')) {
zwarnnam(nam, "you must pass `-f' with a filename", NULL);
return 1;
}
if (OPT_ISSET(ops,'r')) {
read_write |= GDBM_READER;
pmflags |= PM_READONLY;
} else {
read_write |= GDBM_WRCREAT;
}
/* Here should be a lookup of the backend type against
* 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;
}
resource_name = OPT_ARG(ops, 'f');
pmname = *args;
if ((tied_param = (Param)paramtab->getnode(paramtab, pmname)) &&
!(tied_param->node.flags & PM_UNSET)) {
/*
* Unset any existing parameter. Note there's no implicit
* "local" here, but if the existing parameter is local
* 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.
*
* This can fail if the variable is readonly or restricted.
* We could call unsetparam() and check errflag instead
* of the return status.
*/
if (unsetparam_pm(tied_param, 0, 1))
return 1;
}
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);
return 1;
}
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;
}
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->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;
}
/**/
static int
bin_zuntie(char *nam, char **args, Options ops, UNUSED(int func))
{
Param pm;
char *pmname;
int ret = 0;
for (pmname = *args; *args++; pmname = *args) {
pm = (Param) paramtab->getnode(paramtab, pmname);
if(!pm) {
zwarnnam(nam, "cannot untie %s", pmname);
ret = 1;
continue;
}
if (pm->gsu.h != &gdbm_hash_gsu) {
zwarnnam(nam, "not a tied gdbm hash: %s", pmname);
ret = 1;
continue;
}
queue_signals();
if (OPT_ISSET(ops,'u'))
gdbmuntie(pm); /* clear read-only-ness */
if (unsetparam_pm(pm, 0, 1)) {
/* assume already reported */
ret = 1;
}
unqueue_signals();
}
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)
{
datum key, content;
int ret;
GDBM_FILE dbf;
/* 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,¨en);
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;
content = gdbm_fetch(dbf, key);
/* 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_ALLOC);
/* Free key, restoring its original length */
zsfree(umkey);
/* Can return pointer, correctly saved inside hash */
return pm->u.str;
}
/* Free key */
zsfree(umkey);
/* Can this be "" ? */
return (char *) hcalloc(1);
}
/**/
static void
gdbmsetfn(Param pm, char *val)
{
datum key, content;
GDBM_FILE dbf;
/* 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,¨en);
key.dptr = umkey;
key.dsize = umlen;
if (val) {
/* Unmetafy with exact zalloc size */
char *umval = unmetafy_zalloc(val,¨en);
/* 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))
{
/* Set with NULL */
gdbmsetfn(pm, NULL);
}
/**/
static HashNode
getgdbmnode(HashTable ht, const char *name)
{
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 (HashNode) val_pm;
}
/**/
static void
scangdbmkeys(HashTable ht, ScanFunc func, int flags)
{
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) {
/* This returns database-interfacing Param,
* it will return u.str or first fetch data
* if not PM_UPTODATE (newly created) */
char *zkey = ztrduppfx(key.dptr, key.dsize);
HashNode hn = getgdbmnode(ht, zkey);
zsfree( zkey );
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)
{
int i;
HashNode hn;
GDBM_FILE dbf;
datum key, content;
if (!pm->u.hash || pm->u.hash == ht)
return;
if (!(dbf = ((struct gsu_scalar_ext *)pm->u.hash->tmpdata)->dbf))
return;
key = gdbm_firstkey(dbf);
while (key.dptr) {
queue_signals();
(void)gdbm_delete(dbf, key);
free(key.dptr);
unqueue_signals();
key = gdbm_firstkey(dbf);
}
/* just deleted everything, clean up */
(void)gdbm_reorganize(dbf);
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;
v.isarr = v.flags = v.start = 0;
v.end = -1;
v.arr = NULL;
v.pm = (Param) hn;
/* Unmetafy key */
int umlen = 0;
char *umkey = unmetafy_zalloc(v.pm->node.nam,¨en);
key.dptr = umkey;
key.dsize = umlen;
queue_signals();
/* Unmetafy */
char *umval = unmetafy_zalloc(getstrvalue(&v),¨en);
/* 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();
}
}
/**/
static void
gdbmuntie(Param pm)
{
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);
/* 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;
ht->scantab = NULL;
pm->node.flags &= ~(PM_SPECIAL|PM_READONLY);
pm->gsu.h = &stdhash_gsu;
}
/**/
static void
gdbmhashunsetfn(Param pm, UNUSED(int exp))
{
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;
/* 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;
}
#else
# error no gdbm
#endif /* have gdbm */
static struct features module_features = {
bintab, sizeof(bintab)/sizeof(*bintab),
NULL, 0,
NULL, 0,
patab, sizeof(patab)/sizeof(*patab),
0
};
/**/
int
setup_(UNUSED(Module m))
{
return 0;
}
/**/
int
features_(Module m, char ***features)
{
*features = featuresarray(m, &module_features);
return 0;
}
/**/
int
enables_(Module m, int **enables)
{
return handlefeatures(m, &module_features, enables);
}
/**/
int
boot_(UNUSED(Module m))
{
zgdbm_tied = zshcalloc((1) * sizeof(char *));
return 0;
}
/**/
int
cleanup_(Module m)
{
/* This frees `zgdbm_tied` */
return setfeatureenables(m, &module_features, NULL);
}
/**/
int
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;
}
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] db/gdbm rewrite
2017-02-16 14:25 ` Sebastian Gniazdowski
@ 2017-02-16 14:30 ` Sebastian Gniazdowski
2017-02-16 15:11 ` Peter Stephenson
0 siblings, 1 reply; 15+ messages in thread
From: Sebastian Gniazdowski @ 2017-02-16 14:30 UTC (permalink / raw)
To: zsh-workers
PS. Except "testkey" and "testval" don't have meta-characters.. But the
patch for sure is needed. I've checked that length at "testdata" storing
is 8+1.
--
Sebastian Gniazdowski
psprint2@fastmail.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] db/gdbm rewrite
2017-02-16 14:30 ` Sebastian Gniazdowski
@ 2017-02-16 15:11 ` Peter Stephenson
2017-02-16 16:03 ` Sebastian Gniazdowski
0 siblings, 1 reply; 15+ messages in thread
From: Peter Stephenson @ 2017-02-16 15:11 UTC (permalink / raw)
To: zsh-workers
On Thu, 16 Feb 2017 06:30:50 -0800
Sebastian Gniazdowski <psprint2@fastmail.com> wrote:
> PS. Except "testkey" and "testval" don't have meta-characters.. But the
> patch for sure is needed. I've checked that length at "testdata" storing
> is 8+1.
Yes, it doesn't fix the problem, and it looks like it's a bit more
subtle than simply a wrong length.
I've managed to get valgrind reporting errors but with and without
--enable-zsh-mem. Here's one without: --enable-zsh-debug is on
but that's all.
I'm afraid that's all I'm going to have time to do.
pws
% zmodload zsh/db/gdbm
% ztie -d db/gdbm -f db.gdbm dbase
% dbase[testkey]=somewhat
==25034== Invalid write of size 1
==25034== at 0x80E0B78: metafy (utils.c:4652)
==25034== by 0x465CA27: gdbmgetfn (db_gdbm.c:319)
==25034== by 0x80AE5FF: getstrvalue (params.c:2120)
==25034== by 0x80AC838: getarg (params.c:1414)
==25034== by 0x80ADCE6: getindex (params.c:1814)
==25034== by 0x80AE345: fetchvalue (params.c:2030)
==25034== by 0x80AE02B: getvalue (params.c:1951)
==25034== by 0x80B05DF: assignsparam (params.c:2956)
==25034== by 0x8070BEB: addvars (exec.c:2462)
==25034== by 0x806D523: execsimple (exec.c:1173)
==25034== by 0x806DA25: execlist (exec.c:1312)
==25034== by 0x806D3F2: execode (exec.c:1130)
==25034== Address 0x41922b8 is 0 bytes after a block of size 8 alloc'd
==25034== at 0x400677E: malloc (vg_replace_malloc.c:195)
==25034== by 0x102AF2: gdbm_fetch (gdbmfetch.c:68)
==25034== by 0x465C9F6: gdbmgetfn (db_gdbm.c:310)
==25034== by 0x80AE5FF: getstrvalue (params.c:2120)
==25034== by 0x80AC838: getarg (params.c:1414)
==25034== by 0x80ADCE6: getindex (params.c:1814)
==25034== by 0x80AE345: fetchvalue (params.c:2030)
==25034== by 0x80AE02B: getvalue (params.c:1951)
==25034== by 0x80B05DF: assignsparam (params.c:2956)
==25034== by 0x8070BEB: addvars (exec.c:2462)
==25034== by 0x806D523: execsimple (exec.c:1173)
==25034== by 0x806DA25: execlist (exec.c:1312)
==25034==
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] db/gdbm rewrite
2017-02-16 15:11 ` Peter Stephenson
@ 2017-02-16 16:03 ` Sebastian Gniazdowski
2017-02-16 16:25 ` Sebastian Gniazdowski
2017-02-16 18:16 ` Sebastian Gniazdowski
0 siblings, 2 replies; 15+ messages in thread
From: Sebastian Gniazdowski @ 2017-02-16 16:03 UTC (permalink / raw)
To: zsh-workers
[-- Attachment #1: Type: text/plain, Size: 865 bytes --]
On Thu, Feb 16, 2017, at 07:11 AM, Peter Stephenson wrote:
> Yes, it doesn't fix the problem, and it looks like it's a bit more
> subtle than simply a wrong length.
>
> I've managed to get valgrind reporting errors but with and without
> --enable-zsh-mem. Here's one without: --enable-zsh-debug is on
> but that's all.
>
> I'm afraid that's all I'm going to have time to do.
>
> pws
>
> % zmodload zsh/db/gdbm
> % ztie -d db/gdbm -f db.gdbm dbase
> % dbase[testkey]=somewhat
> ==25034== Invalid write of size 1
> ==25034== at 0x80E0B78: metafy (utils.c:4652)
I've used HEAP_ALLOC instead of HEAP_DUP in argument for metafy. Fixing
this resolves valgrind report. Important catch.. Hopefully it will
resolve enable-zsh-mem problem (I cannot reproduce that). Sending as
complete patch plus db_gdbm.c file.
--
Sebastian Gniazdowski
psprint2@fastmail.com
[-- Attachment #2: db_gdbm_rewrite4.diff --]
[-- Type: text/plain, Size: 26829 bytes --]
diff --git a/Doc/Zsh/mod_db_gdbm.yo b/Doc/Zsh/mod_db_gdbm.yo
index 9097429..699e9ab 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 310e329..8a61b2f 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;
}
@@ -162,6 +226,53 @@ bin_zuntie(char *nam, char **args, Options ops, UNUSED(int func))
}
/**/
+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,¨en);
+
+ 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,¨en);
- 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,¨en);
+
+ /* 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 = ztrduppfx(key.dptr, key.dsize);
+ 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,¨en);
+
+ key.dptr = umkey;
+ key.dsize = umlen;
queue_signals();
- content.dptr = getstrvalue(&v);
- content.dsize = strlen(content.dptr);
+ /* Unmetafy */
+ char *umval = unmetafy_zalloc(getstrvalue(&v),¨en);
+ /* 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 ce7926b..210c221 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"
diff --git a/Test/V11db_gdbm.ztst b/Test/V11db_gdbm.ztst
new file mode 100644
index 0000000..a1076dc
--- /dev/null
+++ b/Test/V11db_gdbm.ztst
@@ -0,0 +1,285 @@
+# Tests for the zsh/param/private module
+
+%prep
+
+ modname="zsh/db/gdbm"
+ dbfile=db.gdbm
+ if ! zmodload $modname ; then
+ ZTST_unimplemented="can't load $modname module for testing"
+ fi
+ rm -f db.gdbm
+
+%test
+
+ (zmodload -u $modname && zmodload $modname)
+0:unload and reload the module without crashing
+
+ ztie -d db/gdbm -f $dbfile dbase
+ zuntie dbase
+0:create the database
+
+ ztie -r -d db/gdbm -f $dbfile dbase
+ zuntie -u dbase
+0:open the database read-only
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase[testkey]=testdata
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[testkey]
+ zuntie -u dbase
+0:store key in database
+>testdata
+
+ ztie -d db/gdbm -f $dbfile dbase2
+ unset 'dbase2[testkey]'
+ zuntie dbase2
+ ztie -d db/gdbm -f $dbfile dbase
+ echo $dbase[testkey]
+ zuntie dbase
+0:remove key from database (different variables)
+>
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase[testkey]=testdata
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[testkey]
+ zuntie -u dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ unset 'dbase[testkey]'
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[testkey]
+ zuntie -u dbase
+0:store & remove key from database (the same variables)
+>testdata
+>
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase[testkey]=testdata
+ dbase[testkey2]=$dbase[testkey]
+ dbase[testkey3]=$dbase[testkey]x$dbase[testkey2]
+ zuntie dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ echo $dbase[testkey]
+ echo $dbase[testkey2]
+ echo $dbase[testkey3]
+ zuntie dbase
+0:store 2 keys fetching 1st
+>testdata
+>testdata
+>testdataxtestdata
+
+ ztie -d db/gdbm -f $dbfile dbase
+ val=$dbase[testkey2]
+ unset 'dbase[testkey2]'
+ echo $val
+ zuntie dbase
+0:unset key that was fetched
+>testdata
+
+ ztie -r -d db/gdbm -f $dbfile dbase
+ print -rl -- "${(kv)dbase[@]}"
+ zuntie -u dbase
+0:scan read-only tied hash
+>testkey
+>testdata
+>testkey3
+>testdataxtestdata
+
+ ztie -d db/gdbm -f $dbfile dbase
+ local -a arr
+ arr=( "${(kv)dbase[@]}" )
+ print -rl -- "${arr[@]}"
+ zuntie dbase
+0:different scan, also read-write mode
+>testkey
+>testdata
+>testkey3
+>testdataxtestdata
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase=( a b c d )
+ zuntie dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ print -rl -- "${(kv)dbase[@]}"
+ zuntie dbase
+0:replace hash / database, scan
+>c
+>d
+>a
+>b
+
+ ztie -d db/gdbm -f $dbfile dbase
+ local -a arr
+ arr=( "${dbase[@]}" )
+ print -rl -- "${arr[@]}"
+ zuntie dbase
+0:scan with no (kv)
+>d
+>b
+
+ ztie -d db/gdbm -f $dbfile dbase
+ print -rl -- "${(k)dbase[@]}"
+ zuntie dbase
+0:scan with keys only (k)
+>c
+>a
+
+ ztie -d db/gdbm -f $dbfile dbase
+ print -rl -- "${(v)dbase[@]}"
+ zuntie dbase
+0:scan with keys only explicit (v)
+>d
+>b
+
+ rm -f $dbfile
+ ztie -r -d db/gdbm -f $dbfile dbase 2>/dev/null
+1:read-only open non-existent database
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase+=( a b )
+ echo $dbase[a]
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[a]
+ print -rl -- "${(kv)dbase[@]}"
+ zuntie -u dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase+=( c d )
+ echo $dbase[a]
+ echo $dbase[c]
+ print -rl -- "${(kv)dbase[@]}"
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[a]
+ echo $dbase[c]
+ print -rl -- "${(kv)dbase[@]}"
+ zuntie -u dbase
+0:Append with +=( ), also with existing data, also (kv) scan
+>b
+>b
+>a
+>b
+>b
+>d
+>c
+>d
+>a
+>b
+>b
+>d
+>c
+>d
+>a
+>b
+
+ ztie -d db/gdbm -f $dbfile dbase
+ echo ${(t)dbase}
+ zuntie dbase
+0:Type of tied parameter
+>association-special
+
+ typeset -ga dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ echo ${(t)dbase}
+ zuntie dbase
+0:Type of tied parameter, with preceding unset
+>association-special
+
+ local -a dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ echo ${(t)dbase}
+ zuntie dbase
+0:Type of tied parameter, with local parameter already existing
+>association-local-special
+
+ local -a dbase
+ dbase=( fromarray )
+ () {
+ local -a dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ echo ${(t)dbase}
+ zuntie dbase
+ }
+ echo $dbase[1]
+ ztie -d db/gdbm -f $dbfile dbase2
+ echo "Can connect, so untie happened:" $dbase2[a]
+ zuntie dbase2
+0:Test of automatic untie (use of local scope) and of scoping
+>association-local-special
+>fromarray
+>Can connect, so untie happened: b
+
+ echo $zgdbm_tied ${#zgdbm_tied}
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $zgdbm_tied ${#zgdbm_tied}
+ ztie -d db/gdbm -f ${dbfile}2 dbase2
+ echo $zgdbm_tied ${#zgdbm_tied}
+ zuntie -u dbase
+ echo $zgdbm_tied ${#zgdbm_tied}
+ zuntie dbase2
+ echo $zgdbm_tied ${#zgdbm_tied}
+0:zgdbm_tied parameter
+>0
+>dbase 1
+>dbase dbase2 2
+>dbase2 1
+>0
+
+ unset zgdbm_tied 2>/dev/null
+1:unset of read-only zgdbm_tied parameter
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase[æ¼¢å—]=æ¼¢å—
+ echo $dbase[æ¼¢å—]
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[æ¼¢å—]
+ zuntie -u dbase
+0:Unicode test
+>æ¼¢å—
+>æ¼¢å—
+
+ key="ab"$'\0'"ef"
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase[$key]=value
+ echo $dbase[$key]
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ echo $dbase[$key]
+ zuntie -u dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase[$key]=$key
+ zuntie dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ [[ "$dbase[$key]" = "$key" ]] && echo correct
+ zuntie dbase
+0:Metafication of $'\0'
+>value
+>value
+>correct
+
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase=( æ¼¢å— æ¼¢å— )
+ echo $dbase[æ¼¢å—]
+ zuntie dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ echo $dbase[æ¼¢å—]
+ zuntie dbase
+ key="ab"$'\0'"ef"
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase+=( $key $key )
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ [[ "$dbase[$key]" = "$key" ]] && echo correct
+ zuntie -u dbase
+0:Unicode & metafication test, different hash access
+>æ¼¢å—
+>æ¼¢å—
+>correct
+
+%clean
+
+ rm -f ${dbfile}*
[-- Attachment #3: db_gdbm.c.txt --]
[-- Type: text/plain, Size: 19492 bytes --]
/*
* db_gdbm.c - bindings for gdbm
*
* This file is part of zsh, the Z shell.
*
* 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
* purpose, provided that the above copyright notice and the following
* two paragraphs appear in all copies of this software.
*
* In no event shall Clint Adams or the Zsh Development
* Group be liable to any party for direct, indirect, special, incidental, or
* consequential damages arising out of the use of this software and its
* documentation, even if Peter Stephenson, Sven Wischnowsky and the Zsh
* Development Group have been advised of the possibility of such damage.
*
* Clint Adams and the Zsh Development Group
* specifically disclaim any warranties, including, but not limited to, the
* implied warranties of merchantability and fitness for a particular purpose.
* The software provided hereunder is on an "as is" basis, and Peter
* Stephenson, Sven Wischnowsky and the Zsh Development Group have no
* obligation to provide maintenance, support, updates, enhancements, or
* modifications.
*
*/
#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.
*/
#if defined(HAVE_GDBM_H) && defined(HAVE_GDBM_OPEN)
#include <gdbm.h>
static char *backtype = "db/gdbm";
/*
* 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 };
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 ),
};
/**/
static int
bin_ztie(char *nam, char **args, Options ops, UNUSED(int func))
{
char *resource_name, *pmname;
GDBM_FILE dbf = NULL;
int read_write = GDBM_SYNC, pmflags = PM_REMOVABLE;
Param tied_param;
if(!OPT_ISSET(ops,'d')) {
zwarnnam(nam, "you must pass `-d %s'", backtype);
return 1;
}
if(!OPT_ISSET(ops,'f')) {
zwarnnam(nam, "you must pass `-f' with a filename", NULL);
return 1;
}
if (OPT_ISSET(ops,'r')) {
read_write |= GDBM_READER;
pmflags |= PM_READONLY;
} else {
read_write |= GDBM_WRCREAT;
}
/* Here should be a lookup of the backend type against
* 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;
}
resource_name = OPT_ARG(ops, 'f');
pmname = *args;
if ((tied_param = (Param)paramtab->getnode(paramtab, pmname)) &&
!(tied_param->node.flags & PM_UNSET)) {
/*
* Unset any existing parameter. Note there's no implicit
* "local" here, but if the existing parameter is local
* 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.
*
* This can fail if the variable is readonly or restricted.
* We could call unsetparam() and check errflag instead
* of the return status.
*/
if (unsetparam_pm(tied_param, 0, 1))
return 1;
}
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);
return 1;
}
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;
}
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->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;
}
/**/
static int
bin_zuntie(char *nam, char **args, Options ops, UNUSED(int func))
{
Param pm;
char *pmname;
int ret = 0;
for (pmname = *args; *args++; pmname = *args) {
pm = (Param) paramtab->getnode(paramtab, pmname);
if(!pm) {
zwarnnam(nam, "cannot untie %s", pmname);
ret = 1;
continue;
}
if (pm->gsu.h != &gdbm_hash_gsu) {
zwarnnam(nam, "not a tied gdbm hash: %s", pmname);
ret = 1;
continue;
}
queue_signals();
if (OPT_ISSET(ops,'u'))
gdbmuntie(pm); /* clear read-only-ness */
if (unsetparam_pm(pm, 0, 1)) {
/* assume already reported */
ret = 1;
}
unqueue_signals();
}
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)
{
datum key, content;
int ret;
GDBM_FILE dbf;
/* 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,¨en);
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;
content = gdbm_fetch(dbf, key);
/* 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;
}
/* Free key */
zsfree(umkey);
/* Can this be "" ? */
return (char *) hcalloc(1);
}
/**/
static void
gdbmsetfn(Param pm, char *val)
{
datum key, content;
GDBM_FILE dbf;
/* 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,¨en);
key.dptr = umkey;
key.dsize = umlen;
if (val) {
/* Unmetafy with exact zalloc size */
char *umval = unmetafy_zalloc(val,¨en);
/* 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))
{
/* Set with NULL */
gdbmsetfn(pm, NULL);
}
/**/
static HashNode
getgdbmnode(HashTable ht, const char *name)
{
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 (HashNode) val_pm;
}
/**/
static void
scangdbmkeys(HashTable ht, ScanFunc func, int flags)
{
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) {
/* This returns database-interfacing Param,
* it will return u.str or first fetch data
* if not PM_UPTODATE (newly created) */
char *zkey = ztrduppfx(key.dptr, key.dsize);
HashNode hn = getgdbmnode(ht, zkey);
zsfree( zkey );
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)
{
int i;
HashNode hn;
GDBM_FILE dbf;
datum key, content;
if (!pm->u.hash || pm->u.hash == ht)
return;
if (!(dbf = ((struct gsu_scalar_ext *)pm->u.hash->tmpdata)->dbf))
return;
key = gdbm_firstkey(dbf);
while (key.dptr) {
queue_signals();
(void)gdbm_delete(dbf, key);
free(key.dptr);
unqueue_signals();
key = gdbm_firstkey(dbf);
}
/* just deleted everything, clean up */
(void)gdbm_reorganize(dbf);
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;
v.isarr = v.flags = v.start = 0;
v.end = -1;
v.arr = NULL;
v.pm = (Param) hn;
/* Unmetafy key */
int umlen = 0;
char *umkey = unmetafy_zalloc(v.pm->node.nam,¨en);
key.dptr = umkey;
key.dsize = umlen;
queue_signals();
/* Unmetafy */
char *umval = unmetafy_zalloc(getstrvalue(&v),¨en);
/* 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();
}
}
/**/
static void
gdbmuntie(Param pm)
{
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);
/* 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;
ht->scantab = NULL;
pm->node.flags &= ~(PM_SPECIAL|PM_READONLY);
pm->gsu.h = &stdhash_gsu;
}
/**/
static void
gdbmhashunsetfn(Param pm, UNUSED(int exp))
{
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;
/* 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;
}
#else
# error no gdbm
#endif /* have gdbm */
static struct features module_features = {
bintab, sizeof(bintab)/sizeof(*bintab),
NULL, 0,
NULL, 0,
patab, sizeof(patab)/sizeof(*patab),
0
};
/**/
int
setup_(UNUSED(Module m))
{
return 0;
}
/**/
int
features_(Module m, char ***features)
{
*features = featuresarray(m, &module_features);
return 0;
}
/**/
int
enables_(Module m, int **enables)
{
return handlefeatures(m, &module_features, enables);
}
/**/
int
boot_(UNUSED(Module m))
{
zgdbm_tied = zshcalloc((1) * sizeof(char *));
return 0;
}
/**/
int
cleanup_(Module m)
{
/* This frees `zgdbm_tied` */
return setfeatureenables(m, &module_features, NULL);
}
/**/
int
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;
}
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] db/gdbm rewrite
2017-02-16 16:03 ` Sebastian Gniazdowski
@ 2017-02-16 16:25 ` Sebastian Gniazdowski
2017-02-16 16:36 ` Peter Stephenson
2017-02-16 18:16 ` Sebastian Gniazdowski
1 sibling, 1 reply; 15+ messages in thread
From: Sebastian Gniazdowski @ 2017-02-16 16:25 UTC (permalink / raw)
To: zsh-workers
On Thu, Feb 16, 2017, at 08:03 AM, Sebastian Gniazdowski wrote:
> On Thu, Feb 16, 2017, at 07:11 AM, Peter Stephenson wrote:
> > % zmodload zsh/db/gdbm
> > % ztie -d db/gdbm -f db.gdbm dbase
> > % dbase[testkey]=somewhat
> > ==25034== Invalid write of size 1
> > ==25034== at 0x80E0B78: metafy (utils.c:4652)
>
> I've used HEAP_ALLOC instead of HEAP_DUP in argument for metafy. Fixing
> this resolves valgrind report. Important catch.. Hopefully it will
> resolve enable-zsh-mem problem (I cannot reproduce that). Sending as
> complete patch plus db_gdbm.c file.
Yeah this will fix the problem. I did:
pm->u.str = metafy(content.dptr, content.dsize, META_ALLOC);
There were no meta characters so metafy() didn't allocate memory,
returned internal GDBM pointer. META_DUP always allocates. Thanks!
--
Sebastian Gniazdowski
psprint2@fastmail.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] db/gdbm rewrite
2017-02-16 16:25 ` Sebastian Gniazdowski
@ 2017-02-16 16:36 ` Peter Stephenson
2017-02-16 17:12 ` Sebastian Gniazdowski
0 siblings, 1 reply; 15+ messages in thread
From: Peter Stephenson @ 2017-02-16 16:36 UTC (permalink / raw)
To: zsh-workers
On Thu, 16 Feb 2017 08:25:24 -0800
Sebastian Gniazdowski <psprint2@fastmail.com> wrote:
> On Thu, Feb 16, 2017, at 08:03 AM, Sebastian Gniazdowski wrote:
> > On Thu, Feb 16, 2017, at 07:11 AM, Peter Stephenson wrote:
> > > % zmodload zsh/db/gdbm
> > > % ztie -d db/gdbm -f db.gdbm dbase
> > > % dbase[testkey]=somewhat
> > > ==25034== Invalid write of size 1
> > > ==25034== at 0x80E0B78: metafy (utils.c:4652)
> >
> > I've used HEAP_ALLOC instead of HEAP_DUP in argument for metafy. Fixing
> > this resolves valgrind report. Important catch.. Hopefully it will
> > resolve enable-zsh-mem problem (I cannot reproduce that). Sending as
> > complete patch plus db_gdbm.c file.
>
> Yeah this will fix the problem. I did:
>
> pm->u.str = metafy(content.dptr, content.dsize, META_ALLOC);
>
> There were no meta characters so metafy() didn't allocate memory,
> returned internal GDBM pointer. META_DUP always allocates. Thanks!
Yes, test now runs in the case where it originally failed with zsh
allocation.
I think that's probably all the sanity checking this is going to get in
these parts. Unless someone screams in appropriately technical language
(or, even better, wants to do some checking of their own) I'll commit
it.
pws
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] db/gdbm rewrite
2017-02-16 16:36 ` Peter Stephenson
@ 2017-02-16 17:12 ` Sebastian Gniazdowski
0 siblings, 0 replies; 15+ messages in thread
From: Sebastian Gniazdowski @ 2017-02-16 17:12 UTC (permalink / raw)
To: zsh-workers
On Thu, Feb 16, 2017, at 08:36 AM, Peter Stephenson wrote:
> I think that's probably all the sanity checking this is going to get in
> these parts. Unless someone screams in appropriately technical language
> (or, even better, wants to do some checking of their own) I'll commit
> it.
I think there's "patch bomb" aura but:
– I used existing ways of doing things that the module had, they were
good (now trying to apply them to redis database),
– had much experience with Param after zpopulator where I was copying
stdin to a hash – crawled through many tricks like if(delunset).
There might be problems but it's just that this wasn't weekend coding
and much of previous module is preserved
--
Sebastian Gniazdowski
psprint2@fastmail.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] db/gdbm rewrite
2017-02-16 16:03 ` Sebastian Gniazdowski
2017-02-16 16:25 ` Sebastian Gniazdowski
@ 2017-02-16 18:16 ` Sebastian Gniazdowski
1 sibling, 0 replies; 15+ messages in thread
From: Sebastian Gniazdowski @ 2017-02-16 18:16 UTC (permalink / raw)
To: zsh-workers
[-- Attachment #1: Type: text/plain, Size: 200 bytes --]
Hello,
missed one metafication (in hash scan function), added test for that,
and also for zgdbmpath builtin. Incremental to db_gdbm_rewrite4.diff.
--
Sebastian Gniazdowski
psprint2@fastmail.com
[-- Attachment #2: db_gdbm_rewrite4_plus.diff --]
[-- Type: text/plain, Size: 1271 bytes --]
diff --git a/Src/Modules/db_gdbm.c b/Src/Modules/db_gdbm.c
index 8a61b2f..596a8ae 100644
--- a/Src/Modules/db_gdbm.c
+++ b/Src/Modules/db_gdbm.c
@@ -442,7 +442,7 @@ scangdbmkeys(HashTable ht, ScanFunc func, int flags)
/* This returns database-interfacing Param,
* it will return u.str or first fetch data
* if not PM_UPTODATE (newly created) */
- char *zkey = ztrduppfx(key.dptr, key.dsize);
+ char *zkey = metafy(key.dptr, key.dsize, META_DUP);
HashNode hn = getgdbmnode(ht, zkey);
zsfree( zkey );
diff --git a/Test/V11db_gdbm.ztst b/Test/V11db_gdbm.ztst
index a1076dc..486ad48 100644
--- a/Test/V11db_gdbm.ztst
+++ b/Test/V11db_gdbm.ztst
@@ -280,6 +280,28 @@
>æ¼¢å—
>correct
+ ztie -d db/gdbm -f $dbfile dbase
+ dbase=( æ¼¢å— æ¼¢å— )
+ zuntie dbase
+ ztie -d db/gdbm -f $dbfile dbase
+ noglob print -rl ${(kv)dbase[@]}
+ zuntie dbase
+0:Hash scanning and metafication
+>æ¼¢å—
+>æ¼¢å—
+
+ ztie -d db/gdbm -f $dbfile dbase
+ zgdbmpath dbase
+ [[ $REPLY = */Test/db.gdbm ]] && echo correct
+ zuntie dbase
+ ztie -r -d db/gdbm -f $dbfile dbase
+ zgdbmpath dbase
+ [[ $REPLY = */Test/db.gdbm ]] && echo correct
+ zuntie -u dbase
+0:zgdbmpath builtin
+>correct
+>correct
+
%clean
rm -f ${dbfile}*
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] db/gdbm rewrite
2017-02-19 18:19 ` Bart Schaefer
@ 2017-02-20 8:32 ` Sebastian Gniazdowski
0 siblings, 0 replies; 15+ messages in thread
From: Sebastian Gniazdowski @ 2017-02-20 8:32 UTC (permalink / raw)
To: zsh-workers
On Sun, Feb 19, 2017, at 10:19 AM, Bart Schaefer wrote:
> GDBM_SYNC is meaningless for read-only mode; in any write mode it means
> that changes made to by the program are immediately flushed out to the
Ah, I had a matrix on my eyes and read GDBM_SYNC as GDBM_LOCK.
> Man page also says:
>
> It is important that every file opened is also closed. This is needed
> to update the reader/writer count on the file.
>
> Why would there be a reader/writer count if there is no concurrent change
> allowed?
It must be for multiple threads. Sorry for not reading the docs doing
update, but by testing gdbmtool behavior I have got correct image:
- database access is guarded by locks
- writing to single file by 2 threads without locking corrupts database
- gdbm isn't a server and doesn't order transactions with a level of
concurrency
We could update db_gdbm to use GDBM_NOLOCK and seamlessly ~PM_UPTODATE,
providing a zgdbmlock builtin. I did a good thing with PM_UPTODATE, all
doors are open.
--
Sebastian Gniazdowski
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] db/gdbm rewrite
2017-02-19 8:46 ` Sebastian Gniazdowski
@ 2017-02-19 18:19 ` Bart Schaefer
2017-02-20 8:32 ` Sebastian Gniazdowski
0 siblings, 1 reply; 15+ messages in thread
From: Bart Schaefer @ 2017-02-19 18:19 UTC (permalink / raw)
To: zsh-workers
On Feb 19, 12:46am, Sebastian Gniazdowski wrote:
}
} As the other thread pointed out, GDBM_SYNC flag means no change to
} database can be done, even when opening in read-only mode.
?? That's not what it means at all:
GDBM_READER reader
GDBM_WRITER writer
GDBM_WRCREAT writer - if database does not exist create new one
GDBM_NEWDB writer - create new database regardless if one exists
For the last three (writers of the database) the following may be
added added to read_write by bitwise or: GDBM_SYNC, which causes all
database operations to be synchronized to the disk, and GDBM_NOLOCK,
which prevents the library from performing any locking on the database
file.
GDBM_SYNC is meaningless for read-only mode; in any write mode it means
that changes made to by the program are immediately flushed out to the
file, so that other readers can immediately see the change. Locking is
a separate op. zsh/db/gdbm does use locking by default (does not pass
teh NOLOCK flag); I have never deeply investigated how gdbm handles that
underneath.
Man page also says:
It is important that every file opened is also closed. This is needed
to update the reader/writer count on the file.
Why would there be a reader/writer count if there is no concurrent change
allowed?
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-02-20 8:32 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-14 12:20 [PATCH] db/gdbm rewrite Sebastian Gniazdowski
2017-02-15 10:22 ` Sebastian Gniazdowski
2017-02-16 10:16 ` Peter Stephenson
2017-02-16 11:46 ` Sebastian Gniazdowski
2017-02-16 12:52 ` Peter Stephenson
2017-02-16 14:25 ` Sebastian Gniazdowski
2017-02-16 14:30 ` Sebastian Gniazdowski
2017-02-16 15:11 ` Peter Stephenson
2017-02-16 16:03 ` Sebastian Gniazdowski
2017-02-16 16:25 ` Sebastian Gniazdowski
2017-02-16 16:36 ` Peter Stephenson
2017-02-16 17:12 ` Sebastian Gniazdowski
2017-02-16 18:16 ` Sebastian Gniazdowski
2017-02-19 0:43 (Fwd) " Bart Schaefer
2017-02-19 8:46 ` Sebastian Gniazdowski
2017-02-19 18:19 ` Bart Schaefer
2017-02-20 8:32 ` Sebastian Gniazdowski
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).