From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 134 invoked from network); 13 Nov 1998 16:37:54 -0000 Received: from math.gatech.edu (list@130.207.146.50) by ns1.primenet.com.au with SMTP; 13 Nov 1998 16:37:54 -0000 Received: (from list@localhost) by math.gatech.edu (8.9.1/8.9.1) id LAA13757; Fri, 13 Nov 1998 11:33:18 -0500 (EST) Resent-Date: Fri, 13 Nov 1998 11:33:18 -0500 (EST) Message-Id: <9811131616.AA30250@ibmth.df.unipi.it> To: zsh-workers@math.gatech.edu (Zsh hackers list) Subject: Associative arrays and memory In-Reply-To: ""Bart Schaefer""'s message of "Wed, 11 Nov 1998 10:16:21 NFT." <981111101622.ZM32551@candle.brasslantern.com> Date: Fri, 13 Nov 1998 17:16:36 +0100 From: Peter Stephenson Resent-Message-ID: <"VYiMD1.0.uM3.Ez5Js"@math> Resent-From: zsh-workers@math.gatech.edu X-Mailing-List: archive/latest/4623 X-Loop: zsh-workers@math.gatech.edu Precedence: list Resent-Sender: zsh-workers-request@math.gatech.edu "Bart Schaefer" wrote: > What I'm worried about is that copyparam() uses PERMALLOC, but I think > the pointers that it returns are being placed in a heap-allocated array > in some cases. There's also a ztrdup() in scancopyparams(). MUSTUSEHEAP > wouldn't report either problem. This would come up e.g. when you have a > local with the same name as an existing hash, so you have to save and > restore the hash. It does seem the existing ztrdup()'s in what's now copyparam() are memory leaks. This will need fixing in any case, independently from assoc arrays. However, it looks like the only purpose of copyparam() is simply to save special parameters and only in the following case: foo=bar some-builtin to restore $foo (it's separate from the usual local restore mechanism). There are currently no special assoc arrays, of course, and it should probably be possible to prevent there being any --- the fact that the shell uses a parameter doesn't make it special, it's only necessary when the parameter has an otherwise inappropriate relationship with a shell variable, that's why a lot of parameters have recently been despecialised. So I'd recommend removing all references to assoc arrays from copyparam(), i.e. putting that part of the code back how it was, maybe with a DPUTS(PM_TYPE(pm->flags) == PM_HASHED, "attempt to copy assoc array") to be on the safe side. As an aside, the ordinary `local' mechanism never deletes or copies anything, it just stacks the old parameters. So there shouldn't be any big issues there. Combining that with my other suggestion of separating assoc array returns out of getaparam() would be a big help in warding off possible memory leaks. (Unless `shift hash', which is where getaparam() comes in, is really supposed to work? It does something at the moment, but what does it mean --- randomly remove element from hash?? Same goes for the call in `set -A': there's no point getting an array only to find you can't set it because it's the wrong type.) Taking this all into account, I don't believe proper use of assoc arrays is going to cause memory leaks, even at present. Ensuring improper use doesn't either is the hard part. -- Peter Stephenson Tel: +39 050 844536 WWW: http://www.ifh.de/~pws/ Dipartimento di Fisica, Via Buonarroti 2, 56100 Pisa, Italy