[PATCH] Refine string and array slice assignments

[Philippe Altherr <philippe.altherr@xxxxxxxxx>]

String and array slice assignments are very similar but their implementations in setarrvalue and assignstrvalue use different variables names and have slightly different code structures. Both implementations try to reuse and/or reallocate the string/array pointer of the assigned value but both do it only under rather restricted conditions.

This patch unifies the variable names and the code structure of the two implementations and modifies them to reuse and/or reallocate the string/array pointer of the assigned value whenever possible. More reuse is particularly beneficial for array assignments because whenever the pointer can be reused it avoids the need to duplicate the array strings that are preserved by the slice assignment.

The patch is the sum of multiple commits visible on GitHub:

- Refine string and array slice assignments

The patch depends on workers/54309 and workers/54342 on top of which it is built.

Philippe

diff --git a/Src/params.c b/Src/params.c
index 43a1a8b3a..749004499 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -2728,29 +2728,28 @@ assignstrvalue(Value v, char *val, int flags)
 		!v->pm->width)
 		v->pm->width = strlen(val);
 	} else {
-	    char *z, *x;
-            int zlen, vlen, newsize;
-
-            z = v->pm->gsu.s->getfn(v->pm);
-            zlen = strlen(z);
+	    char *const old = v->pm->gsu.s->getfn(v->pm);
+	    char *new;
+            const int oldlen = strlen(old), vallen = strlen(val);
+	    int newlen;
 
 	    if ((v->valflags & VALFLAG_INV) && unset(KSHARRAYS))
 		v->start--, v->end--;
 	    if (v->start < 0) {
-		v->start += zlen;
+		v->start += oldlen;
 		if (v->start < 0)
 		    v->start = 0;
 	    }
-	    if (v->start > zlen)
-		v->start = zlen;
+	    if (v->start > oldlen)
+		v->start = oldlen;
 	    if (v->end < 0) {
-		v->end += zlen;
+		v->end += oldlen;
 		if (v->end < 0) {
 		    v->end = 0;
 		} else {
 #ifdef MULTIBYTE_SUPPORT
 		    if (isset(MULTIBYTE)) {
-			v->end += MB_METACHARLEN(z + v->end);
+			v->end += MB_METACHARLEN(old + v->end);
 		    } else {
 			v->end++;
 		    }
@@ -2761,30 +2760,33 @@ assignstrvalue(Value v, char *val, int flags)
 	    }
 	    if (v->end < v->start)
 		v->end = v->start;
-	    else if (v->end > zlen)
-		v->end = zlen;
-
-            vlen = strlen(val);
-            /* Characters preceding start index +
-               characters of what is assigned +
-               characters following end index */
-            newsize = v->start + vlen + (zlen - v->end);
-
-            /* Does new size differ? */
-            if (newsize != zlen ||
-		v->pm->gsu.s->setfn != strsetfn || z != v->pm->u.str) {
-                x = (char *) zalloc(newsize + 1);
-                strncpy(x, z, v->start);
-                strcpy(x + v->start, val);
-                strcat(x + v->start, z + v->end);
-                v->pm->gsu.s->setfn(v->pm, x);
+	    else if (v->end > oldlen)
+		v->end = oldlen;
+
+	    /* Chars before slice + chars from val + chars after slice */
+            newlen = v->start + vallen + (oldlen - v->end);
+
+            if (v->pm->gsu.s->setfn == strsetfn && old == v->pm->u.str) {
+		v->pm->u.str = NULL; /* Steal the old string */
+		/* If the string shrinks, move left chars after the slice */
+		if (newlen < oldlen && v->end < oldlen)
+		    memmove(old + v->start + vallen, old + v->end,
+			    oldlen - v->end);
+		/* If the string shrinks or expands, reallocate it */
+		new = oldlen == newlen ? old : (char *) zrealloc(old, newlen + 1);
+		/* If the string expands, move right chars after the slice */
+		if (newlen > oldlen && v->end < oldlen)
+		    memmove(new + v->start + vallen, new + v->end,
+			    oldlen - v->end);
             } else {
-                /* Size doesn't change, can limit actions to only
-                 * overwriting bytes in already allocated string */
-		memcpy(z + v->start, val, vlen);
-                v->pm->gsu.s->setfn(v->pm, z);
+                new = (char *) zalloc(newlen + 1);
+                strncpy(new, old, v->start);
+                strncpy(new + v->start + vallen, old + v->end, oldlen - v->end);
             }
+	    strncpy(new + v->start, val, vallen);
             zsfree(val);
+	    new[newlen] = '\0';
+	    v->pm->gsu.s->setfn(v->pm, new);
 	}
 	break;
     case PM_INTEGER:
@@ -2940,13 +2942,9 @@ setarrvalue(Value v, char **val)
 	return;
     } else {
 	char **const old = v->pm->gsu.a->getfn(v->pm);
-	char **new;
-	char **p, **q, **r; /* index variables */
-	const int pre_assignment_length = arrlen(old);
-	int post_assignment_length;
-	int i;
-
-	q = old;
+	char **new, **p, **q;
+	const int oldlen = arrlen(old), vallen = arrlen(val);
+	int newlen, i;
 
 	if ((v->valflags & VALFLAG_INV) && unset(KSHARRAYS)) {
 	    if (v->start > 0)
@@ -2954,95 +2952,55 @@ setarrvalue(Value v, char **val)
 	    v->end--;
 	}
 	if (v->start < 0) {
-	    v->start += pre_assignment_length;
+	    v->start += oldlen;
 	    if (v->start < 0)
 		v->start = 0;
 	}
 	if (v->end < 0) {
-	    v->end += pre_assignment_length + 1;
+	    v->end += oldlen + 1;
 	    if (v->end < 0)
 		v->end = 0;
 	}
-	if (v->end < v->start)
+	if (v->end < v->start || v->start > oldlen)
 	    v->end = v->start;
-
-	post_assignment_length = v->start + arrlen(val);
-	if (v->end < pre_assignment_length) {
-	    /* 
-	     * Allocate room for array elements between the end of the slice `v'
-	     * and the original array's end.
-	     */
-	    post_assignment_length += pre_assignment_length - v->end;
-	}
-
-	if (pre_assignment_length == post_assignment_length
-	    && v->pm->gsu.a->setfn == arrsetfn && old == v->pm->u.arr)
-	{
-	    /* v->start is 0-based */
-	    p = old + v->start;
-	    for (r = val; *r;) {
-		/* Free previous string */
-		zsfree(*p);
-		/* Give away ownership of the string */
-		*p++ = *r++;
-	    }
-	    v->pm->gsu.a->setfn(v->pm, old);
+	else if (v->end > oldlen)
+	    v->end = oldlen;
+
+	/* Strings before slice + strings from val + strings after slice */
+	newlen = v->start + vallen + MAX(0, oldlen - v->end);
+
+	if (v->pm->gsu.a->setfn == arrsetfn && old == v->pm->u.arr) {
+	    v->pm->u.arr = NULL; /* Steal the old array */
+	    /* Free strings that are part of the slice */
+	    for (q = old + v->start, i = v->end - v->start; i > 0; i--)
+		zsfree(*q++);
+	    /* If the array shrinks, move left strings after the slice */
+	    if (newlen < oldlen && v->end < oldlen)
+		memmove(old + v->start + vallen, old + v->end,
+			sizeof(char *) * (oldlen - v->end));
+	    /* If the array shrinks or expands, reallocate it */
+	    new = oldlen == newlen ? old :
+		(char **) zrealloc(old, sizeof(char *) * (newlen + 1));
+	    /* If the array expands, move right strings after the slice */
+	    if (newlen > oldlen && v->end < oldlen)
+		memmove(new + v->start + vallen, new + v->end,
+			sizeof(char *) * (oldlen - v->end));
 	} else {
-            /* arr+=( ... )
-             * arr[${#arr}+x,...]=( ... ) */
-            if (post_assignment_length > pre_assignment_length &&
-                    pre_assignment_length <= v->start &&
-                    v->pm->gsu.a->setfn == arrsetfn && old == v->pm->u.arr)
-            {
-                p = new = (char **) zrealloc(old, sizeof(char *)
-                                           * (post_assignment_length + 1));
-
-                p += pre_assignment_length; /* after old elements */
-
-                /* Consider 1 < 0, case for a=( 1 ); a[1,..] =
-                 *          1 < 1, case for a=( 1 ); a[2,..] = */
-                if (pre_assignment_length < v->start) {
-                    for (i = pre_assignment_length; i < v->start; i++) {
-                        *p++ = ztrdup("");
-                    }
-                }
-
-                for (r = val; *r;) {
-                    /* Give away ownership of the string */
-                    *p++ = *r++;
-                }
-
-                /* v->end doesn't matter:
-                 * a=( 1 2 ); a[4,100]=( a b ); echo "${(q@)a}"
-                 * 1 2 '' a b */
-                *p = NULL;
-
-                v->pm->u.arr = NULL;
-                v->pm->gsu.a->setfn(v->pm, new);
-            } else {
-                p = new = (char **) zalloc(sizeof(char *)
-                                           * (post_assignment_length + 1));
-                for (i = 0; i < v->start; i++)
-                    *p++ = i < pre_assignment_length ? ztrdup(*q++) : ztrdup("");
-                for (r = val; *r;) {
-                    /* Give away ownership of the string */
-                    *p++ = *r++;
-                }
-                if (v->end < pre_assignment_length)
-                    for (q = old + v->end; *q;)
-                        *p++ = ztrdup(*q++);
-                *p = NULL;
-
-                v->pm->gsu.a->setfn(v->pm, new);
-            }
-
-	    DPUTS2(p - new != post_assignment_length, "setarrvalue: wrong allocation: %d 1= %lu",
-		   post_assignment_length, (unsigned long)(p - new));
+	    new = (char **) zalloc(sizeof(char *) * (newlen + 1));
+	    for (p = new, q = old, i = MIN(v->start, oldlen); i > 0; i--)
+		*p++ = ztrdup(*q++);
+	    if (v->end < oldlen)
+		for (p = new + v->start + vallen, q = old + v->end; *q;)
+		    *p++ = ztrdup(*q++);
 	}
-
-        /* Ownership of all strings has been
-         * given away, can plainly free */
+	/* Copy and give away ownership of the strings from val */
+	memcpy(new + v->start, val, sizeof(char *) * vallen);
 	free(val);
+	/* Fill any gap between the old array end and the slice start */
+	for (p = new + oldlen, i = v->start - oldlen; i > 0; i--)
+	    *p++ = ztrdup("");
+	new[newlen] = NULL;
+	v->pm->gsu.a->setfn(v->pm, new);
     }
 }