From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24855 invoked by alias); 3 Feb 2015 10:22:59 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 34455 Received: (qmail 7299 invoked from network); 3 Feb 2015 10:22:46 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_HI, SPF_HELO_PASS autolearn=ham version=3.3.2 X-AuditID: cbfec7f5-b7fc86d0000066b7-15-54d0a0df2e0d Date: Tue, 03 Feb 2015 10:22:39 +0000 From: Peter Stephenson To: Zsh hackers list Subject: Re: Anyone want to help make zsh/db/gdbm work? Message-id: <20150203102239.7585c67a@pwslap01u.europe.root.pri> In-reply-to: <20150202094643.3ce41ebc@pwslap01u.europe.root.pri> References: <150122211942.ZM28918@torch.brasslantern.com> <20150129204658.4a800bcb@ntlworld.com> <150129140625.ZM14730@torch.brasslantern.com> <20150130085949.4d44007d@pwslap01u.europe.root.pri> <20150130195635.1845793e@ntlworld.com> <20150202094643.3ce41ebc@pwslap01u.europe.root.pri> Organization: Samsung Cambridge Solution Centre X-Mailer: Claws Mail 3.7.9 (GTK+ 2.22.0; i386-redhat-linux-gnu) MIME-version: 1.0 Content-type: text/plain; charset=US-ASCII Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrMLMWRmVeSWpSXmKPExsVy+t/xq7r3F1wIMTjRZWNxsPkhkwOjx6qD H5gCGKO4bFJSczLLUov07RK4Mho+3WYumCRaMe02VwPjYYEuRg4OCQETiYO3OboYOYFMMYkL 99azdTFycQgJLGWU6Lo3kxXCWcIksW72HyhnG6PEn5vvWEFaWARUJXZvWs4CYrMJGEpM3TSb EcQWEdCS2HHyJBPIBmEBc4k5Z/1AwrwC9hJXjzcyg9icAg4SR3obwMYICWxlljj4hBfE5hfQ l7j69xMTxEX2EjOvnGGE6BWU+DH5HtgqZqDxm7c1sULY8hKb17xlhpijLnHj7m72CYxCs5C0 zELSMgtJywJG5lWMoqmlyQXFSem5RnrFibnFpXnpesn5uZsYIQH7dQfj0mNWhxgFOBiVeHh3 fj0fIsSaWFZcmXuIUYKDWUmEd/u8CyFCvCmJlVWpRfnxRaU5qcWHGJk4OKUaGI/n+U+71vFN fWfd9aunY3ndToc5Hez32edlzH5u5iFX19oy2+UWTlEWf3klZigcmp5XycM5Zc9Mhwd8Yb8U dzFGXbiXz2Rf+/eh6eKAR3If08TfSf2LytXK+7M34cmrHRfdH3Mu/vK789iUYzyMD4QDZkSx umtcstE7b5xqe+lageqfxU5Sy5VYijMSDbWYi4oTAY8iW4Y2AgAA On Mon, 2 Feb 2015 09:46:43 +0000 Peter Stephenson wrote: > On Fri, 30 Jan 2015 12:03:04 -0800 > Bart Schaefer wrote: > > If you're actually going to look up the parameter first, you should also > > delay the gdbm_open() until you've verified the parameter. > > There's another point here that if unsetting the old parameter happens > to untie an existing database, possibly even the same one, it needs to > be done before the new open. This moves things around. I kept the code so I could test the return value, but as noted in the comments it would currently be good enough to use unsetparam() and test errflag. I can't see why pmname was duplicated; createparam() doesn't require this and doesn't ever free what you passed in. It was suspicious that the duplicated value of pmname was used in an error message just before a return: either the value was invalid, or it needed to be freed, and I think the latter. I've simply removed the duplication. diff --git a/Src/Modules/db_gdbm.c b/Src/Modules/db_gdbm.c index 2e2bd3a..76d4751 100644 --- a/Src/Modules/db_gdbm.c +++ b/Src/Modules/db_gdbm.c @@ -85,14 +85,7 @@ bin_ztie(char *nam, char **args, Options ops, UNUSED(int func)) } resource_name = OPT_ARG(ops, 'f'); - - dbf = gdbm_open(resource_name, 0, read_write, 0666, 0); - if(!dbf) { - zwarnnam(nam, "error opening database file %s", resource_name); - return 1; - } - - pmname = ztrdup(*args); + pmname = *args; if ((tied_param = (Param)paramtab->getnode(paramtab, pmname)) && !(tied_param->node.flags & PM_UNSET)) { @@ -100,13 +93,24 @@ 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. + * + * 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)) { - zsfree(pmname); - gdbm_close(dbf); + if (unsetparam_pm(tied_param, 0, 1)) return 1; - } } + + dbf = gdbm_open(resource_name, 0, read_write, 0666, 0); + if(!dbf) { + zwarnnam(nam, "error opening database file %s", resource_name); + return 1; + } + if (!(tied_param = createspecialhash(pmname, &getgdbmnode, &scangdbmkeys, pmflags))) { zwarnnam(nam, "cannot create the requested parameter %s", pmname);