PATCH: $killring now works, perhaps...

[Peter Stephenson <pws@xxxxxxx>]

I took a closer look at using $killring in anger and there were a whole
heap of problems...

- memory management was wrong in several places leading to core dumps
  (I hadn't properly worked out what was a NULL-terminated string and
  what had an explicit length and what that length actually was)
- looping over elements when setting $killring was wrong, it went in
  the wrong direction
- getting the killring when there was nothing in it should have returned
  the default length but didn't.

I've been validating this with the following chunk of code which
emulates bash-backward-kill-word.  I think this gives a reasonably
intuitive method for cycling the kill ring, but let me know if you think
it should work differently.

(The flag that makes consecutive kills join together in the cut buffer
is unimplemented.  I think it can be done in shell code by looking at
LASTWIDGET and seeing if it contained the string `kill', although that's
a bit ugly.)


emulate -L zsh
setopt extendedglob

local -a match mbegin mend

LBUFFER=${LBUFFER%%(#b)([[:alnum:]]##[^[:alnum:]]#)}

[[ -z $match[1] ]] && return 1

killring=($CUTBUFFER "${(@)killring[1,-2]}")
CUTBUFFER=$match[1]


Index: Src/Zle/zle_params.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_params.c,v
retrieving revision 1.9
diff -u -r1.9 zle_params.c
--- Src/Zle/zle_params.c	5 Mar 2003 17:24:55 -0000	1.9
+++ Src/Zle/zle_params.c	24 Mar 2003 13:00:10 -0000
@@ -365,8 +365,8 @@
     if (x) {
 	unmetafy(x, &cutbuf.len);
 	cutbuf.buf = zalloc(cutbuf.len);
-	strcpy((char *)cutbuf.buf, x);
-	zsfree(x);
+	memcpy((char *)cutbuf.buf, x, cutbuf.len);
+	free(x);
     } else {
 	cutbuf.buf = NULL;
 	cutbuf.len = 0;
@@ -398,7 +398,7 @@
     if (kring) {
 	for (kptr = kring, kcnt = 0; kcnt < kringsize; kcnt++, kptr++)
 	    if (kptr->buf)
-		free(kptr->buf);
+		zfree(kptr->buf, kptr->len);
 	zfree(kring, kringsize * sizeof(struct cutbuffer));
 	kring = NULL;
 	kringsize = kringnum = 0;
@@ -408,23 +408,23 @@
 	 * Insert the elements into the kill ring.
 	 * Regardless of the old order, we number it with the current
 	 * entry first.
+	 *
+	 * Be careful to add elements by looping backwards; this
+	 * fits in with how we cycle the ring.
 	 */
+	int kpos = 0;
 	kringsize = arrlen(x);
 	kring = (Cutbuffer)zcalloc(kringsize * sizeof(struct cutbuffer));
-	for (p = x, kptr = kring; *p; p++, kptr++) {
+	for (p = x; *p; p++) {
 	    int len = strlen(*p);
-	    kptr->buf = (char *)zalloc(len);
-	    strcpy(kptr->buf, *p);
-	    unmetafy(kptr->buf, &kptr->len);
-	    if (len != kptr->len) {
-		/* Might as well have the lengths consistent. */
-		char *p2 = zalloc(kptr->len);
-		memcpy(p2, kptr->buf, kptr->len);
-		zfree(kptr->buf, len);
-		kptr->buf = p2;
-	    }
+	    kptr = kring + kpos;
+	    unmetafy(*p, &kptr->len);
+	    kptr->buf = (char *)zalloc(kptr->len);
+	    memcpy(kptr->buf, *p, kptr->len);
+	    zfree(*p, len+1);
+	    kpos = (kpos + kringsize -1 ) % kringsize;
 	}
-	freearray(x);
+	free(x);
     }
 }
 
@@ -441,10 +441,9 @@
     char **ret, **p;
 
     /* Supposed to work even if kring is NULL */
-    for (kpos = kringnum, kcnt = 0; kcnt < kringsize; kcnt++) {
-	if (!kring[kpos].buf)
-	    break;
-	kpos = (kpos + kringsize - 1) % kringsize;
+    if (!kring) {
+	kringsize = KRINGCTDEF;
+	kring = (Cutbuffer)zcalloc(kringsize * sizeof(struct cutbuffer));
     }
 
     p = ret = (char **)zhalloc((kringsize+1) * sizeof(char *));
@@ -452,7 +451,13 @@
     for (kpos = kringnum, kcnt = 0; kcnt < kringsize; kcnt++) {
 	Cutbuffer kptr = kring + kpos;
 	if (kptr->buf)
+	{
+	    /*
+	     * Need to use HEAPDUP to make sure there's room for the
+	     * terminating NULL.
+	     */
 	    *p++ = metafy((char *)kptr->buf, kptr->len, META_HEAPDUP);
+	}
 	else
 	    *p++ = dupstring("");
 	kpos = (kpos + kringsize - 1) % kringsize;

-- 
Peter Stephenson <pws@xxxxxxx>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************