xstrlcat: Allow overlapped pointers. (#6017)

memcpy is not equivalent to memmove (which is used by vim_strcat), this
could cause subtle bugs if xstrlcat is used as a replacement for
vim_strcat. But vim_strcat is inconsistent: in the `else` branch it uses
strcpy, which doesn't allow overlap.

Helped-by: oni-link <knil.ino@gmail.com>
Helped-by: James McCoy <jamessan@jamessan.com>
Helped-by: Nikolai Aleksandrovich Pavlov <kp-pav@yandex.ru>

3 files changed, 32 insertions, 28 deletions

diff --git a/src/clint.py b/src/clint.py
index 4fa78d3c82..ce31822ada 100755
--- a/src/clint.py
+++ b/src/clint.py
@@ -3176,7 +3176,7 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension,
     if match:
         error(filename, linenum, 'runtime/printf', 4,
               'Use xstrlcpy or snprintf instead of %s' % match.group(1))
-    match = Search(r'\b(STRNCAT|strncat|strcat)\b', line)
+    match = Search(r'\b(STRNCAT|strncat|strcat|vim_strcat)\b', line)
     if match:
         error(filename, linenum, 'runtime/printf', 4,
               'Use xstrlcat or snprintf instead of %s' % match.group(1))
diff --git a/src/nvim/memory.c b/src/nvim/memory.c
index 99d4acf25a..25fa2f150e 100644
--- a/src/nvim/memory.c
+++ b/src/nvim/memory.c
@@ -372,15 +372,15 @@ char *xstpncpy(char *restrict dst, const char *restrict src, size_t maxlen)
 ///
 /// @param dst    Buffer to store the result
 /// @param src    String to be copied
-/// @param size   Size of `dst`
+/// @param dsize  Size of `dst`
 /// @return       strlen(src). If retval >= dstsize, truncation occurs.
-size_t xstrlcpy(char *restrict dst, const char *restrict src, size_t size)
+size_t xstrlcpy(char *restrict dst, const char *restrict src, size_t dsize)
   FUNC_ATTR_NONNULL_ALL
 {
   size_t slen = strlen(src);
 
-  if (size) {
-    size_t len = MIN(slen, size - 1);
+  if (dsize) {
+    size_t len = MIN(slen, dsize - 1);
     memcpy(dst, src, len);
     dst[len] = '\0';
   }
@@ -388,31 +388,34 @@ size_t xstrlcpy(char *restrict dst, const char *restrict src, size_t size)
   return slen;  // Does not include NUL.
 }
 
-/// xstrlcat - Appends string src to the end of dst.
+/// Appends `src` to string `dst` of size `dsize` (unlike strncat, dsize is the
+/// full size of `dst`, not space left).  At most dsize-1 characters
+/// will be copied.  Always NUL terminates. `src` and `dst` may overlap.
 ///
-/// Compatible with *BSD strlcat: Appends at most (dstsize - strlen(dst) - 1)
-/// characters. dst will be NUL-terminated.
+/// @see vim_strcat from Vim.
+/// @see strlcat from OpenBSD.
 ///
-/// Note: Replaces `vim_strcat`.
-///
-/// @param dst      Buffer to store the string
-/// @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.
-size_t xstrlcat(char *restrict dst, const char *restrict src, size_t dstsize)
+/// @param dst      Buffer to be appended-to. Must have a NUL byte.
+/// @param src      String to put at the end of `dst`
+/// @param dsize    Size of `dst` including NUL byte. Must be greater than 0.
+/// @return         strlen(src) + strlen(initial dst)
+///                 If retval >= dsize, truncation occurs.
+size_t xstrlcat(char *const dst, const char *const src, const size_t dsize)
   FUNC_ATTR_NONNULL_ALL
 {
-  assert(dstsize > 0);
-  size_t srclen = strlen(src);
-  size_t dstlen = strlen(dst);
-  size_t ret = srclen + dstlen;  // Total string length (excludes NUL)
-  if (srclen) {
-    size_t len = (ret >= dstsize) ? dstsize - 1 : ret;
-    memcpy(dst + dstlen, src, len - dstlen);
-    dst[len] = '\0';
+  assert(dsize > 0);
+  const size_t dlen = strlen(dst);
+  assert(dlen < dsize);
+  const size_t slen = strlen(src);
+
+  if (slen > dsize - dlen - 1) {
+    memmove(dst + dlen, src, dsize - dlen - 1);
+    dst[dsize - 1] = '\0';
+  } else {
+    memmove(dst + dlen, src, slen + 1);
   }
-  return ret;  // Does not include NUL.
+
+  return slen + dlen;  // Does not include NUL.
 }
 
 /// strdup() wrapper
diff --git a/src/nvim/window.c b/src/nvim/window.c
index 89228e1b0f..801969a594 100644
--- a/src/nvim/window.c
+++ b/src/nvim/window.c
@@ -129,9 +129,10 @@ newwindow:
       vim_snprintf(cbuf, sizeof(cbuf) - 5, "%" PRId64, (int64_t)Prenum);
     else
       cbuf[0] = NUL;
-    if (nchar == 'v' || nchar == Ctrl_V)
-      strcat(cbuf, "v");
-    strcat(cbuf, "new");
+    if (nchar == 'v' || nchar == Ctrl_V) {
+      xstrlcat(cbuf, "v", sizeof(cbuf));
+    }
+    xstrlcat(cbuf, "new", sizeof(cbuf));
     do_cmdline_cmd(cbuf);
     break;