Re: Avoid strlen calls after sprintf

[Oliver Kiddle <opk@xxxxxxx>]

Bart Schaefer wrote:
> On Mon, Jan 27, 2025 at 5:36 PM Oliver Kiddle <opk@xxxxxxx> wrote:
> In several of these cases it would also be possible to change
> dupstring() to dupstring_wlen(), because now the length is known
> before dup-ing.

At some point we also added dupstring_glen() which factors the strlen()
into the function but returns the value back out. The one use of that,
added alongside it, got removed at some point leaving that function
completely unused. It occurs to me that, given, the wlen variant, it is
redundant because you can call strlen() outside, pass the value in and
achieve the result of only one strlen() call. So I remove it in this
patch.

I also looked more widely for places where we already have the length
available to us and can simply pass it in. In a couple of cases, this
also avoids the need to temporarily add a NULL because dupstring_wlen()
doesn't need one.

There are further cases, mostly in subst.c and params.c, where we would need
to add a new variable for the length and it is only doing sprintf with
something like "%ld" so I'm not sure it's worth doing.

Oliver

diff --git a/Src/Zle/compcore.c b/Src/Zle/compcore.c
index 09282d42d..fb703f801 100644
--- a/Src/Zle/compcore.c
+++ b/Src/Zle/compcore.c
@@ -708,7 +708,7 @@ callcompfunc(char *s, char *fn)
 
 	    sav = *ss;
 	    *ss = '\0';
-	    tmp = (linwhat == IN_MATH ? dupstring(s) : multiquote(s, 0));
+	    tmp = (linwhat == IN_MATH ? dupstring_wlen(s, offs) : multiquote(s, 0));
 	    untokenize(tmp);
 	    compprefix = ztrdup(tmp);
 	    *ss = sav;
@@ -1820,7 +1820,7 @@ set_comp_sep(void)
      */
     sav = s[(i = swb - 1 - sqq + dq)];
     s[i] = '\0';
-    qp = (qttype == QT_SINGLE) ? dupstring(s) : rembslash(s);
+    qp = (qttype == QT_SINGLE) ? dupstring_wlen(s, i) : rembslash(s);
     s[i] = sav;
     if (swe < swb)
 	swe = swb;
@@ -2244,10 +2244,10 @@ addmatches(Cadata dat, char **argv)
 	if (dat->aflags & CAF_MATCH) {
 	    lipre = dupstring(compiprefix);
 	    lisuf = dupstring(compisuffix);
-	    lpre = dupstring(compprefix);
-	    lsuf = dupstring(compsuffix);
-	    llpl = strlen(lpre);
-	    llsl = strlen(lsuf);
+	    llpl = strlen(compprefix);
+	    llsl = strlen(compsuffix);
+	    lpre = dupstring_wlen(compprefix, llpl);
+	    lsuf = dupstring_wlen(compsuffix, llsl);
 
 	    /* This used to reference compqiprefix and compqisuffix, why? */
 	    if (llpl + (int)strlen(qipre) + (int)strlen(lipre) != origlpre
@@ -2300,12 +2300,8 @@ addmatches(Cadata dat, char **argv)
 		for (p = lpre + 2; *p && *p != ')'; p++);
 
 		if (*p == ')') {
-		    char sav = p[1];
-
-		    p[1] = '\0';
-		    globflag = dupstring(lpre);
 		    gfl = p - lpre + 1;
-		    p[1] = sav;
+		    globflag = dupstring_wlen(lpre, gfl);
 
 		    lpre = p + 1;
 		    llpl -= gfl;
@@ -2731,7 +2727,7 @@ add_match_data(int alt, char *str, char *orig, Cline line,
 	    sl = tsl;
 	}
 	if (qisl) {
-	    Cline qsl = bld_parts(dupstring(qisuf), qisl, qisl, NULL, NULL);
+	    Cline qsl = bld_parts(dupstring_wlen(qisuf, qisl), qisl, qisl, NULL, NULL);
 
 	    qsl->flags |= CLF_SUF;
 	    qsl->suffix = qsl->prefix;
@@ -2814,7 +2810,7 @@ add_match_data(int alt, char *str, char *orig, Cline line,
 	    line = p;
 	}
 	if (qipl) {
-	    Cline lp, p = bld_parts(dupstring(qipre), qipl, qipl, &lp, NULL);
+	    Cline lp, p = bld_parts(dupstring_wlen(qipre, qipl), qipl, qipl, &lp, NULL);
 
 	    lp->next = line;
 	    line = p;
diff --git a/Src/Zle/compctl.c b/Src/Zle/compctl.c
index 08355d1b9..de3ccdfce 100644
--- a/Src/Zle/compctl.c
+++ b/Src/Zle/compctl.c
@@ -3201,8 +3201,8 @@ makecomplistflags(Compctl cc, char *s, int incmd, int compadd)
 	memcpy(lpre, s, lpl);
     lpre[lpl] = '\0';
     qlpre = quotename(lpre);
-    lsuf = dupstring(s + offs);
-    lsl = strlen(lsuf);
+    lsl = strlen(s + offs);
+    lsuf = dupstring_wlen(s + offs, lsl);
     qlsuf = quotename(lsuf);
 
     /* First check for ~.../... */
diff --git a/Src/Zle/compmatch.c b/Src/Zle/compmatch.c
index ddcecd589..b58bd1f05 100644
--- a/Src/Zle/compmatch.c
+++ b/Src/Zle/compmatch.c
@@ -1161,8 +1161,8 @@ comp_match(char *pfx, char *sfx, char *w, Patprog cp, Cline *clp, int qu,
 
 	/* We still break it into parts here, trying to build a sensible
 	 * cline list for these matches, too. */
-	w = dupstring(w);
 	wl = strlen(w);
+	w = dupstring_wlen(w, wl);
 	*clp = bld_parts(w, wl, wl, NULL, NULL);
 	*exact = 0;
     } else {
@@ -2127,7 +2127,7 @@ cmp_anchors(Cline o, Cline n, int join)
 	(j = join_strs(o->wlen, o->word, n->wlen, n->word))) {
 	o->flags |= CLF_JOIN;
 	o->wlen = strlen(j);
-	o->word = dupstring(j);
+	o->word = dupstring_wlen(j, o->wlen);
 
 	return 2;
     }
diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c
index 360667884..6ac458c91 100644
--- a/Src/Zle/computil.c
+++ b/Src/Zle/computil.c
@@ -1296,8 +1296,7 @@ parse_cadef(char *nam, char **args)
 		int l = strlen(p) - 1;
 
 		if (*p == '(' && p[l] == ')') {
-		    axor = p = dupstring(p + 1);
-		    p[l - 1] = '\0';
+		    axor = p = dupstring_wlen(p + 1, l - 1);
 		} else
 		    axor = NULL;
 		if (!*p) {
@@ -1317,8 +1316,7 @@ parse_cadef(char *nam, char **args)
 	    p = *++args;
 	    l = strlen(p) - 1;
 	    if (*p == '(' && p[l] == ')') {
-		axor = p = dupstring(p + 1);
-		p[l - 1] = '\0';
+		axor = p = dupstring_wlen(p + 1, l - 1);
 	    } else
 		axor = NULL;
 	    if (!*p) {
@@ -1339,7 +1337,7 @@ parse_cadef(char *nam, char **args)
 
 	    LinkList list = newlinklist();
 	    LinkNode node;
-	    char **xp, sav;
+	    char **xp;
 
 	    while (*p && *p != ')') {
 		for (p++; inblank(*p); p++);
@@ -1351,11 +1349,8 @@ parse_cadef(char *nam, char **args)
 		if (!*p)
 		    break;
 
-		sav = *p;
-		*p = '\0';
-		addlinknode(list, dupstring(q));
+		addlinknode(list, dupstring_wlen(q, p - q));
 		xnum++;
-		*p = sav;
 	    }
 	    /* Oops, end-of-string. */
 	    if (*p != ')') {
diff --git a/Src/hist.c b/Src/hist.c
index d0960a284..fa1ede3f0 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -3553,9 +3553,8 @@ bufferwords(LinkList list, char *buf, int *index, int flags)
 	} else if (buf) {
 	    if (IS_REDIROP(tok) && tokfd >= 0) {
 		char b[20];
-
-		sprintf(b, "%d%s", tokfd, tokstrings[tok]);
-		addlinknode(list, dupstring(b));
+		int l = sprintf(b, "%d%s", tokfd, tokstrings[tok]);
+		addlinknode(list, dupstring_wlen(b, l));
 		num++;
 	    } else if (tok != NEWLIN) {
 		addlinknode(list, dupstring(tokstrings[tok]));
diff --git a/Src/string.c b/Src/string.c
index 5f439926e..0fa13ac0d 100644
--- a/Src/string.c
+++ b/Src/string.c
@@ -57,21 +57,6 @@ dupstring_wlen(const char *s, unsigned len)
     return t;
 }
 
-/* Duplicate string on heap, returning length of string */
-
-/**/
-mod_export char *
-dupstring_glen(const char *s, unsigned *len_ret)
-{
-    char *t;
-
-    if (!s)
-	return NULL;
-    t = (char *) zhalloc((*len_ret = strlen((char *)s)) + 1);
-    strcpy(t, s);
-    return t;
-}
-
 /**/
 mod_export char *
 ztrdup(const char *s)