zsh-workers
 help / color / mirror / code / Atom feed
From: Peter Stephenson <pws@ibmth.df.unipi.it>
To: zsh-workers@math.gatech.edu (Zsh hackers list)
Subject: Associative arrays and memory
Date: Fri, 13 Nov 1998 17:16:36 +0100	[thread overview]
Message-ID: <9811131616.AA30250@ibmth.df.unipi.it> (raw)
In-Reply-To: ""Bart Schaefer""'s message of "Wed, 11 Nov 1998 10:16:21 NFT." <981111101622.ZM32551@candle.brasslantern.com>

"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 <pws@ibmth.df.unipi.it>       Tel: +39 050 844536
WWW:  http://www.ifh.de/~pws/
Dipartimento di Fisica, Via Buonarroti 2, 56100 Pisa, Italy


  reply	other threads:[~1998-11-13 16:37 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
1998-11-11  6:44 PATCH: 3.1.5 - sample associative array implementation Bart Schaefer
1998-11-11 13:58 ` Peter Stephenson
1998-11-11 14:43   ` Bruce Stephens
1998-11-11 20:00     ` Timothy Writer
1998-11-11 20:52       ` Bart Schaefer
1998-11-12  8:22         ` Timothy Writer
1998-11-12  9:23           ` Bart Schaefer
1998-11-13  0:04             ` Timothy Writer
1998-11-13  1:32               ` Bart Schaefer
1998-11-14  1:55                 ` Timothy Writer
1998-11-14  6:41                   ` Bart Schaefer
1998-11-15  8:42                     ` Timothy Writer
1998-11-15 20:03                       ` Bart Schaefer
1998-11-16 19:16                         ` Timothy Writer
1998-11-15 20:47                   ` PATCH: 3.1.5 + associative arrays under ksh emulation Bart Schaefer
1998-11-11 18:16   ` PATCH: 3.1.5 - sample associative array implementation Bart Schaefer
1998-11-13 16:16     ` Peter Stephenson [this message]
1998-11-13 17:57       ` Associative arrays and memory Bart Schaefer
1998-11-14 15:26         ` Peter Stephenson
1998-11-14 18:47           ` Bart Schaefer
1998-11-15 15:54             ` Peter Stephenson
1998-11-16  9:54 Sven Wischnowsky
1998-11-16 12:43 ` Bart Schaefer
1998-11-16 13:15 Sven Wischnowsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9811131616.AA30250@ibmth.df.unipi.it \
    --to=pws@ibmth.df.unipi.it \
    --cc=zsh-workers@math.gatech.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).