vim-patch:9.1.1508: string manipulation can be improved in cmdexpand.c (#34755)

Problem:  String manipulation can be improved in cmdexpand.c
Solution: Refactor cmdexpand.c to remove calls to
          STRLEN()/STRMOVE()/STRCAT() (John Marriott)

This commit does the following:

In function nextwild():
  - slightly refactor the for loop to remove an array access
  - call STRLEN() and store it's result for reuse
  - move some variables closer to where they are used, renaming some on
    the way

In function ExpandOne():
  - move some calculations outside of the for loops
  - factor out calls to STRCAT() (which has an inherent STRLEN() call) in
    the for loop
  - move some variables closer to where they are used

In function expand_files_and_dirs():
  - factor out calls to STRMOVE() (which has an inherent STRLEN() call)

In function get_filetypecmd_arg():
  - move declarations of the string arrays into the blocks where they are
    used

In function get_breakadd_arg():
  - move declaration of the string array into the block where it is
    used

In function globpath():
  - factor out calls to STRLEN() and STRCAT()
  - move some variables closer to where they are used

And finally some minor cosmetic style changes

closes: vim/vim#17639

a494ce1c64

Co-authored-by: John Marriott <basilisk@internode.on.net>

src/nvim/cmdexpand.c

@@ -242,7 +242,7 @@ static void ExpandEscape(expand_T *xp, char *str, int numfiles, char **files, in
 int nextwild(expand_T *xp, int type, int options, bool escape)
 {
   CmdlineInfo *const ccline = get_cmdline_info();
-  char *p2;
+  char *p;
 
   if (xp->xp_numfiles == -1) {
     if (ccline->input_fn && ccline->xp_context == EXPAND_COMMANDS) {
@@ -281,14 +281,14 @@ int nextwild(expand_T *xp, int type, int options, bool escape)
       || type == WILD_PAGEUP || type == WILD_PAGEDOWN
       || type == WILD_PUM_WANT) {
     // Get next/previous match for a previous expanded pattern.
-    p2 = ExpandOne(xp, NULL, NULL, 0, type);
+    p = ExpandOne(xp, NULL, NULL, 0, type);
   } else {
-    char *p1;
+    char *tmp;
     if (cmdline_fuzzy_completion_supported(xp)) {
       // If fuzzy matching, don't modify the search string
-      p1 = xstrnsave(xp->xp_pattern, xp->xp_pattern_len);
+      tmp = xstrnsave(xp->xp_pattern, xp->xp_pattern_len);
     } else {
-      p1 = addstar(xp->xp_pattern, xp->xp_pattern_len, xp->xp_context);
+      tmp = addstar(xp->xp_pattern, xp->xp_pattern_len, xp->xp_context);
     }
     // Translate string into pattern and expand it.
     const int use_options = ((options & ~WILD_KEEP_SOLE_ITEM)
@@ -297,26 +297,27 @@ int nextwild(expand_T *xp, int type, int options, bool escape)
                              | WILD_SILENT
                              | (escape ? WILD_ESCAPE : 0)
                              | (p_wic ? WILD_ICASE : 0));
-    p2 = ExpandOne(xp, p1, xstrnsave(&ccline->cmdbuff[i], xp->xp_pattern_len),
+    p = ExpandOne(xp, tmp, xstrnsave(&ccline->cmdbuff[i], xp->xp_pattern_len),
                   use_options, type);
-    xfree(p1);
+    xfree(tmp);
     // Longest match: make sure it is not shorter, happens with :help.
-    if (p2 != NULL && type == WILD_LONGEST) {
+    if (p != NULL && type == WILD_LONGEST) {
       int j;
       for (j = 0; (size_t)j < xp->xp_pattern_len; j++) {
-        if (ccline->cmdbuff[i + j] == '*'
+        char c = ccline->cmdbuff[i + j];
-            || ccline->cmdbuff[i + j] == '?') {
+        if (c == '*' || c == '?') {
           break;
         }
       }
-      if ((int)strlen(p2) < j) {
+      if ((int)strlen(p) < j) {
-        XFREE_CLEAR(p2);
+        XFREE_CLEAR(p);
       }
     }
   }
 
-  if (p2 != NULL && !got_int) {
+  if (p != NULL && !got_int) {
-    int difflen = (int)strlen(p2) - (int)(xp->xp_pattern_len);
+    size_t plen = strlen(p);
+    int difflen = (int)plen - (int)(xp->xp_pattern_len);
     if (ccline->cmdlen + difflen + 4 > ccline->cmdbufflen) {
       realloc_cmdbuff(ccline->cmdlen + difflen + 4);
       xp->xp_pattern = ccline->cmdbuff + i;
@@ -325,7 +326,7 @@ int nextwild(expand_T *xp, int type, int options, bool escape)
     memmove(&ccline->cmdbuff[ccline->cmdpos + difflen],
             &ccline->cmdbuff[ccline->cmdpos],
             (size_t)ccline->cmdlen - (size_t)ccline->cmdpos + 1);
-    memmove(&ccline->cmdbuff[i], p2, strlen(p2));
+    memmove(&ccline->cmdbuff[i], p, plen);
     ccline->cmdlen += difflen;
     ccline->cmdpos += difflen;
   }
@@ -335,18 +336,18 @@ int nextwild(expand_T *xp, int type, int options, bool escape)
 
   // When expanding a ":map" command and no matches are found, assume that
   // the key is supposed to be inserted literally
-  if (xp->xp_context == EXPAND_MAPPINGS && p2 == NULL) {
+  if (xp->xp_context == EXPAND_MAPPINGS && p == NULL) {
     return FAIL;
   }
 
-  if (xp->xp_numfiles <= 0 && p2 == NULL) {
+  if (xp->xp_numfiles <= 0 && p == NULL) {
     beep_flush();
   } else if (xp->xp_numfiles == 1 && !(options & WILD_KEEP_SOLE_ITEM)) {
     // free expanded pattern
     ExpandOne(xp, NULL, NULL, 0, WILD_FREE);
   }
 
-  xfree(p2);
+  xfree(p);
 
   return OK;
 }
@@ -922,33 +923,36 @@ char *ExpandOne(expand_T *xp, char *str, char *orig, int options, int mode)
   // Concatenate all matching names.  Unless interrupted, this can be slow
   // and the result probably won't be used.
   if (mode == WILD_ALL && xp->xp_numfiles > 0 && !got_int) {
-    size_t len = 0;
+    size_t ss_size = 0;
-    for (int i = 0; i < xp->xp_numfiles; i++) {
+    char *prefix = "";
-      if (i > 0) {
+    char *suffix = (options & WILD_USE_NL) ? "\n" : " ";
+    const int n = xp->xp_numfiles - 1;
+
     if (xp->xp_prefix == XP_PREFIX_NO) {
-          len += 2;   // prefix "no"
+      prefix = "no";
+      ss_size = STRLEN_LITERAL("no") * (size_t)n;
     } else if (xp->xp_prefix == XP_PREFIX_INV) {
-          len += 3;   // prefix "inv"
+      prefix = "inv";
+      ss_size = STRLEN_LITERAL("inv") * (size_t)n;
     }
+
+    for (int i = 0; i < xp->xp_numfiles; i++) {
+      ss_size += strlen(xp->xp_files[i]) + 1;  // +1 for the suffix
     }
-      len += strlen(xp->xp_files[i]) + 1;
+    ss_size++;  // +1 for the NUL
-    }
+
-    ss = xmalloc(len);
+    ss = xmalloc(ss_size);
     *ss = NUL;
     char *ssp = ss;
     for (int i = 0; i < xp->xp_numfiles; i++) {
       if (i > 0) {
-        if (xp->xp_prefix == XP_PREFIX_NO) {
+        ssp = xstpcpy(ssp, prefix);
-          ssp = xstpcpy(ssp, "no");
-        } else if (xp->xp_prefix == XP_PREFIX_INV) {
-          ssp = xstpcpy(ssp, "inv");
-        }
       }
       ssp = xstpcpy(ssp, xp->xp_files[i]);
-
+      if (i < n) {
-      if (i != xp->xp_numfiles - 1) {
+        ssp = xstpcpy(ssp, suffix);
-        ssp = xstpcpy(ssp, (options & WILD_USE_NL) ? "\n" : " ");
       }
+      assert(ssp < ss + ss_size);
     }
   }
 
@@ -2554,25 +2558,37 @@ static int expand_files_and_dirs(expand_T *xp, char *pat, char ***matches, int *
   // for ":set path=" and ":set tags=" halve backslashes for escaped space
   if (xp->xp_backslash != XP_BS_NONE) {
     free_pat = true;
-    pat = xstrdup(pat);
+    size_t pat_len = strlen(pat);
-    for (int i = 0; pat[i]; i++) {
+    pat = xstrnsave(pat, pat_len);
-      if (pat[i] == '\\') {
+
+    char *pat_end = pat + pat_len;
+    for (char *p = pat; *p != NUL; p++) {
+      if (*p != '\\') {
+        continue;
+      }
+
       if (xp->xp_backslash & XP_BS_THREE
-            && pat[i + 1] == '\\'
+          && *(p + 1) == '\\'
-            && pat[i + 2] == '\\'
+          && *(p + 2) == '\\'
-            && pat[i + 3] == ' ') {
+          && *(p + 3) == ' ') {
-          STRMOVE(pat + i, pat + i + 3);
+        char *from = p + 3;
+        memmove(p, from, (size_t)(pat_end - from) + 1);  // +1 for NUL
+        pat_end -= 3;
       } else if (xp->xp_backslash & XP_BS_ONE
-                   && pat[i + 1] == ' ') {
+                 && *(p + 1) == ' ') {
-          STRMOVE(pat + i, pat + i + 1);
+        char *from = p + 1;
-        } else if ((xp->xp_backslash & XP_BS_COMMA)
+        memmove(p, from, (size_t)(pat_end - from) + 1);  // +1 for NUL
-                   && pat[i + 1] == '\\'
+        pat_end--;
-                   && pat[i + 2] == ',') {
+      } else if (xp->xp_backslash & XP_BS_COMMA) {
-          STRMOVE(pat + i, pat + i + 2);
+        if (*(p + 1) == '\\' && *(p + 2) == ',') {
+          char *from = p + 2;
+          memmove(p, from, (size_t)(pat_end - from) + 1);  // +1 for NUL
+          pat_end -= 2;
 #ifdef BACKSLASH_IN_FILENAME
-        } else if ((xp->xp_backslash & XP_BS_COMMA)
+        } else if (*(p + 1) == ',') {
-                   && pat[i + 1] == ',') {
+          char *from = p + 1;
-          STRMOVE(pat + i, pat + i + 1);
+          memmove(p, from, (size_t)(pat_end - from) + 1);  // +1 for NUL
+          pat_end--;
 #endif
         }
       }
@@ -2623,21 +2639,24 @@ static int expand_files_and_dirs(expand_T *xp, char *pat, char ***matches, int *
 /// ":filetype {plugin,indent}" command.
 static char *get_filetypecmd_arg(expand_T *xp FUNC_ATTR_UNUSED, int idx)
 {
-  char *opts_all[] = { "indent", "plugin", "on", "off" };
+  if (idx < 0) {
-  char *opts_plugin[] = { "plugin", "on", "off" };
+    return NULL;
-  char *opts_indent[] = { "indent", "on", "off" };
+  }
-  char *opts_onoff[] = { "on", "off" };
 
   if (filetype_expand_what == EXP_FILETYPECMD_ALL && idx < 4) {
+    char *opts_all[] = { "indent", "plugin", "on", "off" };
     return opts_all[idx];
   }
   if (filetype_expand_what == EXP_FILETYPECMD_PLUGIN && idx < 3) {
+    char *opts_plugin[] = { "plugin", "on", "off" };
     return opts_plugin[idx];
   }
   if (filetype_expand_what == EXP_FILETYPECMD_INDENT && idx < 3) {
+    char *opts_indent[] = { "indent", "on", "off" };
     return opts_indent[idx];
   }
   if (filetype_expand_what == EXP_FILETYPECMD_ONOFF && idx < 2) {
+    char *opts_onoff[] = { "on", "off" };
     return opts_onoff[idx];
   }
   return NULL;
@@ -2648,9 +2667,9 @@ static char *get_filetypecmd_arg(expand_T *xp FUNC_ATTR_UNUSED, int idx)
 /// ":breakdel {func, file, here}" command.
 static char *get_breakadd_arg(expand_T *xp FUNC_ATTR_UNUSED, int idx)
 {
+  if (idx >= 0 && idx <= 3) {
     char *opts[] = { "expr", "file", "func", "here" };
 
-  if (idx >= 0 && idx <= 3) {
     // breakadd {expr, file, func, here}
     if (breakpt_expand_what == EXP_BREAKPT_ADD) {
       return opts[idx];
@@ -3368,19 +3387,24 @@ static int ExpandUserLua(expand_T *xp, int *num_file, char ***file)
 void globpath(char *path, char *file, garray_T *ga, int expand_options, bool dirs)
   FUNC_ATTR_NONNULL_ALL
 {
+  char *buf = xmalloc(MAXPATHL);
+
   expand_T xpc;
   ExpandInit(&xpc);
   xpc.xp_context = dirs ? EXPAND_DIRECTORIES : EXPAND_FILES;
 
-  char *buf = xmalloc(MAXPATHL);
+  size_t filelen = strlen(file);
 
   // Loop over all entries in {path}.
   while (*path != NUL) {
     // Copy one item of the path to buf[] and concatenate the file name.
-    copy_option_part(&path, buf, MAXPATHL, ",");
+    size_t buflen = copy_option_part(&path, buf, MAXPATHL, ",");
-    if (strlen(buf) + strlen(file) + 2 < MAXPATHL) {
+    if (buflen + filelen + 2 < MAXPATHL) {
-      add_pathsep(buf);
+      if (*buf != NUL && !after_pathsep(buf, buf + buflen)) {
-      strcat(buf, file);
+        STRCPY(buf + buflen, PATHSEPSTR);
+        buflen += STRLEN_LITERAL(PATHSEPSTR);
+      }
+      STRCPY(buf + buflen, file);
 
       char **p;
       int num_p = 0;