vim-patch:partial:9.2.0126: String handling can be improved (#38214)

Problem:  String handling can be improved
Solution: Pass string length where it is known to avoid strlen() calls,
          do a few minor refactors (John Marriott).

This commit changes some calls to function `set_vim_var_string()` to pass
the string length where it is known or can be easily calculated.

In addition:
In `evalvars.c`:
  * In function `set_reg_var()` turn variable `regname` into a C string
    because that is how it used.
  * Small cosmetics.
In `option.c`:
  * Slightly refactor function `apply_optionset_autocmd()` to move some
    variables closer to where they are used.
In `getchar.c`:
  * Slightly refactor function `do_key_input_pre()`:
    -> change call to `dict_add_string()` to `dict_add_string_len()` and
       pass it the length of `buf`.
    -> only call `get_vim_var_string()` once.
In `message.c`:
  * Use a `string_T` to store local variable `p`.
In `normal.c`:
  * Move some variables closer to where they are used.

closes: vim/vim#19618

727f6e2686

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

src/nvim/eval/vars.c

@@ -2081,9 +2081,9 @@ void set_vim_var_special(const VimVarIndex idx, const SpecialVarValue val)
 void set_vim_var_char(int c)
 {
   char buf[MB_MAXCHAR + 1];
-
+  int buflen = utf_char2bytes(c, buf);
-  buf[utf_char2bytes(c, buf)] = NUL;
+  buf[buflen] = NUL;
-  set_vim_var_string(VV_CHAR, buf, -1);
+  set_vim_var_string(VV_CHAR, buf, buflen);
 }
 
 /// Set string v: variable to the given string
@@ -2155,17 +2155,18 @@ void set_vim_var_partial(const VimVarIndex idx, partial_T *val)
 /// Set v:register if needed.
 void set_reg_var(int c)
 {
-  char regname;
+  char regname[2];
 
   if (c == 0 || c == ' ') {
-    regname = '"';
+    regname[0] = '"';
   } else {
-    regname = (char)c;
+    regname[0] = (char)c;
   }
+  regname[1] = NUL;
   // Avoid free/alloc when the value is already right.
   typval_T *tv = get_vim_var_tv(VV_REG);
   if (tv->vval.v_string == NULL || tv->vval.v_string[0] != c) {
-    set_vim_var_string(VV_REG, &regname, 1);
+    set_vim_var_string(VV_REG, regname, 1);
   }
 }
 

src/nvim/ex_cmds.c

@@ -2301,15 +2301,14 @@ bool set_swapcommand(char *command, linenr_T newlnum)
   if ((command == NULL && newlnum <= 0) || *get_vim_var_str(VV_SWAPCOMMAND) != NUL) {
     return false;
   }
-  const size_t len = (command != NULL) ? strlen(command) + 3 : 30;
+  const size_t valsize = (command != NULL) ? strlen(command) + 3 : 30;
-  char *const p = xmalloc(len);
+  String val;
-  if (command != NULL) {
+  val.data = xmalloc(valsize);
-    vim_snprintf(p, len, ":%s\r", command);
+  val.size = (command != NULL)
-  } else {
+             ? vim_snprintf_safelen(val.data, valsize, ":%s\r", command)
-    vim_snprintf(p, len, "%" PRId64 "G", (int64_t)newlnum);
+             : vim_snprintf_safelen(val.data, valsize, "%" PRId64 "G", (int64_t)newlnum);
-  }
+  set_vim_var_string(VV_SWAPCOMMAND, val.data, (ptrdiff_t)val.size);
-  set_vim_var_string(VV_SWAPCOMMAND, p, -1);
+  xfree(val.data);
-  xfree(p);
   return true;
 }
 

src/nvim/ex_eval.c

@@ -589,13 +589,14 @@ static void catch_exception(except_T *excp)
   set_vim_var_string(VV_EXCEPTION, excp->value, -1);
   set_vim_var_list(VV_STACKTRACE, excp->stacktrace);
   if (*excp->throw_name != NUL) {
+    size_t IObufflen;
     if (excp->throw_lnum != 0) {
-      vim_snprintf(IObuff, IOSIZE, _("%s, line %" PRId64),
+      IObufflen = vim_snprintf_safelen(IObuff, IOSIZE, _("%s, line %" PRId64),
-                   excp->throw_name, (int64_t)excp->throw_lnum);
+                                       excp->throw_name, (int64_t)excp->throw_lnum);
     } else {
-      vim_snprintf(IObuff, IOSIZE, "%s", excp->throw_name);
+      IObufflen = vim_snprintf_safelen(IObuff, IOSIZE, "%s", excp->throw_name);
     }
-    set_vim_var_string(VV_THROWPOINT, IObuff, -1);
+    set_vim_var_string(VV_THROWPOINT, IObuff, (ptrdiff_t)IObufflen);
   } else {
     // throw_name not set on an exception from a command that was typed.
     set_vim_var_string(VV_THROWPOINT, NULL, -1);
@@ -639,15 +640,16 @@ static void finish_exception(except_T *excp)
     set_vim_var_string(VV_EXCEPTION, caught_stack->value, -1);
     set_vim_var_list(VV_STACKTRACE, caught_stack->stacktrace);
     if (*caught_stack->throw_name != NUL) {
+      size_t IObufflen;
       if (caught_stack->throw_lnum != 0) {
-        vim_snprintf(IObuff, IOSIZE,
+        IObufflen = vim_snprintf_safelen(IObuff, IOSIZE, _("%s, line %" PRId64),
-                     _("%s, line %" PRId64), caught_stack->throw_name,
+                                         caught_stack->throw_name,
-                     (int64_t)caught_stack->throw_lnum);
+                                         (int64_t)caught_stack->throw_lnum);
       } else {
-        vim_snprintf(IObuff, IOSIZE, "%s",
+        IObufflen = vim_snprintf_safelen(IObuff, IOSIZE, "%s",
-                     caught_stack->throw_name);
+                                         caught_stack->throw_name);
       }
-      set_vim_var_string(VV_THROWPOINT, IObuff, -1);
+      set_vim_var_string(VV_THROWPOINT, IObuff, (ptrdiff_t)IObufflen);
     } else {
       // throw_name not set on an exception from a command that was
       // typed.

src/nvim/fold.c

@@ -1720,7 +1720,7 @@ char *get_foldtext(win_T *wp, linenr_T lnum, linenr_T lnume, foldinfo_T foldinfo
     int level = MIN(foldinfo.fi_level, (int)sizeof(dashes) - 1);
     memset(dashes, '-', (size_t)level);
     dashes[level] = NUL;
-    set_vim_var_string(VV_FOLDDASHES, dashes, -1);
+    set_vim_var_string(VV_FOLDDASHES, dashes, level);
     set_vim_var_nr(VV_FOLDLEVEL, (varnumber_T)level);
 
     // skip evaluating 'foldtext' on errors

src/nvim/main.c

@@ -1546,10 +1546,10 @@ scripterror:
 
   // If there is a "+123" or "-c" command, set v:swapcommand to the first one.
   if (parmp->n_commands > 0) {
-    const size_t swcmd_len = strlen(parmp->commands[0]) + 3;
+    const size_t swcmd_len = strlen(parmp->commands[0]) + 2;
-    char *const swcmd = xmalloc(swcmd_len);
+    char *const swcmd = xmalloc(swcmd_len + 1);
-    snprintf(swcmd, swcmd_len, ":%s\r", parmp->commands[0]);
+    snprintf(swcmd, swcmd_len + 1, ":%s\r", parmp->commands[0]);
-    set_vim_var_string(VV_SWAPCOMMAND, swcmd, -1);
+    set_vim_var_string(VV_SWAPCOMMAND, swcmd, (ptrdiff_t)swcmd_len);
     xfree(swcmd);
   }
 

src/nvim/message.c

@@ -2678,21 +2678,21 @@ void msg_ui_flush(void)
 static void inc_msg_scrolled(void)
 {
   if (*get_vim_var_str(VV_SCROLLSTART) == NUL) {
-    char *p = SOURCING_NAME;
+    String p = { .data = SOURCING_NAME };
     char *tofree = NULL;
 
     // v:scrollstart is empty, set it to the script/function name and line
     // number
-    if (p == NULL) {
+    if (p.data == NULL) {
-      p = _("Unknown");
+      p = cstr_as_string(_("Unknown"));
     } else {
-      size_t len = strlen(p) + 40;
+      size_t tofreesize = strlen(p.data) + 40;
-      tofree = xmalloc(len);
+      tofree = xmalloc(tofreesize);
-      vim_snprintf(tofree, len, _("%s line %" PRId64),
+      p.size = vim_snprintf_safelen(tofree, tofreesize, _("%s line %" PRId64),
-                   p, (int64_t)SOURCING_LNUM);
+                                    p.data, (int64_t)SOURCING_LNUM);
-      p = tofree;
+      p.data = tofree;
     }
-    set_vim_var_string(VV_SCROLLSTART, p, -1);
+    set_vim_var_string(VV_SCROLLSTART, p.data, (ptrdiff_t)p.size);
     xfree(tofree);
   }
   msg_scrolled++;

src/nvim/normal.c

@@ -5857,7 +5857,7 @@ static void set_op_var(int optype)
     opchars[1] = (char)opchar1;
 
     opchars[2] = NUL;
-    set_vim_var_string(VV_OP, opchars, -1);
+    set_vim_var_string(VV_OP, opchars, 2);
   }
 }
 

src/nvim/option.c

@@ -1839,25 +1839,26 @@ static void apply_optionset_autocmd(OptIndex opt_idx, int opt_flags, OptVal oldv
   typval_T oldval_l_tv = optval_as_tv(oldval_l, false);
   typval_T newval_tv = optval_as_tv(newval, false);
 
-  vim_snprintf(buf_type, sizeof(buf_type), "%s", (opt_flags & OPT_LOCAL) ? "local" : "global");
-  set_vim_var_tv(VV_OPTION_NEW, &newval_tv);
   set_vim_var_tv(VV_OPTION_OLD, &oldval_tv);
-  set_vim_var_string(VV_OPTION_TYPE, buf_type, -1);
+  set_vim_var_tv(VV_OPTION_NEW, &newval_tv);
+  size_t typelen = vim_snprintf_safelen(buf_type, sizeof(buf_type), "%s",
+                                        (opt_flags & OPT_LOCAL) ? "local" : "global");
+  set_vim_var_string(VV_OPTION_TYPE, buf_type, (ptrdiff_t)typelen);
   if (opt_flags & OPT_LOCAL) {
-    set_vim_var_string(VV_OPTION_COMMAND, "setlocal", -1);
+    set_vim_var_string(VV_OPTION_COMMAND, S_LEN("setlocal"));
     set_vim_var_tv(VV_OPTION_OLDLOCAL, &oldval_tv);
   }
   if (opt_flags & OPT_GLOBAL) {
-    set_vim_var_string(VV_OPTION_COMMAND, "setglobal", -1);
+    set_vim_var_string(VV_OPTION_COMMAND, S_LEN("setglobal"));
     set_vim_var_tv(VV_OPTION_OLDGLOBAL, &oldval_tv);
   }
   if ((opt_flags & (OPT_LOCAL | OPT_GLOBAL)) == 0) {
-    set_vim_var_string(VV_OPTION_COMMAND, "set", -1);
+    set_vim_var_string(VV_OPTION_COMMAND, S_LEN("set"));
     set_vim_var_tv(VV_OPTION_OLDLOCAL, &oldval_l_tv);
     set_vim_var_tv(VV_OPTION_OLDGLOBAL, &oldval_g_tv);
   }
   if (opt_flags & OPT_MODELINE) {
-    set_vim_var_string(VV_OPTION_COMMAND, "modeline", -1);
+    set_vim_var_string(VV_OPTION_COMMAND, S_LEN("modeline"));
     set_vim_var_tv(VV_OPTION_OLDLOCAL, &oldval_tv);
   }
   apply_autocmds(EVENT_OPTIONSET, options[opt_idx].fullname, NULL, false, NULL);

src/nvim/tag.c

@@ -748,8 +748,8 @@ void do_tag(char *tag, int type, int count, int forceit, bool verbose)
       }
 
       // Let the SwapExists event know what tag we are jumping to.
-      vim_snprintf(IObuff, IOSIZE, ":ta %s\r", name);
+      size_t IObufflen = vim_snprintf_safelen(IObuff, IOSIZE, ":ta %s\r", name);
-      set_vim_var_string(VV_SWAPCOMMAND, IObuff, -1);
+      set_vim_var_string(VV_SWAPCOMMAND, IObuff, (ptrdiff_t)IObufflen);
 
       // Jump to the desired match.
       int i = jumpto_tag(matches[cur_match], forceit, true);