refactor: Remove strncpy/STRNCPY. (#6008)

Closes #731
References #851

Note: This does not remove some intentional legacy usages of strncpy.
      - memcpy isn't equivalent because it doesn't check the string
        length of `src`, and doesn't zero-out the remainder of `dst`.
      - xstrlcpy isn't equivalent because it doesn't zero-out the
        remainder of `dst`. Some Vim logic depends on that (e.g.
        ex_append which calls vim_strnsave).

Helped-by: Douglas Schneider <ds3@ualberta.ca>
Helped-by: oni-link <knil.ino@gmail.com>
Helped-by: James McCoy <jamessan@jamessan.com>

src/clint.py

@@ -3167,14 +3167,19 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension,
     if Search(r'\bsprintf\b', line):
         error(filename, linenum, 'runtime/printf', 5,
               'Use snprintf instead of sprintf.')
-    match = Search(r'\b(STRCPY|strcpy)\b', line)
+    match = Search(r'\b(strncpy|STRNCPY)\b', line)
+    if match:
+        error(filename, linenum, 'runtime/printf', 4,
+              'Use xstrlcpy or snprintf instead of %s (unless this is from Vim)'
+              % match.group(1))
+    match = Search(r'\b(strcpy)\b', line)
     if match:
         error(filename, linenum, 'runtime/printf', 4,
               'Use xstrlcpy or snprintf instead of %s' % match.group(1))
-    match = Search(r'\b(STRNCAT|strncat)\b', line)
+    match = Search(r'\b(STRNCAT|strncat|strcat)\b', line)
     if match:
         error(filename, linenum, 'runtime/printf', 4,
-              'Use xstrlcat instead of %s' % match.group(1))
+              'Use xstrlcat or snprintf instead of %s' % match.group(1))
 
     # Check for suspicious usage of "if" like
     # } if (a == b) {

src/nvim/api/vim.c

@@ -310,7 +310,7 @@ void nvim_set_current_dir(String dir, Error *err)
   }
 
   char string[MAXPATHL];
-  strncpy(string, dir.data, dir.size);
+  memcpy(string, dir.data, dir.size);
   string[dir.size] = NUL;
 
   try_start();

src/nvim/edit.c

@@ -3847,13 +3847,11 @@ static int ins_compl_get_exp(pos_T *ini)
           if ((compl_cont_status & CONT_ADDING)
               && len == compl_length) {
             if (pos->lnum < ins_buf->b_ml.ml_line_count) {
-              /* Try next line, if any. the new word will be
+              // Try next line, if any. the new word will be "join" as if the
-               * "join" as if the normal command "J" was used.
+              // normal command "J" was used. IOSIZE is always greater than
-               * IOSIZE is always greater than
+              // compl_length, so the next STRNCPY always works -- Acevedo
-               * compl_length, so the next STRNCPY always
-               * works -- Acevedo */
               STRNCPY(IObuff, ptr, len);
-              ptr = ml_get_buf(ins_buf, pos->lnum + 1, FALSE);
+              ptr = ml_get_buf(ins_buf, pos->lnum + 1, false);
               tmp_ptr = ptr = skipwhite(ptr);
               /* Find start of next word. */
               tmp_ptr = find_word_start(tmp_ptr);

src/nvim/eval.c

@@ -22314,7 +22314,7 @@ char_u *get_return_cmd(void *rettv)
   }
 
   STRCPY(IObuff, ":return ");
-  STRNCPY(IObuff + 8, s, IOSIZE - 8);
+  STRLCPY(IObuff + 8, s, IOSIZE - 8);
   if (STRLEN(s) + 8 >= IOSIZE)
     STRCPY(IObuff + IOSIZE - 4, "...");
   xfree(tofree);

src/nvim/ex_cmds.c

@@ -357,12 +357,12 @@ static int sort_compare(const void *s1, const void *s2)
     // We need to copy one line into "sortbuf1", because there is no
     // guarantee that the first pointer becomes invalid when obtaining the
     // second one.
-    STRNCPY(sortbuf1, ml_get(l1.lnum) + l1.st_u.line.start_col_nr,
+    memcpy(sortbuf1, ml_get(l1.lnum) + l1.st_u.line.start_col_nr,
-            l1.st_u.line.end_col_nr - l1.st_u.line.start_col_nr + 1);
+           l1.st_u.line.end_col_nr - l1.st_u.line.start_col_nr + 1);
-    sortbuf1[l1.st_u.line.end_col_nr - l1.st_u.line.start_col_nr] = 0;
+    sortbuf1[l1.st_u.line.end_col_nr - l1.st_u.line.start_col_nr] = NUL;
-    STRNCPY(sortbuf2, ml_get(l2.lnum) + l2.st_u.line.start_col_nr,
+    memcpy(sortbuf2, ml_get(l2.lnum) + l2.st_u.line.start_col_nr,
-            l2.st_u.line.end_col_nr - l2.st_u.line.start_col_nr + 1);
+           l2.st_u.line.end_col_nr - l2.st_u.line.start_col_nr + 1);
-    sortbuf2[l2.st_u.line.end_col_nr - l2.st_u.line.start_col_nr] = 0;
+    sortbuf2[l2.st_u.line.end_col_nr - l2.st_u.line.start_col_nr] = NUL;
 
     result = sort_ic ? STRICMP(sortbuf1, sortbuf2)
              : STRCMP(sortbuf1, sortbuf2);
@@ -1404,7 +1404,7 @@ char_u *make_filter_cmd(char_u *cmd, char_u *itmp, char_u *otmp)
                               :       "(%s)";
     vim_snprintf(buf, len, fmt, (char *)cmd);
   } else {
-    strncpy(buf, (char *) cmd, len);
+    xstrlcpy(buf, (char *)cmd, len);
   }
 
   if (itmp != NULL) {
@@ -1414,7 +1414,7 @@ char_u *make_filter_cmd(char_u *cmd, char_u *itmp, char_u *otmp)
 #else
   // For shells that don't understand braces around commands, at least allow
   // the use of commands in a pipe.
-  strncpy(buf, cmd, len);
+  xstrlcpy(buf, cmd, len);
   if (itmp != NULL) {
     // If there is a pipe, we have to put the '<' in front of it.
     // Don't do this when 'shellquote' is not empty, otherwise the

src/nvim/ex_docmd.c

@@ -3854,7 +3854,7 @@ static char_u *replace_makeprg(exarg_T *eap, char_u *p, char_u **cmdlinep)
       ptr = new_cmdline;
       while ((pos = (char_u *)strstr((char *)program, "$*")) != NULL) {
         i = (int)(pos - program);
-        STRNCPY(ptr, program, i);
+        memcpy(ptr, program, i);
         STRCPY(ptr += i, p);
         ptr += len;
         program = pos + 2;

src/nvim/fold.c

@@ -1608,7 +1608,7 @@ static void foldAddMarker(linenr_T lnum, char_u *marker, size_t markerlen)
       STRLCPY(newline + line_len, marker, markerlen + 1);
     else {
       STRCPY(newline + line_len, cms);
-      STRNCPY(newline + line_len + (p - cms), marker, markerlen);
+      memcpy(newline + line_len + (p - cms), marker, markerlen);
       STRCPY(newline + line_len + (p - cms) + markerlen, p + 2);
     }
 
@@ -1673,7 +1673,8 @@ static void foldDelMarker(linenr_T lnum, char_u *marker, size_t markerlen)
     if (u_save(lnum - 1, lnum + 1) == OK) {
       /* Make new line: text-before-marker + text-after-marker */
       newline = xmalloc(STRLEN(line) - len + 1);
-      STRNCPY(newline, line, p - line);
+      assert(p >= line);
+      memcpy(newline, line, (size_t)(p - line));
       STRCPY(newline + (p - line), p + len);
       ml_replace(lnum, newline, FALSE);
     }

src/nvim/memory.c

@@ -364,29 +364,28 @@ char *xstpncpy(char *restrict dst, const char *restrict src, size_t maxlen)
     }
 }
 
-/// xstrlcpy - Copy a %NUL terminated string into a sized buffer
+/// xstrlcpy - Copy a NUL-terminated string into a sized buffer
 ///
-/// Compatible with *BSD strlcpy: the result is always a valid
+/// Compatible with *BSD strlcpy: the result is always a valid NUL-terminated
-/// NUL-terminated string that fits in the buffer (unless,
+/// string that fits in the buffer (unless, of course, the buffer size is
-/// of course, the buffer size is zero). It does not pad
+/// zero). It does not pad out the result like strncpy() does.
-/// out the result like strncpy() does.
 ///
-/// @param dst Where to copy the string to
+/// @param dst    Buffer to store the result
-/// @param src Where to copy the string from
+/// @param src    String to be copied
-/// @param size Size of destination buffer
+/// @param size   Size of `dst`
-/// @return Length of the source string (i.e.: strlen(src))
+/// @return       strlen(src). If retval >= dstsize, truncation occurs.
 size_t xstrlcpy(char *restrict dst, const char *restrict src, size_t size)
   FUNC_ATTR_NONNULL_ALL
 {
-  size_t ret = strlen(src);
+  size_t slen = strlen(src);
 
   if (size) {
-    size_t len = (ret >= size) ? size - 1 : ret;
+    size_t len = MIN(slen, size - 1);
     memcpy(dst, src, len);
     dst[len] = '\0';
   }
 
-  return ret;
+  return slen;  // Does not include NUL.
 }
 
 /// xstrlcat - Appends string src to the end of dst.
@@ -396,8 +395,8 @@ size_t xstrlcpy(char *restrict dst, const char *restrict src, size_t size)
 ///
 /// Note: Replaces `vim_strcat`.
 ///
-/// @param dst      Where to copy the string to
+/// @param dst      Buffer to store the string
-/// @param src      Where to copy the string from
+/// @param src      String to be copied
 /// @param dstsize  Size of destination buffer, must be greater than 0
 /// @return         strlen(src) + MIN(dstsize, strlen(initial dst)).
 ///                 If retval >= dstsize, truncation occurs.

src/nvim/os/env.c

@@ -112,11 +112,11 @@ int64_t os_get_pid(void)
 #endif
 }
 
-/// Get the hostname of the machine running Neovim.
+/// Gets the hostname of the current machine.
 ///
-/// @param hostname Buffer to store the hostname.
+/// @param hostname   Buffer to store the hostname.
-/// @param len Length of `hostname`.
+/// @param size       Size of `hostname`.
-void os_get_hostname(char *hostname, size_t len)
+void os_get_hostname(char *hostname, size_t size)
 {
 #ifdef HAVE_SYS_UTSNAME_H
   struct utsname vutsname;
@@ -124,8 +124,7 @@ void os_get_hostname(char *hostname, size_t len)
   if (uname(&vutsname) < 0) {
     *hostname = '\0';
   } else {
-    strncpy(hostname, vutsname.nodename, len - 1);
+    xstrlcpy(hostname, vutsname.nodename, size);
-    hostname[len - 1] = '\0';
   }
 #else
   // TODO(unknown): Implement this for windows.

src/nvim/path.c

@@ -586,7 +586,7 @@ static size_t do_path_expand(garray_T *gap, const char_u *path,
     }
     if (has_mbyte) {
       len = (size_t)(*mb_ptr2len)(path_end);
-      STRNCPY(p, path_end, len);
+      memcpy(p, path_end, len);
       p += len;
       path_end += len;
     } else
@@ -2188,7 +2188,8 @@ static int path_get_absolute_path(const char_u *fname, char_u *buf,
         relative_directory[0] = '/';
         relative_directory[1] = NUL;
       } else {
-        STRNCPY(relative_directory, fname, p-fname);
+        assert(p >= fname);
+        memcpy(relative_directory, fname, (size_t)(p - fname));
         relative_directory[p-fname] = NUL;
       }
       end_of_path = (char *) (p + 1);

src/nvim/strings.c

@@ -51,15 +51,14 @@ char_u *vim_strsave(const char_u *string)
   return (char_u *)xstrdup((char *)string);
 }
 
-/*
+/// Copy up to `len` bytes of `string` into newly allocated memory and
- * Copy up to "len" bytes of "string" into newly allocated memory and
+/// terminate with a NUL. The allocated memory always has size `len + 1`, even
- * terminate with a NUL.
+/// when `string` is shorter.
- * The allocated memory always has size "len + 1", also when "string" is
- * shorter.
- */
 char_u *vim_strnsave(const char_u *string, size_t len)
   FUNC_ATTR_NONNULL_RET FUNC_ATTR_MALLOC FUNC_ATTR_NONNULL_ALL
 {
+  // strncpy is intentional: some parts of Vim use `string` shorter than `len`
+  // and expect the remainder to be zeroed out.
   return (char_u *)strncpy(xmallocz(len), (char *)string, len);
 }
 

src/nvim/syntax.c

@@ -3377,7 +3377,7 @@ static void syn_cmd_onoff(exarg_T *eap, char *name)
   eap->nextcmd = check_nextcmd(eap->arg);
   if (!eap->skip) {
     char buf[100];
-    strncpy(buf, "so ", 4);
+    memcpy(buf, "so ", 4);
     vim_snprintf(buf + 3, sizeof(buf) - 3, SYNTAX_FNAME, name);
     do_cmdline_cmd(buf);
   }