Re: Anyone want to help make zsh/db/gdbm work?

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

Replying to myself with my own conclusions ...

On Jan 30,  7:21pm, Bart Schaefer wrote:
}
} ztie should have a flag (probably -r) to open the database read-only.
} (I've already implemented this, don't go running off to do it.)

Patch below.

} However, this raises the question of whether the resulting parameter
} should also be readonly.

I decided that it should, and added an option "zuntie -u" to forcibly
unset a paramter created in this way.

This also resolves the question of whether writes should silently fail;
they noisily fail.

} However, this makes me wonder whether it's correct for zuntie to unset
} the parameter.  Maybe an explicit zuntie should just convert the param
} into an ordinary hash, closing the database file from under it?  (This
} I have not implemented yet.)

I looked at this, but decided that it is impractical because the ztie'd
parameter reads and writes everything directly from/to the file rather
than caching any of the values in an internal hashtable.

A few other tidbits:

(1) Assigning to a ztie'd hash e.g with DB=( key value ... ) breaks the
ztied property somehow, leaking the GDBM_FILE object.  There must be a
way to fix this, because it doesn't happen for specials like $aliases.

(2) You can get around (1) by using DB+=( key value ... ), but:

(3) There's no operation for bulk deletion of keys from a hash, so to
completely reset the database you need something like

  for k in ${(k)DB}; do unset "DB[$k]"; done

I suspect fixing (1) would also fix (3).

Patch follows, including fixing up the doc but no tests yet.  We don't
have a standard Test/ convention for module tests, other than Test/V*
which is a mixture of tests for module loading in general and for a few
specific modules.  If we were going to add tests for this, where should
they go?


diff --git a/Doc/Zsh/mod_db_gdbm.yo b/Doc/Zsh/mod_db_gdbm.yo
index 6065f86..9097429 100644
--- a/Doc/Zsh/mod_db_gdbm.yo
+++ b/Doc/Zsh/mod_db_gdbm.yo
@@ -11,17 +11,41 @@ The builtins in this module are:
 
 startitem()
 findex(ztie)
-cindex(tied array, creating)
-item(tt(ztie -d db/gdbm -f) var(filename) var(arrayname))(
+cindex(database tied array, creating)
+item(tt(ztie -d db/gdbm -f) var(filename) [ tt(-r) ] var(arrayname))(
 Open the GDBM database identified by var(filename) and, if successful,
-create the associative array var(arrayname) linked to the file.  Note
-that var(arrayname) must be unset at the time tt(ztie) is called, and
-is always created as a global parameter (as if with `tt(typeset -g)').
+create the associative array var(arrayname) linked to the file.  To create
+a local tied array, the parameter must first be declared, so commands
+similar to the following would be executed inside a function scope:
+
+example(local -A sampledb
+ztie -d db/gdbm -f sample.gdbm sampledb)
+
+The tt(-r) option opens the database file for reading only, creating a
+parameter with the readonly attribute.  Without this option, using
+`tt(ztie)' on a file for which the user does not have write permission is
+an error.  If writable, the database is opened synchronously so fields
+changed in var(arrayname) are immediately written to var(filename).
+
+Changes to the file modes var(filename) after it has been opened do not
+alter the state of var(arrayname), but `tt(typeset -r) var(arrayname)'
+works as expected.
 )
 findex(zuntie)
-cindex(tied array, destroying)
-item(tt(zuntie) var(arrayname) ...)(
+cindex(database tied array, destroying)
+item(tt(zuntie) [ tt(-u) ] var(arrayname) ...)(
 Close the GDBM database associated with each var(arrayname) and then
-unset the variable.
+unset the parameter.  The tt(-u) option forces an unset of parameters
+made readonly with `tt(ztie -r)'.
+
+This happens automatically if the parameter is explicitly unset or its
+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)'.
 )
 enditem()
+
+The fields of an associative array tied to GDBM are neither cached nor
+otherwise stored in memory, they are read from or written to the database
+on each reference.  Thus, for example, the values in a readonly array may
+be changed by a second writer of the same database file.
diff --git a/Src/Modules/db_gdbm.c b/Src/Modules/db_gdbm.c
index 9896bb5..a83e32a 100644
--- a/Src/Modules/db_gdbm.c
+++ b/Src/Modules/db_gdbm.c
@@ -48,8 +48,8 @@ static const struct gsu_hash gdbm_hash_gsu =
 { hashgetfn, hashsetfn, gdbmhashunsetfn };
 
 static struct builtin bintab[] = {
-    BUILTIN("ztie", 0, bin_ztie, 1, -1, 0, "d:f:", NULL),
-    BUILTIN("zuntie", 0, bin_zuntie, 1, -1, 0, NULL, NULL),
+    BUILTIN("ztie", 0, bin_ztie, 1, -1, 0, "d:f:r", NULL),
+    BUILTIN("zuntie", 0, bin_zuntie, 1, -1, 0, "u", NULL),
 };
 
 /**/
@@ -58,6 +58,7 @@ 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')) {
@@ -68,6 +69,12 @@ bin_ztie(char *nam, char **args, Options ops, UNUSED(int func))
         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.
@@ -79,9 +86,9 @@ bin_ztie(char *nam, char **args, Options ops, UNUSED(int func))
 
     resource_name = OPT_ARG(ops, 'f');
 
-    dbf = gdbm_open(resource_name, 0, GDBM_WRCREAT | GDBM_SYNC, 0666, 0);
+    dbf = gdbm_open(resource_name, 0, read_write, 0666, 0);
     if(!dbf) {
-        zwarnnam(nam, "error opening database file %s", resource_name);
+	zwarnnam(nam, "error opening database file %s", resource_name);
 	return 1;
     }
 
@@ -101,7 +108,7 @@ bin_ztie(char *nam, char **args, Options ops, UNUSED(int func))
 	}
     }
     if (!(tied_param = createspecialhash(pmname, &getgdbmnode, &scangdbmkeys,
-					 PM_REMOVABLE))) {
+					 pmflags))) {
         zwarnnam(nam, "cannot create the requested parameter %s", pmname);
 	gdbm_close(dbf);
 	return 1;
@@ -135,6 +142,8 @@ bin_zuntie(char *nam, char **args, Options ops, UNUSED(int func))
 	}
 
 	queue_signals();
+	if (OPT_ISSET(ops,'u'))
+	    gdbmuntie(pm);	/* clear read-only-ness */
 	if (unsetparam_pm(pm, 0, 1)) {
 	    /* assume already reported */
 	    ret = 1;
@@ -250,19 +259,30 @@ scangdbmkeys(HashTable ht, ScanFunc func, int flags)
 
 /**/
 static void
-gdbmhashunsetfn(Param pm, UNUSED(int exp))
+gdbmuntie(Param pm)
 {
     GDBM_FILE dbf = (GDBM_FILE)(pm->u.hash->tmpdata);
+    HashTable ht = pm->u.hash;
 
-    if (!dbf) /* paranoia */
-	return;
+    if (dbf) /* paranoia */
+	gdbm_close(dbf);
 
-    gdbm_close(dbf);
-    pm->u.hash->tmpdata = NULL;
+    ht->tmpdata = NULL;
 
-    /* hash table is now normal, so proceed normally... */
-    pm->node.flags &= ~PM_SPECIAL;
+    /* 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);
+    /* hash table is now normal, so proceed normally... */
     pm->gsu.h->setfn(pm, NULL);
     pm->node.flags |= PM_UNSET;
 }