* Re: Associative arrays and memory
@ 1998-11-16 9:54 Sven Wischnowsky
1998-11-16 12:43 ` Bart Schaefer
0 siblings, 1 reply; 12+ messages in thread
From: Sven Wischnowsky @ 1998-11-16 9:54 UTC (permalink / raw)
To: zsh-workers
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Associative arrays and memory 1998-11-16 9:54 Associative arrays and memory Sven Wischnowsky @ 1998-11-16 12:43 ` Bart Schaefer 1998-11-16 14:58 ` Ksh93 (was Re: Associative arrays and memory) Bruce Stephens 1998-11-16 17:16 ` PATCH: 3.1.5: assoc array memory mucking around tedium Peter Stephenson 0 siblings, 2 replies; 12+ messages in thread From: Bart Schaefer @ 1998-11-16 12:43 UTC (permalink / raw) To: zsh-workers 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Ksh93 (was Re: Associative arrays and memory) 1998-11-16 12:43 ` Bart Schaefer @ 1998-11-16 14:58 ` Bruce Stephens 1998-11-16 17:16 ` PATCH: 3.1.5: assoc array memory mucking around tedium Peter Stephenson 1 sibling, 0 replies; 12+ messages in thread From: Bruce Stephens @ 1998-11-16 14:58 UTC (permalink / raw) To: zsh-workers "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. ^ permalink raw reply [flat|nested] 12+ messages in thread
* PATCH: 3.1.5: assoc array memory mucking around tedium @ 1998-11-16 17:16 ` Peter Stephenson 1998-11-17 8:15 ` assoc array memory mucking, and semantics of patterned keys Bart Schaefer 0 siblings, 1 reply; 12+ messages in thread From: Peter Stephenson @ 1998-11-16 17:16 UTC (permalink / raw) To: Zsh hackers list 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: assoc array memory mucking, and semantics of patterned keys 1998-11-16 17:16 ` PATCH: 3.1.5: assoc array memory mucking around tedium Peter Stephenson @ 1998-11-17 8:15 ` Bart Schaefer 1998-11-17 8:47 ` Peter Stephenson 0 siblings, 1 reply; 12+ messages in thread From: Bart Schaefer @ 1998-11-17 8:15 UTC (permalink / raw) To: zsh-workers 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: assoc array memory mucking, and semantics of patterned keys 1998-11-17 8:15 ` assoc array memory mucking, and semantics of patterned keys Bart Schaefer @ 1998-11-17 8:47 ` Peter Stephenson 0 siblings, 0 replies; 12+ messages in thread From: Peter Stephenson @ 1998-11-17 8:47 UTC (permalink / raw) To: zsh-workers "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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Associative arrays and memory @ 1998-11-16 13:15 Sven Wischnowsky 0 siblings, 0 replies; 12+ messages in thread From: Sven Wischnowsky @ 1998-11-16 13:15 UTC (permalink / raw) To: zsh-workers 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: PATCH: 3.1.5 - sample associative array implementation @ 1998-11-11 18:16 Bart Schaefer 1998-11-13 16:16 ` Associative arrays and memory Peter Stephenson 0 siblings, 1 reply; 12+ messages in thread From: Bart Schaefer @ 1998-11-11 18:16 UTC (permalink / raw) To: Peter Stephenson, Zsh hackers list Thanks for looking at this, Peter. On Nov 11, 2:58pm, Peter Stephenson wrote: } Subject: Re: PATCH: 3.1.5 - sample associative array implementation } } "Bart Schaefer" wrote: } > However, there are some caveats: (1) You can't assign to multiple elements } > of an associative array in a single expression; you must always use the } > subscript syntax. } } One question is whether } hash=(key1 val1 key2 val2) } replaces the array entirely, or just adds/replaces those elements. The problem with that syntax is that you can't tell whether `hash' is intended to be an associative array, so you still have to declare it first. (Perl handles this by always prefixing variable names with $ @ or % even when assigning.) The more lispy hash=((key1 val1) (key2 val2)) appears to be syntactically invalid at present ("number expected" ??). However, it appears that it WAS valid in 3.0.5, so perhaps that's a bug in 3.1.5. } In } the former case it's difficult to think of a way of replacing multiple } elements at once [...] } Another thing: there's no way of getting the keys of the hash. What's needed is a syntax to have the hash produce its key/value pairs in the appropriate form so that you can do e.g. hash=(${(kv)hash} key1 newval1 key2 newval2) (where ${(k)hash} means substitute the keys, and ${(v)hash} the values, which is also the default when (k) is not present). Then simply decree that, after the whole hash is replaced, elements are assigned in left-to-right order, replacing duplicates. Then you can do hash=(key 1 newval1 key2 newval2 ${(kv)hash}) to insert key1 and key2 only if they were not already present. } Something like $hash[(k)*] would be OK, except that * and @ don't seem } to work with flags at the moment. I think an expansion flag rather than a subscription flag is the way, see sample above. } > I'm afraid there may be some memory leaks in the code to form } > arrays from the values. } } I put in a MUSTUSEHEAP to check for this. It hasn't printed a message } so far. 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. } > When an associative array element is referenced, it's hash table slot is } > created and initially marked PM_UNSET. } } This means (post patch): } } % typeset -H hash } % hash[one]=eins } % print $hash[two] } } % print -l "$hash[@]" } one } <- $hash[two] was created unset } % } } Is this really correct? We can fix that by changing the way the hashtable scan happens, so it ignores unset parameters. The patch below fixes this, and adds some support for the ${(k)hash} substitution flag, but I didn't actually put in those flags themselves. Will v->pm->flags &= ~PM_UNSET hurt anything in setstrvalue()? It didn't seem as if it should, but there are many details I don't understand. } > (5) $assoc[bread,3] produces "but" (the first 3 characters of the } > value) which I think is because getarg() doesn't return soon enough; it } > really ought to either ignore or gripe about what comes after the comma. } } I haven't touched this. I no longer think it's as simple as I previously thought, so I didn't do any more with it either. This patch of course requires both my previous one and Peter's. Index: Src/params.c =================================================================== --- params.c 1998/11/11 17:40:25 1.5 +++ params.c 1998/11/11 18:14:34 @@ -295,13 +295,21 @@ return nht; } +#define SCANPM_WANTVALS (1<<0) +#define SCANPM_WANTKEYS (1<<1) +#define SCANPM_WANTINDEX (1<<2) + static unsigned numparamvals; /**/ static void scancountparams(HashNode hn, int flags) { - ++numparamvals; + if (!(((Param)hn)->flags & PM_UNSET)) { + ++numparamvals; + if ((flags & SCANPM_WANTKEYS) && (flags & SCANPM_WANTVALS)) + ++numparamvals; + } } static char **paramvals; @@ -311,26 +319,33 @@ scanparamvals(HashNode hn, int flags) { struct value v; - v.isarr = (flags & 1); v.pm = (Param)hn; - v.inv = (flags & 2); - v.a = 0; - v.b = -1; - paramvals[numparamvals++] = getstrvalue(&v); + if (!(v.pm->flags & PM_UNSET)) { + if (flags & SCANPM_WANTKEYS) { + paramvals[numparamvals++] = v.pm->nam; + if (!(flags & SCANPM_WANTVALS)) + return; + } + v.isarr = (PM_TYPE(v.pm->flags) & (PM_ARRAY|PM_HASHED)); + v.inv = (flags & SCANPM_WANTINDEX); + v.a = 0; + v.b = -1; + paramvals[numparamvals++] = getstrvalue(&v); + } } /**/ char ** -paramvalarr(HashTable ht) +paramvalarr(HashTable ht, unsigned flags) { MUSTUSEHEAP("paramvalarr"); numparamvals = 0; if (ht) - scanhashtable(ht, 0, 0, 0, scancountparams, 0); + scanhashtable(ht, 0, 0, 0, scancountparams, flags); paramvals = (char **) alloc(numparamvals * sizeof(char **) + 1); if (ht) { numparamvals = 0; - scanhashtable(ht, 0, 0, 0, scanparamvals, 0); + scanhashtable(ht, 0, 0, 0, scanparamvals, flags); } paramvals[numparamvals] = 0; return paramvals; @@ -348,7 +363,7 @@ else if (PM_TYPE(v->pm->flags) == PM_ARRAY) return v->arr = v->pm->gets.afn(v->pm); else if (PM_TYPE(v->pm->flags) == PM_HASHED) - return v->arr = paramvalarr(v->pm->gets.hfn(v->pm)); + return v->arr = paramvalarr(v->pm->gets.hfn(v->pm), 0); else return NULL; } @@ -1133,6 +1148,7 @@ zsfree(val); return; } + v->pm->flags &= ~PM_UNSET; switch (PM_TYPE(v->pm->flags)) { case PM_SCALAR: MUSTUSEHEAP("setstrvalue"); -- Bart Schaefer Brass Lantern Enterprises http://www.well.com/user/barts http://www.brasslantern.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* Associative arrays and memory 1998-11-11 18:16 PATCH: 3.1.5 - sample associative array implementation Bart Schaefer @ 1998-11-13 16:16 ` Peter Stephenson 1998-11-13 17:57 ` Bart Schaefer 0 siblings, 1 reply; 12+ messages in thread From: Peter Stephenson @ 1998-11-13 16:16 UTC (permalink / raw) To: Zsh hackers list "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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Associative arrays and memory 1998-11-13 16:16 ` Associative arrays and memory Peter Stephenson @ 1998-11-13 17:57 ` Bart Schaefer 1998-11-14 15:26 ` Peter Stephenson 0 siblings, 1 reply; 12+ messages in thread From: Bart Schaefer @ 1998-11-13 17:57 UTC (permalink / raw) To: Peter Stephenson, Zsh hackers list 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Associative arrays and memory 1998-11-13 17:57 ` Bart Schaefer @ 1998-11-14 15:26 ` Peter Stephenson 1998-11-14 18:47 ` Bart Schaefer 0 siblings, 1 reply; 12+ messages in thread From: Peter Stephenson @ 1998-11-14 15:26 UTC (permalink / raw) To: Zsh hackers list "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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Associative arrays and memory 1998-11-14 15:26 ` Peter Stephenson @ 1998-11-14 18:47 ` Bart Schaefer 1998-11-15 15:54 ` Peter Stephenson 0 siblings, 1 reply; 12+ messages in thread From: Bart Schaefer @ 1998-11-14 18:47 UTC (permalink / raw) To: Peter Stephenson, Zsh hackers list 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Associative arrays and memory 1998-11-14 18:47 ` Bart Schaefer @ 1998-11-15 15:54 ` Peter Stephenson 0 siblings, 0 replies; 12+ messages in thread From: Peter Stephenson @ 1998-11-15 15:54 UTC (permalink / raw) To: Zsh hackers list "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 ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~1998-11-17 9:10 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 1998-11-16 9:54 Associative arrays and memory Sven Wischnowsky 1998-11-16 12:43 ` Bart Schaefer 1998-11-16 14:58 ` Ksh93 (was Re: Associative arrays and memory) Bruce Stephens 1998-11-16 17:16 ` PATCH: 3.1.5: assoc array memory mucking around tedium Peter Stephenson 1998-11-17 8:15 ` assoc array memory mucking, and semantics of patterned keys Bart Schaefer 1998-11-17 8:47 ` Peter Stephenson -- strict thread matches above, loose matches on Subject: below -- 1998-11-16 13:15 Associative arrays and memory Sven Wischnowsky 1998-11-11 18:16 PATCH: 3.1.5 - sample associative array implementation Bart Schaefer 1998-11-13 16:16 ` Associative arrays and memory Peter Stephenson 1998-11-13 17:57 ` Bart Schaefer 1998-11-14 15:26 ` Peter Stephenson 1998-11-14 18:47 ` Bart Schaefer 1998-11-15 15:54 ` Peter Stephenson
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).