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>

diff --git a/src/fileio.c b/src/fileio.c
index 2c6c40ed3d9c9dcd26836d71afd03cd5c2a7d5be..c87ae4d4695dd0d1a853bdba0de2fe7a9ba882b0 100644 (file)
--- a/src/fileio.c
+++ b/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))
                {

diff --git a/src/findfile.c b/src/findfile.c
index 49def2fe017660c77d81fe4bf06db0146fb39a69..0f5f2dc624b47f2c82bfbf53ecafeb082b6b62b3 100644 (file)
--- a/src/findfile.c
+++ b/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
        {

diff --git a/src/if_cscope.c b/src/if_cscope.c
index 6b0f920923b58b25d73fcb7718200715ff83a254..86bfbd81b8208fd36464dfee4aad6338c4413d70 100644 (file)
--- a/src/if_cscope.c
+++ b/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)

diff --git a/src/mark.c b/src/mark.c
index 2b0391981db64de04626fb59a6f1a8e862d4129d..9bab352a8f964489325a1ff72532beb795e88f15 100644 (file)
--- a/src/mark.c
+++ b/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

diff --git a/src/misc1.c b/src/misc1.c
index 142a6161ea6c8a3301356ce59c802a65e5eaa904..4571ff27f0c072bee1a292cf10be7fcaca015de0 100644 (file)
--- a/src/misc1.c
+++ b/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);
 }
 
 /*

diff --git a/src/proto/misc1.pro b/src/proto/misc1.pro
index 1a053e3dbcab0f81b8909a5d6b4d358c350345a8..d7c187af61050c3616476e65828cf5b5637431b4 100644 (file)
--- a/src/proto/misc1.pro
+++ b/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);

diff --git a/src/version.c b/src/version.c
index 0b6a3b028e4df74d18aa108c1d17d62d416079b9..168ff4e674473b9c3c3f36a02013c873762f9565 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -709,6 +709,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1467,
 /**/
     1466,
 /**/

diff --git a/src/viminfo.c b/src/viminfo.c
index 5e4caf81cd4f56f980283bfc1e54d920e243d402..489cb2db0dc569aa46f072424baa01df3f193763 100644 (file)
--- a/src/viminfo.c
+++ b/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);
 }