refactor(api/nvim_cmd): use `kvec_t` for constructing cmdline string

Co-authored-by: Björn Linse <bjorn.linse@gmail.com>

5 files changed, 111 insertions, 109 deletions

diff --git a/src/nvim/api/private/helpers.c b/src/nvim/api/private/helpers.c
index 542e5c4953..dcede27bcb 100644
--- a/src/nvim/api/private/helpers.c
+++ b/src/nvim/api/private/helpers.c
@@ -1707,144 +1707,113 @@ bool string_iswhite(String str)
   return true;
 }
 
-// Add modifier string for command into the command line. Includes trailing whitespace if non-empty.
-// @return OK or FAIL.
-static int add_cmd_modifier_str(char *cmdline, size_t *pos, const size_t bufsize,
-                                CmdParseInfo *cmdinfo)
+/// Build cmdline string for command, used by `nvim_cmd()`.
+///
+/// @return OK or FAIL.
+void build_cmdline_str(char **cmdlinep, exarg_T *eap, CmdParseInfo *cmdinfo, char **args,
+                       size_t argc)
 {
-#define APPEND_MODIFIER(...) \
-  do { \
-    if (*pos < bufsize) { \
-      *pos += (size_t)snprintf(cmdline + *pos, bufsize - *pos, __VA_ARGS__); \
-    } \
-    if (*pos < bufsize) { \
-      cmdline[*pos] = ' '; \
-      *pos += 1; \
-    } else { \
-      goto err; \
-    } \
-  } while (0)
-
-#define APPEND_MODIFIER_IF(cond, mod) \
-  do { \
-    if (cond) { \
-      APPEND_MODIFIER(mod); \
-    } \
-  } while (0)
+  StringBuilder cmdline = KV_INITIAL_VALUE;
 
+  // Add command modifiers
   if (cmdinfo->cmdmod.tab != 0) {
-    APPEND_MODIFIER("%dtab", cmdinfo->cmdmod.tab - 1);
+    kv_printf(cmdline, "%dtab ", cmdinfo->cmdmod.tab - 1);
   }
   if (cmdinfo->verbose != -1) {
-    APPEND_MODIFIER("%ldverbose", cmdinfo->verbose);
+    kv_printf(cmdline, "%ldverbose ", cmdinfo->verbose);
+  }
+
+  if (cmdinfo->emsg_silent) {
+    kv_concat(cmdline, "silent! ");
+  } else if (cmdinfo->silent) {
+    kv_concat(cmdline, "silent ");
   }
 
   switch (cmdinfo->cmdmod.split & (WSP_ABOVE | WSP_BELOW | WSP_TOP | WSP_BOT)) {
   case WSP_ABOVE:
-    APPEND_MODIFIER("aboveleft");
+    kv_concat(cmdline, "aboveleft ");
     break;
   case WSP_BELOW:
-    APPEND_MODIFIER("belowright");
+    kv_concat(cmdline, "belowright ");
     break;
   case WSP_TOP:
-    APPEND_MODIFIER("topleft");
+    kv_concat(cmdline, "topleft ");
     break;
   case WSP_BOT:
-    APPEND_MODIFIER("botright");
+    kv_concat(cmdline, "botright ");
     break;
   default:
     break;
   }
 
-  APPEND_MODIFIER_IF(cmdinfo->cmdmod.split & WSP_VERT, "vertical");
-
-  if (cmdinfo->emsg_silent) {
-    APPEND_MODIFIER("silent!");
-  } else if (cmdinfo->silent) {
-    APPEND_MODIFIER("silent");
-  }
-
-  APPEND_MODIFIER_IF(cmdinfo->sandbox, "sandbox");
-  APPEND_MODIFIER_IF(cmdinfo->noautocmd, "noautocmd");
-  APPEND_MODIFIER_IF(cmdinfo->cmdmod.browse, "browse");
-  APPEND_MODIFIER_IF(cmdinfo->cmdmod.confirm, "confirm");
-  APPEND_MODIFIER_IF(cmdinfo->cmdmod.hide, "hide");
-  APPEND_MODIFIER_IF(cmdinfo->cmdmod.keepalt, "keepalt");
-  APPEND_MODIFIER_IF(cmdinfo->cmdmod.keepjumps, "keepjumps");
-  APPEND_MODIFIER_IF(cmdinfo->cmdmod.keepmarks, "keepmarks");
-  APPEND_MODIFIER_IF(cmdinfo->cmdmod.keeppatterns, "keeppatterns");
-  APPEND_MODIFIER_IF(cmdinfo->cmdmod.lockmarks, "lockmarks");
-  APPEND_MODIFIER_IF(cmdinfo->cmdmod.noswapfile, "noswapfile");
-
-  return OK;
-err:
-  return FAIL;
-
-#undef APPEND_MODIFIER
-#undef APPEND_MODIFIER_IF
-}
-
-/// Build cmdline string for command, used by `nvim_cmd()`.
-///
-/// @return OK or FAIL.
-int build_cmdline_str(char **cmdlinep, exarg_T *eap, CmdParseInfo *cmdinfo, char **args,
-                      size_t argc)
-{
-  const size_t bufsize = IOSIZE;
-  size_t pos = 0;
-  char *cmdline = xcalloc(bufsize, sizeof(char));
-
-#define CMDLINE_APPEND(...) \
+#define CMDLINE_APPEND_IF(cond, str) \
   do { \
-    if (pos < bufsize) { \
-      pos += (size_t)snprintf(cmdline + pos, bufsize - pos, __VA_ARGS__); \
-    } else { \
-      goto err; \
+    if (cond) { \
+      kv_concat(cmdline, str); \
     } \
   } while (0)
 
-  // Command modifiers.
-  if (add_cmd_modifier_str(cmdline, &pos, bufsize, cmdinfo) == FAIL) {
-    goto err;
-  }
+  CMDLINE_APPEND_IF(cmdinfo->cmdmod.split & WSP_VERT, "vertical ");
+  CMDLINE_APPEND_IF(cmdinfo->sandbox, "sandbox ");
+  CMDLINE_APPEND_IF(cmdinfo->noautocmd, "noautocmd ");
+  CMDLINE_APPEND_IF(cmdinfo->cmdmod.browse, "browse ");
+  CMDLINE_APPEND_IF(cmdinfo->cmdmod.confirm, "confirm ");
+  CMDLINE_APPEND_IF(cmdinfo->cmdmod.hide, "hide ");
+  CMDLINE_APPEND_IF(cmdinfo->cmdmod.keepalt, "keepalt ");
+  CMDLINE_APPEND_IF(cmdinfo->cmdmod.keepjumps, "keepjumps ");
+  CMDLINE_APPEND_IF(cmdinfo->cmdmod.keepmarks, "keepmarks ");
+  CMDLINE_APPEND_IF(cmdinfo->cmdmod.keeppatterns, "keeppatterns ");
+  CMDLINE_APPEND_IF(cmdinfo->cmdmod.lockmarks, "lockmarks ");
+  CMDLINE_APPEND_IF(cmdinfo->cmdmod.noswapfile, "noswapfile ");
+#undef CMDLINE_APPEND_IF
 
   // Command range / count.
   if (eap->argt & EX_RANGE) {
     if (eap->addr_count == 1) {
-      CMDLINE_APPEND("%ld", eap->line2);
+      kv_printf(cmdline, "%ld", eap->line2);
     } else if (eap->addr_count > 1) {
-      CMDLINE_APPEND("%ld,%ld", eap->line1, eap->line2);
+      kv_printf(cmdline, "%ld,%ld", eap->line1, eap->line2);
       eap->addr_count = 2;  // Make sure address count is not greater than 2
     }
   }
 
   // Keep the index of the position where command name starts, so eap->cmd can point to it.
-  size_t cmdname_idx = pos;
-  CMDLINE_APPEND("%s", eap->cmd);
-  eap->cmd = cmdline + cmdname_idx;
+  size_t cmdname_idx = cmdline.size;
+  kv_printf(cmdline, "%s", eap->cmd);
 
   // Command bang.
   if (eap->argt & EX_BANG && eap->forceit) {
-    CMDLINE_APPEND("!");
+    kv_printf(cmdline, "!");
   }
 
   // Command register.
   if (eap->argt & EX_REGSTR && eap->regname) {
-    CMDLINE_APPEND(" %c", eap->regname);
+    kv_printf(cmdline, " %c", eap->regname);
   }
 
-  // Iterate through each argument and store the starting position and length of each argument in
-  // the cmdline string in `eap->args` and `eap->arglens`, respectively.
-  eap->args = xcalloc(argc, sizeof(char *));
+  // Iterate through each argument and store the starting index and length of each argument
+  size_t *argidx = xcalloc(argc, sizeof(size_t));
+  eap->argc = argc;
   eap->arglens = xcalloc(argc, sizeof(size_t));
   for (size_t i = 0; i < argc; i++) {
-    eap->args[i] = cmdline + pos + 1;  // add 1 to skip the leading space.
+    argidx[i] = cmdline.size + 1;  // add 1 to account for the space.
     eap->arglens[i] = STRLEN(args[i]);
-    CMDLINE_APPEND(" %s", args[i]);
+    kv_printf(cmdline, " %s", args[i]);
   }
-  eap->argc = argc;
-  // If there isn't an argument, make eap->arg point to end of cmd
-  eap->arg = argc > 0 ? eap->args[0] : cmdline + pos;
+
+  // Now that all the arguments are appended, use the command index and argument indices to set the
+  // values of eap->cmd, eap->arg and eap->args.
+  eap->cmd = cmdline.items + cmdname_idx;
+  eap->args = xcalloc(argc, sizeof(char *));
+  for (size_t i = 0; i < argc; i++) {
+    eap->args[i] = cmdline.items + argidx[i];
+  }
+  // If there isn't an argument, make eap->arg point to end of cmdline.
+  eap->arg = argc > 0 ? eap->args[0] : cmdline.items + cmdline.size;
+
+  // Finally, make cmdlinep point to the cmdline string.
+  *cmdlinep = cmdline.items;
+  xfree(argidx);
 
   // Replace, :make and :grep with 'makeprg' and 'grepprg'.
   char *p = replace_makeprg(eap, eap->arg, cmdlinep);
@@ -1854,12 +1823,4 @@ int build_cmdline_str(char **cmdlinep, exarg_T *eap, CmdParseInfo *cmdinfo, char
     eap->arg = p;
     eap->args[0] = p;
   }
-
-  *cmdlinep = cmdline;
-  return OK;
-err:
-  xfree(cmdline);
-  return FAIL;
-
-#undef CMDLINE_APPEND
 }
diff --git a/src/nvim/api/vimscript.c b/src/nvim/api/vimscript.c
index 02f406d686..506b3e7f10 100644
--- a/src/nvim/api/vimscript.c
+++ b/src/nvim/api/vimscript.c
@@ -1290,9 +1290,7 @@ String nvim_cmd(uint64_t channel_id, Dict(cmd) *cmd, Dict(cmd_opts) *opts, Error
 
   // Finally, build the command line string that will be stored inside ea.cmdlinep.
   // This also sets the values of ea.cmd, ea.arg, ea.args and ea.arglens.
-  if (build_cmdline_str(&cmdline, &ea, &cmdinfo, args, argc) == FAIL) {
-    goto end;
-  }
+  build_cmdline_str(&cmdline, &ea, &cmdinfo, args, argc);
   ea.cmdlinep = &cmdline;
 
   garray_T capture_local;
diff --git a/src/nvim/lib/kvec.h b/src/nvim/lib/kvec.h
index 34332ba34b..68841e0af8 100644
--- a/src/nvim/lib/kvec.h
+++ b/src/nvim/lib/kvec.h
@@ -94,17 +94,26 @@
     memcpy((v1).items, (v0).items, sizeof((v1).items[0]) * (v0).size); \
   } while (0)
 
-#define kv_splice(v1, v0) \
+/// fit at least "len" more items
+#define kv_ensure_space(v, len) \
   do { \
-    if ((v1).capacity < (v1).size + (v0).size) { \
-      (v1).capacity = (v1).size + (v0).size; \
-      kv_roundup32((v1).capacity); \
-      kv_resize((v1), (v1).capacity); \
+    if ((v).capacity < (v).size + len) { \
+      (v).capacity = (v).size + len; \
+      kv_roundup32((v).capacity); \
+      kv_resize((v), (v).capacity); \
     } \
-    memcpy((v1).items + (v1).size, (v0).items, sizeof((v1).items[0]) * (v0).size); \
-    (v1).size = (v1).size + (v0).size; \
   } while (0)
 
+#define kv_concat_len(v, data, len) \
+  do { \
+    kv_ensure_space(v, len); \
+    memcpy((v).items + (v).size, data, sizeof((v).items[0]) * len); \
+    (v).size = (v).size + len; \
+  } while (0)
+
+#define kv_concat(v, str) kv_concat_len(v, str, STRLEN(str))
+#define kv_splice(v1, v0) kv_concat_len(v1, (v0).items, (v0).size)
+
 #define kv_pushp(v) \
   ((((v).size == (v).capacity) ? (kv_resize_full(v), 0) : 0), \
    ((v).items + ((v).size++)))
@@ -123,6 +132,8 @@
          : 0UL)), \
      &(v).items[(i)]))
 
+#define kv_printf(v, ...) kv_do_printf(&(v), __VA_ARGS__)
+
 /// Type of a vector with a few first members allocated on stack
 ///
 /// Is compatible with #kv_A, #kv_pop, #kv_size, #kv_max, #kv_last.
diff --git a/src/nvim/strings.c b/src/nvim/strings.c
index 0307cf4696..de1c8b9c36 100644
--- a/src/nvim/strings.c
+++ b/src/nvim/strings.c
@@ -1480,3 +1480,32 @@ int vim_vsnprintf_typval(char *str, size_t str_m, const char *fmt, va_list ap, t
   // written to the buffer if it were large enough.
   return (int)str_l;
 }
+
+int kv_do_printf(StringBuilder *str, const char *fmt, ...)
+  FUNC_ATTR_PRINTF(2, 3)
+{
+  size_t remaining = str->capacity - str->size;
+
+  va_list ap;
+  va_start(ap, fmt);
+  int printed = vsnprintf(str->items ? str->items + str->size : NULL, remaining, fmt, ap);
+  va_end(ap);
+
+  if (printed < 0) {
+    return -1;
+  }
+
+  // printed string didn't fit, resize and try again
+  if ((size_t)printed >= remaining) {
+    kv_ensure_space(*str, (size_t)printed + 1);  // include space for NUL terminator at the end
+    va_start(ap, fmt);
+    printed = vsnprintf(str->items + str->size, str->capacity - str->size, fmt, ap);
+    va_end(ap);
+    if (printed < 0) {
+      return -1;
+    }
+  }
+
+  str->size += (size_t)printed;
+  return printed;
+}
diff --git a/src/nvim/strings.h b/src/nvim/strings.h
index 893b0ea269..0503cecc8a 100644
--- a/src/nvim/strings.h
+++ b/src/nvim/strings.h
@@ -7,6 +7,7 @@
 
 #include "nvim/eval/typval.h"
 #include "nvim/types.h"
+#include "nvim/lib/kvec.h"
 
 /// Append string to string and return pointer to the next byte
 ///
@@ -25,6 +26,8 @@ static inline char *strappend(char *const dst, const char *const src)
   return (char *)memmove(dst, src, src_len) + src_len;
 }
 
+typedef kvec_t(char) StringBuilder;
+
 #ifdef INCLUDE_GENERATED_DECLARATIONS
 # include "strings.h.generated.h"
 #endif