Re: Associative arrays and memory

Re: Associative arrays and memory

[Sven Wischnowsky]

Peter Stephenson wrote:

> ...
>
> So, the question is are there any uses for special hashes which would
> require tying them directly to an internal variable or function, or
> can they always be accessed by the standard parameter functions?  I
> would think the whole point of using assoc arrays is to avoid any
> unpleasantness of the former kind.  Probably Sven can answer that
> better than anyone.

When thinking about using assoc. arrays for completion (and zle
widgets) I think of two uses: the one Bart mentioned in his reply
(`zle[...]') for access to various zle internals and one for the
mapping of command strings or environments to function( name)s which
produce the matches. The second one would be fully user-controlled (in 
the sense that the user might use it if (s)he wishes but the
completion code wouldn't enforce it). Hence this one would make no
trouble.
The first use might cause some trouble, you already described the
current use of LBUFFER and so on. For the completion stuff I'm not yet 
sure if we really need/should have settable parameters. If they are
settable it shouldn't be too hard to access their values via the
normal parameter interface (although making them special would
probably make things cleaner and easier to add the new things we may
one day find interesting).

So, I would like to reduce it to the question if we should use an
assoc array for the zle information or not. Using an assoc array only
for this may look a bit queer unless we use them in other places, too.
(Every time I think about this I can't help remembering the discussion 
about a new option system, am I the only one?)

Two more things about assoc arrays:

  typeset -A foo
  foo[hello]=world
  echo $foo[(i)w*]

gives `1', this should be `hello'.

And if we use an assoc array for completion command name patterns we
would need pattern matching the other way round: the keys are taken as 
patterns and `$funcs[(x)$cmdstr]' (for some value of `x') should give
the values of all entries whose key (taken as a pattern) match the
value of $cmdstr. But of course we could use a simple array for the
patterns and loop through it (the question is: are there other uses
where such a feature might be interesting to have, and: if we have a
way to get a list of matching entries, should we make this with a new
modifier flag that can be combined with `i', `I', `r', and `R' so that 
all of them give a list, not only the first matching one?).

Bye
 Sven


--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: Associative arrays and memory

[Bart Schaefer]

On Nov 16, 10:54am, Sven Wischnowsky wrote:
} Subject: Re: Associative arrays and memory
}
} (Every time I think about this I can't help remembering the discussion 
} about a new option system, am I the only one?)

You are not.  However, I thought the "new options system" mainly meant
new syntax for "namespaces"?  In which case it doesn't really help with
any of these questions about special parameters.  It could remove one
level of indirection in the save/restore loops, I guess.

} Two more things about assoc arrays:
} 
}   typeset -A foo
}   foo[hello]=world
}   echo $foo[(i)w*]
} 
} gives `1', this should be `hello'.

Yes, I wondered about this.  (Does ksh93 have any equivalent syntax?)
The problem at present is that $foo is the array of values, not of both
values and keys.  So for example:

	% bar=($foo)
	% echo $bar[$foo[(i)w*]]
	hello

That would fail if $foo[(i)w*] substituted "hello" instead of "1".
(Actually, there appears to be a bug in this code; the correct index is
not always substituted.  Patch hopefully to follow.)

Then there are these examples:

	% echo ${(k)foo[@]}
	hello
	% echo ${(k)foo[(i)h*]}
	1
	% echo $foo[(kv)*]
	hello world
	% echo $foo[(kvi)w*]
	2

} And if we use an assoc array for completion command name patterns we
} would need pattern matching the other way round: the keys are taken as 
} patterns and `$funcs[(x)$cmdstr]' (for some value of `x') should give
} the values of all entries whose key (taken as a pattern) match the
} value of $cmdstr.  But of course we could use a simple array for the
} patterns and loop through it

I was planning to add something using the hashtable pattern interface to
take the input key as a pattern and return the values whose keys match,
but I wasn't thinking of turning it inside out like that.

I think the way I'd do the above would be the (equivalent of the) loop:
	for pat in ${(k)funcs[@]}
	do
	    if [[ $cmdstr = $pat ]]
	    then
	    	completions=($completions $funcs[$pat])
	    fi
	done

The problem (with or without your magic [(x)...] syntax) is that an
associative array is unordered, but presumably we want some fixed order
to the interpretation of completions when multiple patterns match the
command.  (If we're using an associative array for completions, how do
you implement the equivalent of the -tc option?)

} (the question is: are there other uses
} where such a feature might be interesting to have

I think for a shell-script-level feature, this has gone over the edge of
reasonable complexity.  If perl doesn't have this feature, we should avoid
it too. :-}

} and: if we have a
} way to get a list of matching entries, should we make this with a new
} modifier flag that can be combined with `i', `I', `r', and `R' so that 
} all of them give a list, not only the first matching one?).

Maybe it's because it's 4:30am, but I don't understand that part at all.

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Ksh93 (was Re: Associative arrays and memory)

[Bruce Stephens]

"Bart Schaefer" <schaefer@brasslantern.com> writes:

> Yes, I wondered about this.  (Does ksh93 have any equivalent
> syntax?)

ksh93 can be downloaded (in binary) for a number of systems at
<URL:http://www.research.att.com>.  There's a license, but it's a
pretty harmless one.  Comes with a manpage.

There's other cool stuff there too.  

graphviz draws graphs.  There's an example showing how processes
connect together in pipelines, for example.  (The idea is that you
describe the graph in terms of directed edges, and the tool chooses
sensible positioning and things.)

ciao is a program understanding type tool.  You abstract properties of
source code---functions, datatypes, files, and the relationships
between them---and interrogate them using some tools from graphviz.
Imagine a source tool designed by somebody who'd never seen any other
source navigator, and whose mind had been warped by extended exposure
to Unix, and that's something like ciao.  Beautiful and subtle,
although the GUI is very 80's (Athena).  And the whole thing is stuck
together with ksh93 shellscript, which kind of makes it relevant.

PATCH: 3.1.5: assoc array memory mucking around tedium

[Peter Stephenson]

The following very dull patch is my current best guess to keep memory
management of AA's in order.  At this stage it's probably best for
other people (= Bart, presumably) to play with it, even if there's the
odd buglet remaining.  Needless to say, it goes on top of everything
else.  Tomorrow I might update my patched test version for hard core
enthusiasts.

I fixed the bug Bart already spotted, where the tpm copied to wasn't
actually used in save_params().

It keeps copyparam() etc., i.e. it does a deep copy of any special
AA's for restoration after a builtin or function.  I'm still not 100%
convinced of the need for this at the moment, since we haven't yet
agreed on what special AA's are going to do, but this is how to get it
to work: I've added a useless special AA called $testhash for the sole
purpose of making sure it does.  It will have to be deleted for
production code, of course.  It's not a full test, since a real
special AA will have its own get/set methods ($testhash uses the
standard ones), and that could be where the fun starts.

One potentially interesting thing I did do:  since it was necessary to
make
  testhash=(....) func
work to test this (for reasons discussed in my last posting), I added
the simplest possible form of whole hash assignment,
  testhash=(key1 value1 ...)
or
  set -A testhash key1 value1 ...
so the syntax is identical to ordinary arrays, and you need to
predeclare with typeset -A (not for $testhash, since that's special).
However, that's entirely symmetric with everything else about AA's at
the moment.  I don't know what ksh93 thinks about this, I would guess
it converts them mindlessly into ordinary arrays at that point.

By the way, that means the way to copy whole AAs on the command line
is currently
% typesest -A hash
% hash=(${(kv)testhash})
which is entirely logical but maybe not completely obvious.  (It
doesn't use copyparam(), either.)

I've moved some param stuff from hashtable.c to params.c, since I
needed to pass a flag to one of them: there was a memory leak in that
the values of the parameters stored in the AA didn't get unset.

There was a small bug in the paramvals alloc().

gethparam() is now separate from getaparam(): shifting an AA now fails
(silently, as it always did with scalars, which wouldn't be my choice
but I haven't altered it), but zle will still check the values of AA's
when looking through variables for completions, i.e. zle does the same
thing as before.

I changed the test whether to delete a parameter, instead of marking
it unset, to `if (locallevel && locallevel >= pm->level)': the first
part wasn't there before, so they never got deleted, even at top
level.  I hope that >= is still correct, but I think so.  (This is
independent of AA's.)

Warnings about three more hanging else's went over my boredom
threshold, so they were zapped.

Further motivation is mentioned in comments.  The main point is that
when copying an AA using copyparam(), the hash part has to be
assignable as a fully-paid-up, freeable paramtable, so various
additions to get it to work were necessary.  In particular, PM_SPECIAL
flags are removed from lower level `struct param's, since after
copying they aren't any more.  It's up to the sets.hfn() of the
special AA how it deals with that, but it makes sense since sets.hfn()
is in any case going to be passed a non-special hash table when it's
assigned to directly --- which is what it's for.  So if copyparam() is
wrong about this, so is the rest of the code.

I've also discovered I don't understand UNIX memory management.
Memory usage jumps around all over the place, even where I'm pretty
sure there's no leak.

*** Src/Zle/zle_tricky.c.bart2	Sat Nov 14 15:31:43 1998
--- Src/Zle/zle_tricky.c	Mon Nov 16 11:57:48 1998
***************
*** 4415,4421 ****
      } else {
  	/* Otherwise it should be a parameter name. */
  	char **arr = NULL, *val;
! 	if (!(arr = getaparam(nam)) && (val = getsparam(nam))) {
  	    arr = (char **)ncalloc(2*sizeof(char *));
  	    arr[0] = val;
  	    arr[1] = NULL;
--- 4415,4422 ----
      } else {
  	/* Otherwise it should be a parameter name. */
  	char **arr = NULL, *val;
! 	if (!(arr = getaparam(nam)) && !(arr = gethparam(nam)) &&
! 	    (val = getsparam(nam))) {
  	    arr = (char **)ncalloc(2*sizeof(char *));
  	    arr[0] = val;
  	    arr[1] = NULL;
*** Src/exec.c.bart2	Sat Nov 14 15:15:01 1998
--- Src/exec.c	Mon Nov 16 17:30:07 1998
***************
*** 1004,1014 ****
  untokenize(char *s)
  {
      for (; *s; s++)
! 	if (itok(*s))
  	    if (*s == Nularg)
  		chuck(s--);
  	    else
  		*s = ztokens[*s - Pound];
  }
  
  /* Open a file for writing redicection */
--- 1004,1015 ----
  untokenize(char *s)
  {
      for (; *s; s++)
! 	if (itok(*s)) {
  	    if (*s == Nularg)
  		chuck(s--);
  	    else
  		*s = ztokens[*s - Pound];
+ 	}
  }
  
  /* Open a file for writing redicection */
***************
*** 1924,1930 ****
  		       (unset(RESTRICTED) || !(pm->flags & PM_RESTRICTED))) {
  		Param tpm = (Param) alloc(sizeof *tpm);
  		tpm->nam = s;
! 		copyparam(tpm, pm);
  	    }
  	    addlinknode(*remove_p, s);
  	    addlinknode(*restore_p, pm);
--- 1925,1932 ----
  		       (unset(RESTRICTED) || !(pm->flags & PM_RESTRICTED))) {
  		Param tpm = (Param) alloc(sizeof *tpm);
  		tpm->nam = s;
! 		copyparam(tpm, pm, 1);
! 		pm = tpm;
  	    }
  	    addlinknode(*remove_p, s);
  	    addlinknode(*restore_p, pm);
*** Src/hashtable.c.bart2	Wed Nov 11 09:38:30 1998
--- Src/hashtable.c	Mon Nov 16 15:47:31 1998
***************
*** 1061,1165 ****
      putchar('\n');
  }
  
- /**********************************/
- /* Parameter Hash Table Functions */
- /**********************************/
- 
- /**/
- void
- freeparamnode(HashNode hn)
- {
-     Param pm = (Param) hn;
-  
-     zsfree(pm->nam);
-     zfree(pm, sizeof(struct param));
- }
- 
- /* Print a parameter */
- 
- /**/
- void
- printparamnode(HashNode hn, int printflags)
- {
-     Param p = (Param) hn;
-     char *t, **u;
- 
-     if (p->flags & PM_UNSET)
- 	return;
- 
-     /* Print the attributes of the parameter */
-     if (printflags & PRINT_TYPE) {
- 	if (p->flags & PM_INTEGER)
- 	    printf("integer ");
- 	else if (p->flags & PM_ARRAY)
- 	    printf("array ");
- 	else if (p->flags & PM_HASHED)
- 	    printf("association ");
- 	if (p->flags & PM_LEFT)
- 	    printf("left justified %d ", p->ct);
- 	if (p->flags & PM_RIGHT_B)
- 	    printf("right justified %d ", p->ct);
- 	if (p->flags & PM_RIGHT_Z)
- 	    printf("zero filled %d ", p->ct);
- 	if (p->flags & PM_LOWER)
- 	    printf("lowercase ");
- 	if (p->flags & PM_UPPER)
- 	    printf("uppercase ");
- 	if (p->flags & PM_READONLY)
- 	    printf("readonly ");
- 	if (p->flags & PM_TAGGED)
- 	    printf("tagged ");
- 	if (p->flags & PM_EXPORTED)
- 	    printf("exported ");
-     }
- 
-     if (printflags & PRINT_NAMEONLY) {
- 	zputs(p->nam, stdout);
- 	putchar('\n');
- 	return;
-     }
- 
-     /* How the value is displayed depends *
-      * on the type of the parameter       */
-     quotedzputs(p->nam, stdout);
-     putchar('=');
-     switch (PM_TYPE(p->flags)) {
-     case PM_SCALAR:
- 	/* string: simple output */
- 	if (p->gets.cfn && (t = p->gets.cfn(p)))
- 	    quotedzputs(t, stdout);
- 	putchar('\n');
- 	break;
-     case PM_INTEGER:
- 	/* integer */
- 	printf("%ld\n", p->gets.ifn(p));
- 	break;
-     case PM_ARRAY:
- 	/* array */
- 	putchar('(');
- 	u = p->gets.afn(p);
- 	if(*u) {
- 	    quotedzputs(*u++, stdout);
- 	    while (*u) {
- 		putchar(' ');
- 		quotedzputs(*u++, stdout);
- 	    }
- 	}
- 	printf(")\n");
- 	break;
-     case PM_HASHED:
- 	/* association */
- 	putchar('(');
- 	{
-             HashTable ht = p->gets.hfn(p);
-             if (ht)
- 		scanhashtable(ht, 0, 0, 0, ht->printnode, 0);
- 	}
- 	printf(")\n");
- 	break;
-     }
- }
- 
  /****************************************/
  /* Named Directory Hash Table Functions */
  /****************************************/
--- 1061,1066 ----
*** Src/params.c.bart2	Sat Nov 14 15:15:14 1998
--- Src/params.c	Mon Nov 16 17:34:17 1998
***************
*** 232,237 ****
--- 232,242 ----
  IPDEF9("psvar", &psvar, "PSVAR"),
  IPDEF9("watch", &watch, "WATCH"),
  
+ /*TEST BEGIN*/
+ #define IPDEF10(A) {NULL,A,PM_HASHED|PM_SPECIAL|PM_DONTIMPORT,BR((void *)0),SFN(hashsetfn),GFN(hashgetfn),stdunsetfn,0,NULL,NULL,NULL,0}
+ IPDEF10("testhash"),
+ /*TEST END*/
+ 
  #ifdef DYNAMIC
  IPDEF9F("module_path", &module_path, "MODULE_PATH", PM_RESTRICTED),
  #endif
***************
*** 277,286 ****
  static void
  scancopyparams(HashNode hn, int flags)
  {
      Param pm = (Param)hn;
!     Param tpm = (Param) alloc(sizeof *tpm);
      tpm->nam = ztrdup(pm->nam);
!     copyparam(tpm, pm);
      addhashnode(outtable, tpm->nam, tpm);
  }
  
--- 282,292 ----
  static void
  scancopyparams(HashNode hn, int flags)
  {
+     /* Going into a real parameter, so always use permanent storage */
      Param pm = (Param)hn;
!     Param tpm = (Param) zcalloc(sizeof *tpm);
      tpm->nam = ztrdup(pm->nam);
!     copyparam(tpm, pm, 0);
      addhashnode(outtable, tpm->nam, tpm);
  }
  
***************
*** 342,348 ****
      numparamvals = 0;
      if (ht)
  	scanhashtable(ht, 0, 0, 0, scancountparams, flags);
!     paramvals = (char **) alloc(numparamvals * sizeof(char **) + 1);
      if (ht) {
  	numparamvals = 0;
  	scanhashtable(ht, 0, 0, 0, scanparamvals, flags);
--- 348,354 ----
      numparamvals = 0;
      if (ht)
  	scanhashtable(ht, 0, 0, 0, scancountparams, flags);
!     paramvals = (char **) alloc((numparamvals + 1) * sizeof(char *));
      if (ht) {
  	numparamvals = 0;
  	scanhashtable(ht, 0, 0, 0, scanparamvals, flags);
***************
*** 485,490 ****
--- 491,526 ----
      noerrs = 0;
  }
  
+ /* assign various functions used for non-special parameters */
+ 
+ /**/
+ static void
+ assigngetset(Param pm)
+ {
+     switch (PM_TYPE(pm->flags)) {
+     case PM_SCALAR:
+ 	pm->sets.cfn = strsetfn;
+ 	pm->gets.cfn = strgetfn;
+ 	break;
+     case PM_INTEGER:
+ 	pm->sets.ifn = intsetfn;
+ 	pm->gets.ifn = intgetfn;
+ 	break;
+     case PM_ARRAY:
+ 	pm->sets.afn = arrsetfn;
+ 	pm->gets.afn = arrgetfn;
+ 	break;
+     case PM_HASHED:
+ 	pm->sets.hfn = hashsetfn;
+ 	pm->gets.hfn = hashgetfn;
+ 	break;
+     default:
+ 	DPUTS(1, "BUG: tried to create param node without valid flag");
+ 	break;
+     }
+     pm->unsetfn = stdunsetfn;
+ }
+ 
  /* Create a parameter, so that it can be assigned to.  Returns NULL if the *
   * parameter already exists or can't be created, otherwise returns the     *
   * parameter node.  If a parameter of the same name exists in an outer     *
***************
*** 530,559 ****
  	pm = (Param) alloc(sizeof *pm);
      pm->flags = flags;
  
!     if(!(pm->flags & PM_SPECIAL)) {
! 	switch (PM_TYPE(flags)) {
! 	case PM_SCALAR:
! 	    pm->sets.cfn = strsetfn;
! 	    pm->gets.cfn = strgetfn;
! 	    break;
! 	case PM_INTEGER:
! 	    pm->sets.ifn = intsetfn;
! 	    pm->gets.ifn = intgetfn;
! 	    break;
! 	case PM_ARRAY:
! 	    pm->sets.afn = arrsetfn;
! 	    pm->gets.afn = arrgetfn;
! 	    break;
! 	case PM_HASHED:
! 	    pm->sets.hfn = hashsetfn;
! 	    pm->gets.hfn = hashgetfn;
! 	    break;
! 	default:
! 	    DPUTS(1, "BUG: tried to create param node without valid flag");
! 	    break;
! 	}
! 	pm->unsetfn = stdunsetfn;
!     }
      return pm;
  }
  
--- 566,573 ----
  	pm = (Param) alloc(sizeof *pm);
      pm->flags = flags;
  
!     if(!(pm->flags & PM_SPECIAL))
! 	assigngetset(pm);
      return pm;
  }
  
***************
*** 561,570 ****
  
  /**/
  void
! copyparam(Param tpm, Param pm)
  {
      PERMALLOC {
  	tpm->flags = pm->flags;
  	switch (PM_TYPE(pm->flags)) {
  	case PM_SCALAR:
  	    tpm->u.str = ztrdup(pm->gets.cfn(pm));
--- 575,592 ----
  
  /**/
  void
! copyparam(Param tpm, Param pm, int toplevel)
  {
+     /*
+      * Note that tpm, into which we're copying, may not be in permanent
+      * storage.  However, the values themselves are later used directly
+      * to set the parameter, so must be permanently allocated (in accordance
+      * with sets.?fn() usage).
+      */
      PERMALLOC {
  	tpm->flags = pm->flags;
+ 	if (!toplevel)
+ 	    tpm->flags &= ~PM_SPECIAL;
  	switch (PM_TYPE(pm->flags)) {
  	case PM_SCALAR:
  	    tpm->u.str = ztrdup(pm->gets.cfn(pm));
***************
*** 580,585 ****
--- 602,616 ----
  	    break;
  	}
      } LASTALLOC;
+     /*
+      * If called from inside an associative array, that array is later going
+      * to be passed as a real parameter, so we need the gets and sets
+      * functions to be useful.  However, the saved associated array is
+      * not itself special, so we just use the standard ones.
+      * This is also why we switch off PM_SPECIAL.
+      */
+     if (!toplevel)
+ 	assigngetset(tpm);
  }
  
  /* Return 1 if the string s is a valid identifier, else return 0. */
***************
*** 962,972 ****
      s = t = *pptr;
      garr = NULL;
  
!     if (idigit(*s))
  	if (bracks >= 0)
  	    ppar = zstrtol(s, &s, 10);
  	else
  	    ppar = *s++ - '0';
      else if (iident(*s))
  	while (iident(*s))
  	    s++;
--- 993,1004 ----
      s = t = *pptr;
      garr = NULL;
  
!     if (idigit(*s)) {
  	if (bracks >= 0)
  	    ppar = zstrtol(s, &s, 10);
  	else
  	    ppar = *s++ - '0';
+     }
      else if (iident(*s))
  	while (iident(*s))
  	    s++;
***************
*** 1077,1084 ****
  	    break;
  	}
  
! 	if (v->a == 0 && v->b == -1)
  	    LASTALLOC_RETURN s;
  	if (v->a < 0)
  	    v->a += strlen(s);
  	if (v->b < 0)
--- 1109,1117 ----
  	    break;
  	}
  
! 	if (v->a == 0 && v->b == -1) {
  	    LASTALLOC_RETURN s;
+ 	}
  	if (v->a < 0)
  	    v->a += strlen(s);
  	if (v->b < 0)
***************
*** 1268,1284 ****
  	freearray(val);
  	return;
      }
!     if (PM_TYPE(v->pm->flags) != PM_ARRAY) {
  	freearray(val);
  	zerr("attempt to assign array value to non-array", NULL, 0);
  	return;
      }
!     if (v->a == 0 && v->b == -1)
! 	(v->pm->sets.afn) (v->pm, val);
!     else {
  	char **old, **new, **p, **q, **r;
  	int n, ll, i;
  
  	if (v->inv && unset(KSHARRAYS))
  	    v->a--, v->b--;
  	q = old = v->pm->gets.afn(v->pm);
--- 1301,1325 ----
  	freearray(val);
  	return;
      }
!     if (PM_TYPE(v->pm->flags) & ~(PM_ARRAY|PM_HASHED)) {
  	freearray(val);
  	zerr("attempt to assign array value to non-array", NULL, 0);
  	return;
      }
!     if (v->a == 0 && v->b == -1) {
! 	if (PM_TYPE(v->pm->flags) == PM_HASHED)
! 	    arrhashsetfn(v->pm, val);
! 	else
! 	    (v->pm->sets.afn) (v->pm, val);
!     } else {
  	char **old, **new, **p, **q, **r;
  	int n, ll, i;
  
+ 	if ((PM_TYPE(v->pm->flags) == PM_HASHED)) {
+ 	    freearray(val);
+ 	    zerr("attempt to set slice of associative array", NULL, 0);
+ 	    return;
+ 	}
  	if (v->inv && unset(KSHARRAYS))
  	    v->a--, v->b--;
  	q = old = v->pm->gets.afn(v->pm);
***************
*** 1346,1353 ****
  {
      Value v;
  
!     if (!idigit(*s) && (v = getvalue(&s, 0)))
! 	return getvaluearr(v);
      return NULL;
  }
  
--- 1387,1409 ----
  {
      Value v;
  
!     if (!idigit(*s) && (v = getvalue(&s, 0)) &&
! 	PM_TYPE(v->pm->flags) == PM_ARRAY)
! 	return v->pm->gets.afn(v->pm);
!     return NULL;
! }
! 
! /* Retrieve an assoc array parameter as an array */
! 
! /**/
! char **
! gethparam(char *s)
! {
!     Value v;
! 
!     if (!idigit(*s) && (v = getvalue(&s, 0)) &&
! 	PM_TYPE(v->pm->flags) == PM_HASHED)
! 	return paramvalarr(v->pm->gets.hfn(v->pm), SCANPM_WANTVALS);
      return NULL;
  }
  
***************
*** 1412,1418 ****
      } else {
  	if (!(v = getvalue(&s, 1)))
  	    createparam(t, PM_ARRAY);
! 	else if (PM_TYPE(v->pm->flags) != PM_ARRAY &&
  		 !(v->pm->flags & PM_SPECIAL)) {
  	    int uniq = v->pm->flags & PM_UNIQUE;
  	    unsetparam(t);
--- 1468,1474 ----
      } else {
  	if (!(v = getvalue(&s, 1)))
  	    createparam(t, PM_ARRAY);
! 	else if (!(PM_TYPE(v->pm->flags) & (PM_ARRAY|PM_HASHED)) &&
  		 !(v->pm->flags & PM_SPECIAL)) {
  	    int uniq = v->pm->flags & PM_UNIQUE;
  	    unsetparam(t);
***************
*** 1502,1508 ****
       * is called.  Beyond that, there is an ambiguity:  should   *
       * foo() { local bar; unset bar; } make the global bar       *
       * available or not?  The following makes the answer "no".   */
!     if (locallevel >= pm->level)
  	return;
  
      paramtab->removenode(paramtab, pm->nam); /* remove parameter node from table */
--- 1558,1564 ----
       * is called.  Beyond that, there is an ambiguity:  should   *
       * foo() { local bar; unset bar; } make the global bar       *
       * available or not?  The following makes the answer "no".   */
!     if (locallevel && locallevel >= pm->level)
  	return;
  
      paramtab->removenode(paramtab, pm->nam); /* remove parameter node from table */
***************
*** 1603,1619 ****
      return pm->u.hash;
  }
  
  /* Function to set value of an association parameter */
  
  /**/
  static void
  hashsetfn(Param pm, HashTable x)
  {
!     if (pm->u.hash && pm->u.hash != x)
  	deletehashtable(pm->u.hash);
      pm->u.hash = x;
  }
  
  /* This function is used as the set function for      *
   * special parameters that cannot be set by the user. */
  
--- 1659,1719 ----
      return pm->u.hash;
  }
  
+ /* Flag to freeparamnode to unset the struct */
+ 
+ static int delunset;
+ 
  /* Function to set value of an association parameter */
  
  /**/
  static void
  hashsetfn(Param pm, HashTable x)
  {
!     if (pm->u.hash && pm->u.hash != x) {
! 	/* The parameters in the hash table need to be unset *
! 	 * before being deleted.                             */
! 	int odelunset = delunset;
! 	delunset = 1;
  	deletehashtable(pm->u.hash);
+ 	delunset = odelunset;
+     }
      pm->u.hash = x;
  }
  
+ /* Function to set value of an association parameter using key/value pairs */
+ 
+ /**/
+ static void
+ arrhashsetfn(Param pm, char **val)
+ {
+     /* Best not to shortcut this by using the existing hash table,   *
+      * since that could cause trouble for special hashes.  This way, *
+      * it's up to pm->sets.hfn() what to do.                         */
+     int alen = arrlen(val);
+     HashTable opmtab = paramtab, ht;
+     char **aptr = val;
+     Value v = (Value) hcalloc(sizeof *v);
+     v->b = -1;
+ 
+     if (alen % 2) {
+ 	freearray(val);
+ 	zerr("bad set of key/value pairs for associative array\n",
+ 	     NULL, 0);
+ 	return;
+     }
+     ht = paramtab = newparamtable(17, pm->nam);
+     while (*aptr) {
+ 	/* The parameter name is ztrdup'd... */
+ 	v->pm = createparam(*aptr, PM_SCALAR|PM_UNSET);
+ 	zsfree(*aptr++);
+ 	/* ...but we can use the value without copying. */
+ 	setstrvalue(v, *aptr++);
+     }
+     paramtab = opmtab;
+     pm->sets.hfn(pm, ht);
+     free(val);		/* not freearray() */
+ }
+ 
  /* This function is used as the set function for      *
   * special parameters that cannot be set by the user. */
  
***************
*** 2373,2376 ****
--- 2473,2580 ----
      Param pm = (Param)hn;
      if(pm->level > locallevel)
  	unsetparam_pm(pm, 0, 0);
+ }
+ 
+ 
+ /**********************************/
+ /* Parameter Hash Table Functions */
+ /**********************************/
+ 
+ /**/
+ void
+ freeparamnode(HashNode hn)
+ {
+     Param pm = (Param) hn;
+  
+     /* Since the second flag to unsetfn isn't used, I don't *
+      * know what its value should be.                       */
+     if (delunset)
+ 	pm->unsetfn(pm, 1);
+     zsfree(pm->nam);
+     zfree(pm, sizeof(struct param));
+ }
+ 
+ /* Print a parameter */
+ 
+ /**/
+ void
+ printparamnode(HashNode hn, int printflags)
+ {
+     Param p = (Param) hn;
+     char *t, **u;
+ 
+     if (p->flags & PM_UNSET)
+ 	return;
+ 
+     /* Print the attributes of the parameter */
+     if (printflags & PRINT_TYPE) {
+ 	if (p->flags & PM_INTEGER)
+ 	    printf("integer ");
+ 	else if (p->flags & PM_ARRAY)
+ 	    printf("array ");
+ 	else if (p->flags & PM_HASHED)
+ 	    printf("association ");
+ 	if (p->flags & PM_LEFT)
+ 	    printf("left justified %d ", p->ct);
+ 	if (p->flags & PM_RIGHT_B)
+ 	    printf("right justified %d ", p->ct);
+ 	if (p->flags & PM_RIGHT_Z)
+ 	    printf("zero filled %d ", p->ct);
+ 	if (p->flags & PM_LOWER)
+ 	    printf("lowercase ");
+ 	if (p->flags & PM_UPPER)
+ 	    printf("uppercase ");
+ 	if (p->flags & PM_READONLY)
+ 	    printf("readonly ");
+ 	if (p->flags & PM_TAGGED)
+ 	    printf("tagged ");
+ 	if (p->flags & PM_EXPORTED)
+ 	    printf("exported ");
+     }
+ 
+     if (printflags & PRINT_NAMEONLY) {
+ 	zputs(p->nam, stdout);
+ 	putchar('\n');
+ 	return;
+     }
+ 
+     /* How the value is displayed depends *
+      * on the type of the parameter       */
+     quotedzputs(p->nam, stdout);
+     putchar('=');
+     switch (PM_TYPE(p->flags)) {
+     case PM_SCALAR:
+ 	/* string: simple output */
+ 	if (p->gets.cfn && (t = p->gets.cfn(p)))
+ 	    quotedzputs(t, stdout);
+ 	putchar('\n');
+ 	break;
+     case PM_INTEGER:
+ 	/* integer */
+ 	printf("%ld\n", p->gets.ifn(p));
+ 	break;
+     case PM_ARRAY:
+ 	/* array */
+ 	putchar('(');
+ 	u = p->gets.afn(p);
+ 	if(*u) {
+ 	    quotedzputs(*u++, stdout);
+ 	    while (*u) {
+ 		putchar(' ');
+ 		quotedzputs(*u++, stdout);
+ 	    }
+ 	}
+ 	printf(")\n");
+ 	break;
+     case PM_HASHED:
+ 	/* association */
+ 	putchar('(');
+ 	{
+             HashTable ht = p->gets.hfn(p);
+             if (ht)
+ 		scanhashtable(ht, 0, 0, 0, ht->printnode, 0);
+ 	}
+ 	printf(")\n");
+ 	break;
+     }
  }

-- 
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

Re: assoc array memory mucking, and semantics of patterned keys

[Bart Schaefer]

On Nov 16,  6:16pm, Peter Stephenson wrote:
} Subject: PATCH: 3.1.5: assoc array memory mucking around tedium
}
} The following very dull patch is my current best guess to keep memory
} management of AA's in order.  At this stage it's probably best for
} other people (= Bart, presumably) to play with it, even if there's the
} odd buglet remaining.

Looks good.  I'm particularly pleased that you arranged for whole-array
assignments to work, even if it does need ${(kv)assoc} on the rhs.

Speaking of that:

On Nov 16,  4:43am, Bart Schaefer wrote:
} Subject: Re: Associative arrays and memory
}
} 	% bar=($foo)
} 	% echo $bar[$foo[(i)w*]]
} 	hello

That should read

	% bar=($foo)
	% echo $bar[$foo[(i)w*]]
	world

Or

	% bar=(${(k)foo})
	% echo $bar[$foo[(i)w*]]
	hello

} That would fail if $foo[(i)w*] substituted "hello" instead of "1".
} (Actually, there appears to be a bug in this code; the correct index is
} not always substituted.  Patch hopefully to follow.)

The scoop on that bug:  There's no problem when returning the index of a
value, except as Sven noted that it returns a number rather than a key.
The bug is with

	% echo ${(k)foo[(i)h*]}

which you'd expect to return the key index of "hello", which should be the
same as the value index of "world".  Unfortunately, [(i)h*] is interpreted
by getvalue() before paramsubst() has adjusted for the (k) flag, so even
when asking for keys what is returned is the index of the value.

So, in this example:

} 	% echo ${(k)foo[@]}
} 	hello
} 	% echo ${(k)foo[(i)h*]}
} 	1

The current implementation returns 2 (past the end of the array), not 1,
and in this example:

} 	% echo ${(kv)foo[*]}
} 	hello world
} 	% echo ${(kv)foo[(i)w*]}
} 	2

it returns 1 rather than 2 because it hasn't counted the keys yet when
the index is computed.

There are a number of different ways to fix this, but I'd like to get
agreement on what the semantics should be to help choose the best one.
None of the following suggestions resolves the meaning of (k) used with
ordinary array slices, nor a syntax for using a key pattern to "slice" AAs.

The semantics of ordinary arrays are such that $array[(x)pat] for x
in [rRiI] always matches pat against the values.  So one reasonable
semantics is to decree that the same holds for AAs.  That gives the
following interpretations:

                        Associative Array        Ordinary Array
                        -----------------        --------------
  $param[key]           Value in param at key    Value in param at key
                        or empty if none         or empty if none

  ${(k)param[key]}      If key has a value,      If key has a value,
                        then key, else empty     then key, else empty

  $param[(r)pat]        Value in param that      Value in param that
  $param[(R)pat]        matches pattern pat      matches pattern pat

  $param[(i)pat]        Index in $param[@] of    Index in $param[@] of
  $param[(I)pat]        value that matches pat   value that matches pat
                        (_not_ key in $param)

  ${(k)param[(r)pat]}   Key of a value that      None (or, alternately,
  ${(k)param[(R)pat]}   that matches pat (not    same as $param[(i)pat]
                        a key that matches)      and $param[(I)pat])

  ${(k)param[(i)pat]}   As ${(k}param[(r)pat]}   As ${(k)param[(r)pat]}
  ${(k)param[(I)pat]}   and ${(k}param[(R)pat]}  and ${(k)param[(R)pat]}

  ${(kv)param[(r)pat]}  Key and value pair of    None (or, alternately,
  ${(kv)param[(R)pat]}  value that matches pat   same as $param[(r)pat]
                                                 and $param[(R)pat])

  ${(kv)param[(i)pat]}  Key and value pair of    None (or, alternately,
  ${(kv)param[(I)pat]}  value that matches pat   same as $param[(i)pat]
                                                 and $param[(I)pat])

This is nicely symmetrical, and adds a useful meaning for the (k) flag
when given a non-pattern key with either array type.  The potential
confusion is that (r) and (i) become equivalent when (k) is present, even
if (v) is also present, but that's pretty minimal.

Another possibility is similar to the above, except $param[(i)pat] on
an AA would return a key, not just an index, and thus be equivalent to
${(k)param[(r)pat]}.  I think this is less flexible, but some might find
it more intuitive.

The third possibility is that (k) would specify that keys were to be
searched rather than values.  The last four rows of the table then read:

  ${(k)param[(r)pat]}   If there is a key that   None (or, alternately,
  ${(k)param[(R)pat]}   matches pat, then the    same as $param[(r)pat]
                        value at that key, else  and $param[(R)pat])
			empty

  ${(k)param[(i)pat]}   If there is a key that   Same as $param[(i)pat]
  ${(k)param[(I)pat]}   matches pat, then that   and $param[(I)pat]
                        key, else empty

  ${(kv)param[(r)pat]}  Key and value pair of    Same as $param[(r)pat]
  ${(kv)param[(R)pat]}  key that matches pat     and $param[(R)pat]

  ${(kv)param[(i)pat]}  Key and value pair of    Same as $param[(i)pat]
  ${(kv)param[(I)pat]}  key that matches pat     and $param[(I)pat]

Again, this could be combined with having $param[(i)pat] return a key
rather than an index.  Note that there's no way to return a key/value pair
for values that match the pattern.

The final possibility is to change the meanings of (r) and (i) when an
AA is involved, so that (r) means search the values and (i) means search
the keys.  That changes these rows (4th, 6th, and 8th) in the original
table:

  $param[(i)pat]        A key that matches pat   Index of a value that
  $param[(I)pat]        or empty if none         matches pat, or empty

  ${(k)param[(i)pat]}   As $param[(i)pat]        As $param[(i)pat]
  ${(k)param[(I)pat]}   and $param[(I)pat]       and $param[(I)pat]

  ${(kv)param[(i)pat]}  Key and value pair of    Any of several; ignore
  ${(kv)param[(I)pat]}  key that matches pat     either (k) or (v) to
                                                 generate alternatives

And adds this row:

  ${(v)param[(i)pat]}   Value at a key that      As $param[(r)pat]
  ${(v)param[(I)pat]}   matches pat              and $param[(R)pat]

That last one makes this quite interesting, as it supplies an efficient
way to accomplish something that can't be expressed at all in the first
semantics I proposed, and that in the third semantics would be written as
$param[${(k)param[(i)pat]}].  However, I worry that the difference in the
meaning of (i) on associative arrays would be confusing.  Also, that last
is the only case where (v) is required to get a value (because (i) ends up
implying (k) otherwise).

On a pragmatic note, the first and last possible semantics are the easiest
to implement.  The middle three semantics require passing more data either
into or back from of getvalue() than is currently being passed.  I also
like the consistency of the first, but the expressiveness of the last.

Anybody else have an opinion?

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Re: assoc array memory mucking, and semantics of patterned keys

[Peter Stephenson]

"Bart Schaefer" wrote:
> The final possibility is to change the meanings of (r) and (i) when an
> AA is involved, so that (r) means search the values and (i) means search
> the keys.

Without thinking too hard, because I want to get some work done today,
I think I prefer this, because the (k) and (v) flags always refer to
what's returned, while the (r) and (i) flags always refer to what's
being searched.  It's both neat and powerful.  It's maybe annoying
it's different from normal arrays, but I think understandably so.
Once you've got the point that (i) tells you to search the index, not
return it, it's entirely logical.  Otherwise the meanings of (k) and
(i) are mixed in a slightly messy way.  It just needs one sentence in
the manual saying
   
   i [what happens with ordinary arrays]
  
     With associative arrays, specifies that the keys of the array
     should be searched for a match.  The part of the array returned
     is controlled by the (k) and (v) substitution flags in this case.

and pretty much everything else is covered already.

-- 
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

Re: Associative arrays and memory

[Sven Wischnowsky]

Bart Schaefer wrote:

> ...
> 
> On Nov 16, 10:54am, Sven Wischnowsky wrote:
> } Subject: Re: Associative arrays and memory
> }
> } (Every time I think about this I can't help remembering the discussion 
> } about a new option system, am I the only one?)
> 
> You are not.  However, I thought the "new options system" mainly meant
> new syntax for "namespaces"?  In which case it doesn't really help with
> any of these questions about special parameters.  It could remove one
> level of indirection in the save/restore loops, I guess.
> 

Yep. This is just another place where I could imagine assoc arrays to
be used.

> ...
>
> The problem (with or without your magic [(x)...] syntax) is that an
> associative array is unordered, but presumably we want some fixed order
> to the interpretation of completions when multiple patterns match the
> command.  (If we're using an associative array for completions, how do
> you implement the equivalent of the -tc option?)

Right, I forgot about that.

> } (the question is: are there other uses
> } where such a feature might be interesting to have
> 
> I think for a shell-script-level feature, this has gone over the edge of
> reasonable complexity.  If perl doesn't have this feature, we should avoid
> it too. :-}
> 
> } and: if we have a
> } way to get a list of matching entries, should we make this with a new
> } modifier flag that can be combined with `i', `I', `r', and `R' so that 
> } all of them give a list, not only the first matching one?).
> 
> Maybe it's because it's 4:30am, but I don't understand that part at all.

I was thinking about something like the `g' history modifier:

  % typeset -A foo
  % foo[a]=hello
  % foo[b]=world
  % echo $foo[(i)?]
  a
  % echo $foo[(r)*l*]
  hello
  % echo $foo[(xi)*l*]
  a b
  % echo $foo[(xr)*l*]
  hello world

As a side effect, we could use `$foo[(xi)*]' instead of `${(k)foo}'
but the latter would still be faster.

Bye
 Sven


--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: Associative arrays and memory

[Peter Stephenson]

"Bart Schaefer" wrote:
> Of course, a non-special associative array can contain special parameters.
> That was part of the idea of using a parameter hash as the implementation.
> And that would be the way to do it, rather than make the associative array
> itself special.

OK, I missed this point, then you're stuck with something of the
complexity of what you had if you want to restore it.  But what's
wrong with making the associative array special?  It could do some
sort of dispatch based on the values of the hash it stored.  Then you
could get away with doing only a shallow save of the assoc array at
this point (but see below), i.e. pretty much like what happens with
other special parameters.

Possibly in practice the parent AA will have to be marked as special,
even if its own functionality is as normal.  The alternatives to that
are (1) check all non-special AA's for special contents at the point
in question, which probably isn't on (2) the elements of the AA stop
being special, which is a bit counterintuitive.

> It doesn't matter that a whole array is being assigned to the hash, does
> it?  Even creating a scalar with the same parameter name would require
> that it be saved/restored.

That's true if the assoc array is not special, but its elements are.
If the AA is itself special, it retains that behaviour on set/restore.
Consider
   PATH=foo:bar builtin_or_func
--- here PATH still works the same way as always, just with special
save/restore.  Then assigning a scalar to zle would give some kind of
error.

> So the question is, should it ever be the case that
> 
>     zle=(? BUFFER "this is the command line" ... ?) builtin_or_func
> 
> causes the old command line to be restored following builtin_or_func?
> And if we save and restore such a hash by the simple stacking method, is
> $zle[BUFFER] going to give the right thing after builtin_or_func?

If the bits of the AA are special, and you want to restore them, then
as I said above it seems you'll need the full complexity of the
copyarams() mechanism because you can't guarantee restoring the state
otherwise.  Should it work?  Probably yes.  At some stage we will want
whole assoc array assignments like 'hash2=$hash1', which presumably
requires something like copyparams() anyway.  Maybe I'll just have to
bit the bullet and check for memory leaks properly :-(.

-- 
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

Re: Associative arrays and memory

[Bart Schaefer]

On Nov 14,  4:26pm, Peter Stephenson wrote:
} Subject: Re: Associative arrays and memory
}
} "Bart Schaefer" wrote:
} > zagzig<17> HISTSIZE=1000 
} > zagzig<18> HISTSIZE=0 echo hello
} > hello
} > zagzig<19> echo $HISTSIZE
} > 2
} 
} This seems to be OK after restoring the old behaviour, so I haven't
} look further into what had gone wrong.

I found it ... the assignment "pm = tpm;" was mistakenly deleted from
below the code that was replaced by the call to copyparam().  That is,
in save_params() in exec.c, it should look like

	tpm->nam = s;
	copyparam(tpm, pm);
	pm = tpm;

So copyparam() itself is OK.

} > } There are currently no special assoc arrays, of course, and it should
} > } probably be possible to prevent there being any
} > 
} > What about the discussion that started all this -- using an associative
} > array to give user access to shell-internal completion data?
} 
} Rats, I just looked at zle_params.c and you're right --- they're
} marked special.

Of course, a non-special associative array can contain special parameters.
That was part of the idea of using a parameter hash as the implementation.
And that would be the way to do it, rather than make the associative array
itself special.

} That doesn't mean assoc arrays need to be the same, though.  The case
} that needs worrying about is the following:
} 
}   hash=(?...?) builtin_or_func
} 
} where the ?...? represents whatever we pick for whole array
} assignments.

It doesn't matter that a whole array is being assigned to the hash, does
it?  Even creating a scalar with the same parameter name would require
that it be saved/restored.

Or are you talking *about* special parameters within the associative array
that need to be saved, and that's why whole-array assignment would matter?

} There's also no problem if the hash is simply used for storing
} information and is not tied to special variables or functions.  You
} only need a special mechanism for restoration for something like
} $path, where there's an internal variable that needs setting; simply
} restoring the struct param won't do that.

That is, you need to call the pm->sets functions when restoring specials,
which is what restore_params() does.  But my implementation does not call
anything equivalent to restore_params() on the parameter table stored in
the association, so it isn't helping to have save_params() copy it in the
first place.

} If we can agree that use of
} assoc arrays is simply going to be by direct access to the parameter
} we can avoid ever copying it: the above shell pseudocode simply makes
} the supplied $hash available for the duration of builtin_func.

I'm not entirely sure what you mean about "direct access."  If we're
going to put stuff like the present $BUFFER into an association, it needs
to be the case that assignment to (say) zle[BUFFER] actually modifies the
line editor state.

So the question is, should it ever be the case that

    zle=(? BUFFER "this is the command line" ... ?) builtin_or_func

causes the old command line to be restored following builtin_or_func?
And if we save and restore such a hash by the simple stacking method, is
$zle[BUFFER] going to give the right thing after builtin_or_func?

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Re: Associative arrays and memory

[Peter Stephenson]

"Bart Schaefer" wrote:
> } 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.
> 
> I'm not so sure any more.  When restore_params() is called, it uses the
> pm->sets functions to assign directly to the struct param already in the
> paramtab.  So that memory is now owned by the paramtab and should be
> freed when the param next becomes unset ... right?

Yes, it looks like you're right.  Maybe it's still unnecessarily
confusing.  The struct param is really just a placeholder with a type
flag for the real information.

> zagzig<17> HISTSIZE=1000 
> zagzig<18> HISTSIZE=0 echo hello
> hello
> zagzig<19> echo $HISTSIZE
> 2

This seems to be OK after restoring the old behaviour, so I haven't
look further into what had gone wrong.

> } There are currently no special assoc arrays, of course, and it should
> } probably be possible to prevent there being any
> 
> What about the discussion that started all this -- using an associative
> array to give user access to shell-internal completion data?

Rats, I just looked at zle_params.c and you're right --- they're
marked special.  I had wrongly assumed it was just a case of setting
the value at the start and using it at the end, but there tied to
functions.

That doesn't mean assoc arrays need to be the same, though.  The case
that needs worrying about is the following:

  hash=(?...?) builtin_or_func

where the ?...? represents whatever we pick for whole array
assignments.  If there is none, there's no problem; that's the only
case where copying parameters is necessary.

There's also no problem if the hash is simply used for storing
information and is not tied to special variables or functions.  You
only need a special mechanism for restoration for something like
$path, where there's an internal variable that needs setting; simply
restoring the struct param won't do that.  If we can agree that use of
assoc arrays is simply going to be by direct access to the parameter
we can avoid ever copying it: the above shell pseudocode simply makes
the supplied $hash available for the duration of builtin_func.  It
would certainly keep things much simpler, and I can provide a patch
straight away.

So, the question is are there any uses for special hashes which would
require tying them directly to an internal variable or function, or
can they always be accessed by the standard parameter functions?  I
would think the whole point of using assoc arrays is to avoid any
unpleasantness of the former kind.  Probably Sven can answer that
better than anyone.

-- 
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

Re: Associative arrays and memory

[Bart Schaefer]

On Nov 13,  5:16pm, Peter Stephenson wrote:
} Subject: Associative arrays and memory
} 
} 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.

I'm not so sure any more.  When restore_params() is called, it uses the
pm->sets functions to assign directly to the struct param already in the
paramtab.  So that memory is now owned by the paramtab and should be
freed when the param next becomes unset ... right?

} 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).

I appear to have broken it somehow, then:

zagzig<17> HISTSIZE=1000 
zagzig<18> HISTSIZE=0 echo hello
hello
zagzig<19> echo $HISTSIZE
2

Nonspecial variables work:

zagzig<23> FOO=before
zagzig<24> FOO=after echo hello
hello
zagzig<25> echo $FOO
before

} There are currently no special assoc arrays, of course, and it should
} probably be possible to prevent there being any

What about the discussion that started all this -- using an associative
array to give user access to shell-internal completion data?

} 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.
} 
} 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.)

If you have time and inclination, please go ahead and do this.  I'm not
going to be able to work on it again until tomorrow at the earliest.

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Associative arrays and memory

[Peter Stephenson]

"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