patch 9.2.0291: too many strlen() calls

Problem:  too many strlen() calls
Solution: refactor concat_fname() and remove calls to strlen()
          (John Marriott)

Function `concat_fnames()` can make up to 5 calls to `STRLEN()` (either
directly or indirectly via `STRCAT()`). In many cases the lengths of
arguments `fname1` and/or `fname2` are either known or can simply be
calculated.

This Commit refactors this function to accept the lengths of arguments
`fname1` and `fname2` as arguments. It also adds new argument `ret` to
return the resulting string as a `string_T`.

Additionally:
- function `add_pack_dir_to_rtp()` in `scriptfile.c`:
   Use a `string_T` to store local variables `new_rtp` and `afterdir`.
   Replace calls to `STRCAT()` with calls to `STRCPY()`.
   Change type of variable `keep` to `size_t` for consistency with
   other lengths.

- function `qf_get_fnum()` in `quickfix.c`:
  Use a `string_T` to store local variables `ptr` and `bufname`
- function `qf_push_dir()` in `quickfix.c`:
  Use a `string_T` to store local variable `dirname`.
  Replace call to `vim_strsave()` with `vim_strnsave()`.

- function `qf_guess_filepath()` in `quickfix.c`:
  Use a `string_T` to store local variable `fullname`.

- function `make_percent_swname()` in `memline.c`:
  Rename some variables to better reflect their use.
  Use a `string_T` to store local variables `d` and `fixed_name`.
  Slightly refactor to remove need to create an extra string.
- function `get_file_in_dir()` in `memline.c`:
  Use a `string_T` to store local variables `tail` and `retval`.
  Move some variables closer to where they are used.

- function `cs_resolve_file()` in `if_cscope.c`:
  Use a `string_T` to store local variable `csdir`.
  Remove one call to `STRLEN()`.

- function `add_pathsep()` in `filepath.c`:
  Refactor and remove 1 call to `STRLEN()`

- function `set_init_xdg_rtp()` in `option.c`:
  Use a `string_T` to store local variable `vimrc_xdg`.

closes: #19854

Co-authored-by: Christian Brabandt <[email protected]>
Signed-off-by: John Marriott <[email protected]>
Signed-off-by: Christian Brabandt <[email protected]>

Author: John Marriott

src/filepath.c

@@ -3178,19 +3178,25 @@ vim_fnamencmp(char_u *x, char_u *y, size_t len)
  * Only add a '/' or '\\' when 'sep' is TRUE and it is necessary.
  */
     char_u  *
-concat_fnames(char_u *fname1, char_u *fname2, int sep)
+concat_fnames(char_u *fname1, size_t fname1len, char_u *fname2, size_t fname2len, int sep, string_T *ret)
 {
-    char_u  *dest;
-
-    dest = alloc(STRLEN(fname1) + STRLEN(fname2) + 3);
-    if (dest == NULL)
-	return NULL;
+    ret->string = alloc(fname1len + (sep ? STRLEN_LITERAL(PATHSEPSTR) : 0) + fname2len + 1);
+    if (ret->string == NULL)
+	ret->length = 0;
+    else
+    {
+	STRCPY(ret->string, fname1);
+	ret->length = fname1len;
+	if (sep && *ret->string != NUL && !after_pathsep(ret->string, ret->string + ret->length))
+	{
+	    STRCPY(ret->string + ret->length, PATHSEPSTR);
+	    ret->length += STRLEN_LITERAL(PATHSEPSTR);
+	}
+	STRCPY(ret->string + ret->length, fname2);
+	ret->length += fname2len;
+    }
 
-    STRCPY(dest, fname1);
-    if (sep)
-	add_pathsep(dest);
-    STRCAT(dest, fname2);
-    return dest;
+    return ret->string;
 }
 
 /*
@@ -3200,8 +3206,14 @@ concat_fnames(char_u *fname1, char_u *fname2, int sep)
     void
 add_pathsep(char_u *p)
 {
-    if (*p != NUL && !after_pathsep(p, p + STRLEN(p)))
-	STRCAT(p, PATHSEPSTR);
+    size_t  plen;
+
+    if (*p == NUL)
+	return;
+
+    plen = STRLEN(p);
+    if (!after_pathsep(p, p + plen))
+	STRCPY(p + plen, PATHSEPSTR);
 }
 
 /*

src/if_cscope.c

@@ -2413,36 +2413,43 @@ cs_reset(exarg_T *eap UNUSED)
 cs_resolve_file(int i, char *name)
 {
     char	*fullname;
+    string_T	csdir = {NULL, 0};
+    size_t	namelen;
+    size_t	ppathlen = 0;
     int		len;
-    char_u	*csdir = NULL;
 
     /*
      * Ppath is freed when we destroy the cscope connection.
      * Fullname is freed after cs_make_vim_style_matches, after it's been
      * copied into the tag buffer used by Vim.
      */
-    len = (int)(strlen(name) + 2);
+    namelen = STRLEN(name);
+    len = (int)namelen + 2;
     if (csinfo[i].ppath != NULL)
-	len += (int)strlen(csinfo[i].ppath);
+    {
+	ppathlen = STRLEN(csinfo[i].ppath);
+	len += (int)ppathlen;
+    }
     else if (p_csre && csinfo[i].fname != NULL)
     {
 	// If 'cscoperelative' is set and ppath is not set, use cscope.out
 	// path in path resolution.
-	csdir = alloc(MAXPATHL);
-	if (csdir != NULL)
+	csdir.string = alloc(MAXPATHL);
+	if (csdir.string != NULL)
 	{
-	    vim_strncpy(csdir, (char_u *)csinfo[i].fname,
+	    vim_strncpy(csdir.string, (char_u *)csinfo[i].fname,
 					  gettail((char_u *)csinfo[i].fname)
 						 - (char_u *)csinfo[i].fname);
-	    len += (int)STRLEN(csdir);
+	    csdir.length = STRLEN(csdir.string);
+	    len += (int)csdir.length;
 	}
     }
 
     // Note/example: this won't work if the cscope output already starts
     // "../.." and the prefix path is also "../..".  if something like this
     // happens, you are screwed up and need to fix how you're using cscope.
     if (csinfo[i].ppath != NULL
-	    && (strncmp(name, csinfo[i].ppath, strlen(csinfo[i].ppath)) != 0)
+	    && (strncmp(name, csinfo[i].ppath, ppathlen) != 0)
 	    && (name[0] != '/')
 # ifdef MSWIN
 	    && name[0] != '\\' && name[1] != ':'
@@ -2452,18 +2459,20 @@ cs_resolve_file(int i, char *name)
 	if ((fullname = alloc(len)) != NULL)
 	    (void)sprintf(fullname, "%s/%s", csinfo[i].ppath, name);
     }
-    else if (csdir != NULL && csinfo[i].fname != NULL && *csdir != NUL)
+    else if (csdir.string != NULL && csinfo[i].fname != NULL && *csdir.string != NUL)
     {
+	string_T    ret;
+
 	// Check for csdir to be non empty to avoid empty path concatenated to
 	// cscope output.
-	fullname = (char *)concat_fnames(csdir, (char_u *)name, TRUE);
+	fullname = (char *)concat_fnames(csdir.string, csdir.length, (char_u *)name, namelen, TRUE, &ret);
     }
     else
     {
-	fullname = (char *)vim_strsave((char_u *)name);
+	fullname = (char *)vim_strnsave((char_u *)name, namelen);
     }
 
-    vim_free(csdir);
+    vim_free(csdir.string);
     return fullname;
 }
 

src/main.c

@@ -2771,13 +2771,15 @@ command_line_scan(mparm_T *parmp)
 	    if (parmp->diff_mode && mch_isdir(p) && GARGCOUNT > 0
 				      && !mch_isdir(alist_name(&GARGLIST[0])))
 	    {
-		char_u	    *r;
+		char_u	    *tail;
+		string_T    ret;
 
-		r = concat_fnames(p, gettail(alist_name(&GARGLIST[0])), TRUE);
-		if (r != NULL)
+		tail = gettail(alist_name(&GARGLIST[0]));
+		concat_fnames(p, STRLEN(p), tail, STRLEN(tail), TRUE, &ret);
+		if (ret.string != NULL)
 		{
 		    vim_free(p);
-		    p = r;
+		    p = ret.string;
 		}
 	    }
 # endif

src/memline.c

@@ -1892,7 +1892,6 @@ recover_names(
 {
     int		num_names;
     char_u	*(names[6]);
-    char_u	*tail;
     char_u	*p;
     int		num_files;
     int		file_count = 0;
@@ -1969,20 +1968,27 @@ recover_names(
 	{
 	    if (fname == NULL)
 	    {
+		string_T    ret;
+
 #ifdef VMS
-		names[0] = concat_fnames(dir_name.string, (char_u *)"*_sw%", TRUE);
+		names[0] = concat_fnames(dir_name.string, dir_name.length,
+		    (char_u *)"*_sw%", STRLEN_LITERAL("*_sw%"), TRUE, &ret);
 #else
-		names[0] = concat_fnames(dir_name.string, (char_u *)"*.sw?", TRUE);
+		names[0] = concat_fnames(dir_name.string, dir_name.length,
+		    (char_u *)"*.sw?", STRLEN_LITERAL("*.sw?"), TRUE, &ret);
 #endif
 #if defined(UNIX) || defined(MSWIN)
 		// For Unix names starting with a dot are special.  MS-Windows
 		// supports this too, on some file systems.
-		names[1] = concat_fnames(dir_name.string, (char_u *)".*.sw?", TRUE);
-		names[2] = concat_fnames(dir_name.string, (char_u *)".sw?", TRUE);
+		names[1] = concat_fnames(dir_name.string, dir_name.length,
+		    (char_u *)".*.sw?", STRLEN_LITERAL(".*.sw?"), TRUE, &ret);
+		names[2] = concat_fnames(dir_name.string, dir_name.length,
+		    (char_u *)".sw?", STRLEN_LITERAL(".sw?"), TRUE, &ret);
 		num_names = 3;
 #else
 # ifdef VMS
-		names[1] = concat_fnames(dir_name.string, (char_u *)".*_sw%", TRUE);
+		names[1] = concat_fnames(dir_name.string, dir_name.length,
+		    (char_u *)".*_sw%", STRLEN_LITERAL(".*_sw%"), TRUE, &ret);
 		num_names = 2;
 # else
 		num_names = 1;
@@ -1991,6 +1997,8 @@ recover_names(
 	    }
 	    else
 	    {
+		char_u	*tail;
+
 #if defined(UNIX) || defined(MSWIN)
 		p = dir_name.string + dir_name.length;
 		if (after_pathsep(dir_name.string, p) && dir_name.length > 1 && p[-1] == p[-2])
@@ -2001,8 +2009,11 @@ recover_names(
 		else
 #endif
 		{
+		    string_T	ret;
+
 		    tail = gettail(fname_res);
-		    tail = concat_fnames(dir_name.string, tail, TRUE);
+		    tail = concat_fnames(dir_name.string, dir_name.length,
+			tail, STRLEN(tail), TRUE, &ret);
 		}
 		if (tail == NULL)
 		    num_names = 0;
@@ -2132,13 +2143,16 @@ recover_names(
 #ifdef FEAT_EVAL
 	else if (ret_list != NULL)
 	{
+	    string_T	name;
+
 	    for (int i = 0; i < num_files; ++i)
 	    {
-		char_u *name = concat_fnames(dir_name.string, files[i], TRUE);
-		if (name != NULL)
+		concat_fnames(dir_name.string, dir_name.length,
+		    files[i], STRLEN(files[i]), TRUE, &name);
+		if (name.string != NULL)
 		{
-		    list_append_string(ret_list, name, -1);
-		    vim_free(name);
+		    list_append_string(ret_list, name.string, (int)name.length);
+		    vim_free(name.string);
 		}
 	    }
 	}
@@ -2166,26 +2180,26 @@ recover_names(
     char_u *
 make_percent_swname(char_u *dir, char_u *dir_end, char_u *name)
 {
-    char_u *d = NULL, *s, *f;
+    string_T	d = {NULL, 0};
+    string_T	fixed_fname;
+    char_u	*p;
 
-    f = fix_fname(name != NULL ? name : (char_u *)"");
-    if (f == NULL)
+    fixed_fname.string = fix_fname(name != NULL ? name : (char_u *)"");
+    if (fixed_fname.string == NULL)
 	return NULL;
 
-    s = alloc(STRLEN(f) + 1);
-    if (s != NULL)
-    {
-	STRCPY(s, f);
-	for (d = s; *d != NUL; MB_PTR_ADV(d))
-	    if (vim_ispathsep(*d))
-		*d = '%';
-
-	dir_end[-1] = NUL;  // remove one trailing slash
-	d = concat_fnames(dir, s, TRUE);
-	vim_free(s);
-    }
-    vim_free(f);
-    return d;
+    for (p = fixed_fname.string; *p != NUL; MB_PTR_ADV(p))
+	if (vim_ispathsep(*p))
+	    *p = '%';
+    fixed_fname.length = (size_t)(p - fixed_fname.string);
+
+    // remove one trailing slash
+    p = &dir_end[-1];
+    *p = NUL;
+    concat_fnames(dir, (size_t)(p - dir), fixed_fname.string, fixed_fname.length, TRUE, &d);
+    vim_free(fixed_fname.string);
+
+    return d.string;
 }
 #endif
 
@@ -2432,6 +2446,7 @@ recov_file_names(char_u **names, char_u *path, int prepend_dot)
 
     curbuf->b_shortname = FALSE;
 #endif
+    string_T	ret;
 
     num_names = 0;
 
@@ -2451,9 +2466,11 @@ recov_file_names(char_u **names, char_u *path, int prepend_dot)
      * Form the normal swap file name pattern by appending ".sw?".
      */
 #ifdef VMS
-    names[num_names] = concat_fnames(path, (char_u *)"_sw%", FALSE);
+    names[num_names] = concat_fnames(path, STRLEN(path),
+	(char_u *)"_sw%", STRLEN_LITERAL("_sw%"), FALSE, &ret);
 #else
-    names[num_names] = concat_fnames(path, (char_u *)".sw?", FALSE);
+    names[num_names] = concat_fnames(path, STRLEN(path),
+	(char_u *)".sw?", STRLEN_LITERAL(".sw?"), FALSE, &ret);
 #endif
     if (names[num_names] == NULL)
 	goto end;
@@ -4746,45 +4763,61 @@ get_file_in_dir(
     char_u  *fname,
     char_u  *dname)	// don't use "dirname", it is a global for Alpha
 {
-    char_u	*t;
-    char_u	*tail;
-    char_u	*retval;
-    int		save_char;
+    string_T	tail;
+    string_T	retval;
 
-    tail = gettail(fname);
+    tail.string = gettail(fname);
+    tail.length = STRLEN(tail.string);
 
     if (dname[0] == '.' && dname[1] == NUL)
-	retval = vim_strsave(fname);
-    else if (dname[0] == '.' && vim_ispathsep(dname[1]))
+	retval.string =
+	    vim_strnsave(fname, (size_t)(tail.string - fname) + tail.length);
+    else
     {
-	if (tail == fname)	    // no path before file name
-	    retval = concat_fnames(dname + 2, tail, TRUE);
-	else
+	size_t	dname_len = STRLEN(dname);
+
+	if (dname[0] == '.' && vim_ispathsep(dname[1]))
 	{
-	    save_char = *tail;
-	    *tail = NUL;
-	    t = concat_fnames(fname, dname + 2, TRUE);
-	    *tail = save_char;
-	    if (t == NULL)	    // out of memory
-		retval = NULL;
+	    if (tail.string == fname)	    // no path before file name
+		concat_fnames(dname + 2, dname_len - 2,
+		    tail.string, tail.length, TRUE, &retval);
 	    else
 	    {
-		retval = concat_fnames(t, tail, TRUE);
-		vim_free(t);
+		int	    save_char;
+		string_T    tmp;
+
+		save_char = *tail.string;
+		*tail.string = NUL;
+		concat_fnames(fname, (size_t)(tail.string - fname),
+		    dname + 2, dname_len - 2, TRUE, &tmp);
+		*tail.string = save_char;
+		if (tmp.string == NULL)	    // out of memory
+		    retval.string = NULL;
+		else
+		{
+		    concat_fnames(tmp.string, tmp.length,
+			tail.string, tail.length, TRUE, &retval);
+		    vim_free(tmp.string);
+		}
 	    }
 	}
+	else
+	    concat_fnames(dname, dname_len, tail.string, tail.length,
+		TRUE, &retval);
     }
-    else
-	retval = concat_fnames(dname, tail, TRUE);
 
 #ifdef MSWIN
-    if (retval != NULL)
-	for (t = gettail(retval); *t != NUL; MB_PTR_ADV(t))
+    if (retval.string != NULL)
+    {
+	char_u	*t;
+
+	for (t = gettail(retval.string); *t != NUL; MB_PTR_ADV(t))
 	    if (*t == ':')
 		*t = '%';
+    }
 #endif
 
-    return retval;
+    return retval.string;
 }
 
 /*