patch 9.1.1467: too many strlen() calls

Problem:  too many strlen() calls
Solution: Change expand_env() to return string length
          (John Marriott)

This commit does the following changes:
- In expand_env_esc():
  - return the length of the returned dst string.
  - refactor to remove some calls to STRLEN() and STRCAT()
  - add check for out-of-memory condition.
- Change call sites in various source files to use the return value

closes: #17561

Signed-off-by: John Marriott <basilisk@internode.on.net>
Signed-off-by: Christian Brabandt <cb@256bit.org>

src/fileio.c

@@ -5351,14 +5351,13 @@ vim_tempname(
 	    long	nr;
 	    long	off;
 # endif
+	    size_t	itmplen;
 
 	    // Expand $TMP, leave room for "/v1100000/999999999".
 	    // Skip the directory check if the expansion fails.
-	    expand_env((char_u *)tempdirs[i], itmp, TEMPNAMELEN - 20);
+	    itmplen = expand_env((char_u *)tempdirs[i], itmp, TEMPNAMELEN - 20);
 	    if (itmp[0] != '$' && mch_isdir(itmp))
 	    {
-		size_t	itmplen = STRLEN(itmp);
-
 		// directory exists
 		if (!after_pathsep(itmp, itmp + itmplen))
 		{

src/findfile.c

@@ -1820,11 +1820,10 @@ find_file_in_path_option(
 	// copy file name into NameBuff, expanding environment variables
 	save_char = ptr[len];
 	ptr[len] = NUL;
-	expand_env_esc(ptr, NameBuff, MAXPATHL, FALSE, TRUE, NULL);
+	file_to_findlen = expand_env_esc(ptr, NameBuff, MAXPATHL, FALSE, TRUE, NULL);
 	ptr[len] = save_char;
 
 	vim_free(*file_to_find);
-	file_to_findlen = STRLEN(NameBuff);
 	*file_to_find = vim_strnsave(NameBuff, file_to_findlen);
 	if (*file_to_find == NULL)	// out of memory
 	{

src/if_cscope.c

@@ -548,8 +548,7 @@ cs_add_common(
     if ((fname = alloc(MAXPATHL + 1)) == NULL)
 	goto add_err;
 
-    expand_env((char_u *)arg1, (char_u *)fname, MAXPATHL);
-    len = STRLEN(fname);
+    len = expand_env((char_u *)arg1, (char_u *)fname, MAXPATHL);
     fbuf = (char_u *)fname;
     (void)modify_fname((char_u *)":p", FALSE, &usedlen,
 					      (char_u **)&fname, &fbuf, &len);
@@ -829,6 +828,7 @@ cs_create_connection(int i)
     int		cmdlen;
     int		len;
     char	*prog, *cmd, *ppath = NULL;
+    size_t	proglen;
 #ifdef MSWIN
     int		fd;
     SECURITY_ATTRIBUTES sa;
@@ -916,10 +916,10 @@ err_closing:
 	    goto err_closing;
 #endif
 	}
-	expand_env(p_csprg, (char_u *)prog, MAXPATHL);
+	proglen = expand_env(p_csprg, (char_u *)prog, MAXPATHL);
 
 	// alloc space to hold the cscope command
-	cmdlen = (int)(strlen(prog) + strlen(csinfo[i].fname) + 32);
+	cmdlen = (int)(proglen + strlen(csinfo[i].fname) + 32);
 	if (csinfo[i].ppath)
 	{
 	    // expand the prepend path for env var's
@@ -933,9 +933,7 @@ err_closing:
 		goto err_closing;
 #endif
 	    }
-	    expand_env((char_u *)csinfo[i].ppath, (char_u *)ppath, MAXPATHL);
-
-	    cmdlen += (int)strlen(ppath);
+	    cmdlen += (int)expand_env((char_u *)csinfo[i].ppath, (char_u *)ppath, MAXPATHL);
 	}
 
 	if (csinfo[i].flags)

src/mark.c

@@ -540,10 +540,9 @@ fname2fnum(xfmark_T *fm)
 #endif
 		))
     {
-	int len;
+	size_t len;
 
-	expand_env((char_u *)"~/", NameBuff, MAXPATHL);
-	len = (int)STRLEN(NameBuff);
+	len = expand_env((char_u *)"~/", NameBuff, MAXPATHL);
 	vim_strncpy(NameBuff + len, fm->fname + 2, MAXPATHL - len - 1);
     }
     else

src/misc1.c

@@ -1401,16 +1401,16 @@ expand_env_save_opt(char_u *src, int one)
  * Skips over "\ ", "\~" and "\$" (not for Win32 though).
  * If anything fails no expansion is done and dst equals src.
  */
-    void
+    size_t
 expand_env(
     char_u	*src,		// input string e.g. "$HOME/vim.hlp"
     char_u	*dst,		// where to put the result
     int		dstlen)		// maximum length of the result
 {
-    expand_env_esc(src, dst, dstlen, FALSE, FALSE, NULL);
+    return expand_env_esc(src, dst, dstlen, FALSE, FALSE, NULL);
 }
 
-    void
+    size_t
 expand_env_esc(
     char_u	*srcp,		// input string e.g. "$HOME/vim.hlp"
     char_u	*dst,		// where to put the result
@@ -1427,6 +1427,7 @@ expand_env_esc(
     int		mustfree;	// var was allocated, need to free it later
     int		at_start = TRUE; // at start of a name
     int		startstr_len = 0;
+    char_u	*dst_start = dst;
 
     if (startstr != NULL)
 	startstr_len = (int)STRLEN(startstr);
@@ -1577,6 +1578,7 @@ expand_env_esc(
 		 */
 		{
 		    char_u	test[MAXPATHL], paths[MAXPATHL];
+		    size_t	testlen;
 		    char_u	*path, *next_path, *ptr;
 		    stat_T	st;
 
@@ -1588,14 +1590,20 @@ expand_env_esc(
 				next_path++);
 			if (*next_path)
 			    *next_path++ = NUL;
-			STRCPY(test, path);
-			STRCAT(test, "/");
-			STRCAT(test, dst + 1);
+			testlen = vim_snprintf_safelen(
+			    (char *)test,
+			    sizeof(test),
+			    "%s/%s",
+			    path,
+			    dst + 1);
 			if (mch_stat(test, &st) == 0)
 			{
-			    var = alloc(STRLEN(test) + 1);
-			    STRCPY(var, test);
-			    mustfree = TRUE;
+			    var = alloc(testlen + 1);
+			    if (var != NULL)
+			    {
+				STRCPY(var, test);
+				mustfree = TRUE;
+			    }
 			    break;
 			}
 		    }
@@ -1641,23 +1649,26 @@ expand_env_esc(
 		}
 	    }
 
-	    if (var != NULL && *var != NUL
-		    && (STRLEN(var) + STRLEN(tail) + 1 < (unsigned)dstlen))
+	    if (var != NULL && *var != NUL)
 	    {
-		STRCPY(dst, var);
-		dstlen -= (int)STRLEN(var);
 		c = (int)STRLEN(var);
-		// if var[] ends in a path separator and tail[] starts
-		// with it, skip a character
-		if (after_pathsep(dst, dst + c)
+
+		if (c + STRLEN(tail) + 1 < (unsigned)dstlen)
+		{
+		    STRCPY(dst, var);
+		    dstlen -= c;
+		    // if var[] ends in a path separator and tail[] starts
+		    // with it, skip a character
+		    if (after_pathsep(dst, dst + c)
 #if defined(BACKSLASH_IN_FILENAME) || defined(AMIGA)
-			&& dst[c - 1] != ':'
+			    && dst[c - 1] != ':'
 #endif
-			&& vim_ispathsep(*tail))
-		    ++tail;
-		dst += c;
-		src = tail;
-		copy_char = FALSE;
+			    && vim_ispathsep(*tail))
+			++tail;
+		    dst += c;
+		    src = tail;
+		    copy_char = FALSE;
+		}
 	    }
 	    if (mustfree)
 		vim_free(var);
@@ -1692,6 +1703,8 @@ expand_env_esc(
 
     }
     *dst = NUL;
+
+    return (size_t)(dst - dst_start);
 }
 
 /*

src/proto/misc1.pro

@@ -29,8 +29,8 @@ void free_users(void);
 void init_vimdir(void);
 char_u *expand_env_save(char_u *src);
 char_u *expand_env_save_opt(char_u *src, int one);
-void expand_env(char_u *src, char_u *dst, int dstlen);
-void expand_env_esc(char_u *srcp, char_u *dst, int dstlen, int esc, int one, char_u *startstr);
+size_t expand_env(char_u *src, char_u *dst, int dstlen);
+size_t expand_env_esc(char_u *srcp, char_u *dst, int dstlen, int esc, int one, char_u *startstr);
 char_u *vim_getenv(char_u *name, int *mustfree);
 void vim_unsetenv(char_u *var);
 void vim_unsetenv_ext(char_u *var);

src/version.c

@@ -709,6 +709,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1467,
 /**/
     1466,
 /**/

src/viminfo.c

@@ -99,6 +99,8 @@ viminfo_filename(char_u *file)
 {
     if (file == NULL || *file == NUL)
     {
+	size_t  len;
+
 	if (*p_viminfofile != NUL)
 	    file = p_viminfofile;
 	else if ((file = find_viminfo_parameter('n')) == NULL || *file == NUL)
@@ -127,9 +129,12 @@ viminfo_filename(char_u *file)
 #endif
 		file = (char_u *)VIMINFO_FILE;
 	}
-	expand_env(file, NameBuff, MAXPATHL);
+	len = expand_env(file, NameBuff, MAXPATHL);
 	file = NameBuff;
+
+	return vim_strnsave(file, len);
     }
+
     return vim_strsave(file);
 }