Merge pull request #1141 from splinterofchaos/system-nl

vim-patch:7.4.247 + vim-patch:7.4.248 + vim-patch:7.4.256 + f_system() bug fix

src/nvim/eval.c

@@ -4686,12 +4686,13 @@ list_T *list_alloc(void) FUNC_ATTR_NONNULL_RET
 /*
  * Allocate an empty list for a return value.
  */
-static void rettv_list_alloc(typval_T *rettv)
+static list_T *rettv_list_alloc(typval_T *rettv)
 {
   list_T *l = list_alloc();
   rettv->vval.v_list = l;
   rettv->v_type = VAR_LIST;
   ++l->lv_refcount;
+  return l;
 }
 
 /*
@@ -6543,6 +6544,7 @@ static struct fst {
   {"synconcealed",    2, 2, f_synconcealed},
   {"synstack",        2, 2, f_synstack},
   {"system",          1, 2, f_system},
+  {"systemlist",      1, 2, f_systemlist},
   {"tabpagebuflist",  0, 1, f_tabpagebuflist},
   {"tabpagenr",       0, 1, f_tabpagenr},
   {"tabpagewinnr",    1, 2, f_tabpagewinnr},
@@ -9407,16 +9409,15 @@ static void f_getpid(typval_T *argvars, typval_T *rettv)
 static void getpos_both(typval_T *argvars, typval_T *rettv, bool getcurpos)
 {
   pos_T *fp;
-  list_T *l;
   int fnum = -1;
 
-  rettv_list_alloc(rettv);
-  l = rettv->vval.v_list;
   if (getcurpos) {
     fp = &curwin->w_cursor;
   } else {
     fp = var2fpos(&argvars[0], true, &fnum);
   }
+
+  list_T *l = rettv_list_alloc(rettv);
   list_append_number(l, (fnum != -1) ? (varnumber_T)fnum : (varnumber_T)0);
   list_append_number(l, (fp != NULL) ? (varnumber_T)fp->lnum : (varnumber_T)0);
   list_append_number(l,
@@ -9873,7 +9874,7 @@ static void f_has(typval_T *argvars, typval_T *rettv)
     "spell",
     "syntax",
 #if !defined(UNIX)
-    "system",
+    "system",  // TODO(SplinterOfChaos): This IS defined for UNIX!
 #endif
     "tag_binary",
     "tag_old_static",
@@ -12009,8 +12010,7 @@ static void f_remove(typval_T *argvars, typval_T *rettv)
             EMSG(_(e_invrange));
           else {
             vim_list_remove(l, item, item2);
-            rettv_list_alloc(rettv);
-            l = rettv->vval.v_list;
+            l = rettv_list_alloc(rettv);
             l->lv_first = item;
             l->lv_last = item2;
             item->li_prev = NULL;
@@ -14415,8 +14415,8 @@ static void f_synstack(typval_T *argvars, typval_T *rettv)
   }
 }
 
-/// f_system - the VimL system() function
-static void f_system(typval_T *argvars, typval_T *rettv)
+static void get_system_output_as_rettv(typval_T *argvars, typval_T *rettv, 
+                                       bool retlist)
 {
   rettv->v_type = VAR_STRING;
   rettv->vval.v_string = NULL;
@@ -14426,10 +14426,11 @@ static void f_system(typval_T *argvars, typval_T *rettv)
   }
 
   // get input to the shell command (if any), and its length
-  char_u buf[NUMBUFLEN];
-  const char *input = (argvars[1].v_type != VAR_UNKNOWN)
-      ? (char *) get_tv_string_buf_chk(&argvars[1], buf): NULL;
-  size_t input_len = input ? strlen(input) : 0;
+  ssize_t input_len;
+  char *input = (char *) save_tv_as_string(&argvars[1], &input_len);
+  if (input_len == -1) {
+    return;
+  }
 
   // get shell command to execute
   const char *cmd = (char *) get_tv_string(&argvars[0]);
@@ -14439,11 +14440,40 @@ static void f_system(typval_T *argvars, typval_T *rettv)
   char *res = NULL;
   int status = os_system(cmd, input, input_len, &res, &nread);
 
+  free(input);
+
   set_vim_var_nr(VV_SHELL_ERROR, (long) status);
 
+  if (res == NULL) {
+    return;
+  }
+
+  if (retlist) {
+    list_T *list = rettv_list_alloc(rettv);
+
+    // Copy each line to a list element using NL as the delimiter.
+    for (size_t i = 0; i < nread; i++) {
+      char_u *start = (char_u *) res + i;
+      size_t len = (char_u *) xmemscan(start, NL, nread - i) - start;
+      i += len;
+
+      // Don't use a str function to copy res as it may contains NULs.
+      char_u *s = xmemdupz(start, len);
+      memchrsub(s, NUL, NL, len);  // Replace NUL with NL to avoid truncation.
+
+      listitem_T  *li = listitem_alloc();
+      li->li_tv.v_type = VAR_STRING;
+      li->li_tv.vval.v_string = s;
+      list_append(list, li);
+    }
+
+    free(res);
+  } else {
+    // res may contain several NULs before the final terminating one.
+    // Replace them with SOH (1) like in get_cmd_output() to avoid truncation.
+    memchrsub(res, NUL, 1, nread);
 #ifdef USE_CRNL
-  // translate <CR><NL> into <NL>
-  if (res != NULL) {
+    // translate <CR><NL> into <NL>
     char *d = res;
     for (char *s = res; *s; ++s) {
       if (s[0] == CAR && s[1] == NL) {
@@ -14454,12 +14484,23 @@ static void f_system(typval_T *argvars, typval_T *rettv)
     }
 
     *d = NUL;
-  }
 #endif
-
-  rettv->vval.v_string = (char_u *) res;
+    rettv->vval.v_string = (char_u *) res;
+  }
 }
 
+/// f_system - the VimL system() function
+static void f_system(typval_T *argvars, typval_T *rettv)
+{
+  get_system_output_as_rettv(argvars, rettv, false);
+}
+
+static void f_systemlist(typval_T *argvars, typval_T *rettv)
+{
+  get_system_output_as_rettv(argvars, rettv, true);
+}
+
+
 /*
  * "tabpagebuflist()" function
  */
@@ -14600,8 +14641,7 @@ static void f_taglist(typval_T *argvars, typval_T *rettv)
   if (*tag_pattern == NUL)
     return;
 
-  rettv_list_alloc(rettv);
-  (void)get_tags(rettv->vval.v_list, tag_pattern);
+  (void)get_tags(rettv_list_alloc(rettv), tag_pattern);
 }
 
 /*
@@ -15058,6 +15098,89 @@ static void f_winsaveview(typval_T *argvars, typval_T *rettv)
   dict_add_nr_str(dict, "skipcol", (long)curwin->w_skipcol, NULL);
 }
 
+/// Writes list of strings to file
+static bool write_list(FILE *fd, list_T *list, bool binary)
+{
+  int ret = true;
+
+  for (listitem_T *li = list->lv_first; li != NULL; li = li->li_next) {
+    for (char_u *s = get_tv_string(&li->li_tv); *s != NUL; ++s) {
+      if (putc(*s == '\n' ? NUL : *s, fd) == EOF) {
+        ret = false;
+        break;
+      }
+    }
+    if (!binary || li->li_next != NULL) {
+      if (putc('\n', fd) == EOF) {
+        ret = false;
+        break;
+      }
+    }
+    if (ret == false) {
+      EMSG(_(e_write));
+      break;
+    }
+  }
+  return ret;
+}
+
+/// Saves a typval_T as a string.
+///
+/// For lists, replaces NLs with NUL and separates items with NLs.
+///
+/// @param[in]  tv   A value to store as a string.
+/// @param[out] len  The length of the resulting string or -1 on error.
+/// @returns an allocated string if `tv` represents a VimL string, list, or
+///          number; NULL otherwise.
+static char_u *save_tv_as_string(typval_T *tv, ssize_t *len)
+  FUNC_ATTR_MALLOC FUNC_ATTR_NONNULL_ALL
+{
+  if (tv->v_type == VAR_UNKNOWN) {
+    *len = 0;
+    return NULL;
+  }
+
+  // For types other than list, let get_tv_string_buf_chk() get the value or
+  // print an error.
+  if (tv->v_type != VAR_LIST) {
+    char_u *ret = get_tv_string_chk(tv);
+    if (ret && (*len = STRLEN(ret))) {
+      ret = vim_strsave(ret);
+    } else {
+      ret = NULL;
+    }
+    if (tv->v_type != VAR_STRING) {
+      *len = -1;
+    }
+    return ret;
+  }
+
+  // Pre-calculate the resulting length.
+  *len = 0;
+  list_T *list = tv->vval.v_list;
+  for (listitem_T *li = list->lv_first; li != NULL; li = li->li_next) {
+    *len += STRLEN(get_tv_string(&li->li_tv)) + 1;
+  }
+
+  if (*len == 0) {
+    return NULL;
+  }
+
+  char_u *ret = xmalloc(*len);
+  char_u *end = ret;
+  for (listitem_T *li = list->lv_first; li != NULL; li = li->li_next) {
+    for (char_u *s = get_tv_string(&li->li_tv); *s != NUL; s++) {
+      *end++ = (*s == '\n') ? NUL : *s;
+    }
+    if (li->li_next != NULL) {
+      *end++ = '\n';
+    }
+  }
+  *end = NUL;
+  *len = end - ret;
+  return ret;
+}
+
 /*
  * "winwidth(nr)" function
  */
@@ -15072,68 +15195,43 @@ static void f_winwidth(typval_T *argvars, typval_T *rettv)
     rettv->vval.v_number = wp->w_width;
 }
 
-/*
- * "writefile()" function
- */
+/// "writefile()" function
 static void f_writefile(typval_T *argvars, typval_T *rettv)
 {
-  int binary = FALSE;
-  char_u      *fname;
-  FILE        *fd;
-  listitem_T  *li;
-  char_u      *s;
-  int ret = 0;
-  int c;
+  rettv->vval.v_number = 0;  // Assuming success.
 
-  if (check_restricted() || check_secure())
+  if (check_restricted() || check_secure()) {
     return;
+  }
 
   if (argvars[0].v_type != VAR_LIST) {
     EMSG2(_(e_listarg), "writefile()");
     return;
   }
-  if (argvars[0].vval.v_list == NULL)
+  if (argvars[0].vval.v_list == NULL) {
     return;
+  }
 
+  bool binary = false;
   if (argvars[2].v_type != VAR_UNKNOWN
-      && STRCMP(get_tv_string(&argvars[2]), "b") == 0)
-    binary = TRUE;
+      && STRCMP(get_tv_string(&argvars[2]), "b") == 0) {
+    binary = true;
+  }
 
-  /* Always open the file in binary mode, library functions have a mind of
-   * their own about CR-LF conversion. */
-  fname = get_tv_string(&argvars[1]);
+  // Always open the file in binary mode, library functions have a mind of
+  // their own about CR-LF conversion.
+  char_u *fname = get_tv_string(&argvars[1]);
+  FILE *fd;
   if (*fname == NUL || (fd = mch_fopen((char *)fname, WRITEBIN)) == NULL) {
     EMSG2(_(e_notcreate), *fname == NUL ? (char_u *)_("<empty>") : fname);
-    ret = -1;
+    rettv->vval.v_number = -1;
   } else {
-    for (li = argvars[0].vval.v_list->lv_first; li != NULL;
-         li = li->li_next) {
-      for (s = get_tv_string(&li->li_tv); *s != NUL; ++s) {
-        if (*s == '\n')
-          c = putc(NUL, fd);
-        else
-          c = putc(*s, fd);
-        if (c == EOF) {
-          ret = -1;
-          break;
-        }
-      }
-      if (!binary || li->li_next != NULL)
-        if (putc('\n', fd) == EOF) {
-          ret = -1;
-          break;
-        }
-      if (ret < 0) {
-        EMSG(_(e_write));
-        break;
-      }
+    if (write_list(fd, argvars[0].vval.v_list, binary) == false) {
+      rettv->vval.v_number = -1;
     }
     fclose(fd);
   }
-
-  rettv->vval.v_number = ret;
 }
-
 /*
  * "xor(expr, expr)" function
  */

src/nvim/ex_cmds2.c

@@ -3181,8 +3181,8 @@ static char_u **find_locales(void)
 
   /* Find all available locales by running command "locale -a".  If this
    * doesn't work we won't have completion. */
-  char_u *locale_a = get_cmd_output((char_u *)"locale -a",
-      NULL, kShellOptSilent);
+  char_u *locale_a = get_cmd_output((char_u *)"locale -a", NULL,
+                                    kShellOptSilent, NULL);
   if (locale_a == NULL)
     return NULL;
   ga_init(&locales_ga, sizeof(char_u *), 20);

src/nvim/memory.c

@@ -1,5 +1,6 @@
  // Various routines dealing with allocation and deallocation of memory.
 
+#include <assert.h>
 #include <errno.h>
 #include <inttypes.h>
 #include <string.h>
@@ -222,6 +223,66 @@ void *xmemdupz(const void *data, size_t len)
   return memcpy(xmallocz(len), data, len);
 }
 
+/// A version of strchr() that returns a pointer to the terminating NUL if it
+/// doesn't find `c`.
+///
+/// @param str The string to search.
+/// @param c   The char to look for.
+/// @returns a pointer to the first instance of `c`, or to the NUL terminator
+///          if not found.
+char *xstrchrnul(const char *str, char c)
+  FUNC_ATTR_NONNULL_RET FUNC_ATTR_NONNULL_ALL FUNC_ATTR_PURE
+{
+  char *p = strchr(str, c);
+  return p ? p : (char *)(str + strlen(str));
+}
+
+/// A version of memchr() that returns a pointer one past the end
+/// if it doesn't find `c`.
+///
+/// @param addr The address of the memory object.
+/// @param c    The char to look for.
+/// @param size The size of the memory object.
+/// @returns a pointer to the first instance of `c`, or one past the end if not
+///          found.
+void *xmemscan(const void *addr, char c, size_t size)
+  FUNC_ATTR_NONNULL_RET FUNC_ATTR_NONNULL_ALL FUNC_ATTR_PURE
+{
+  char *p = memchr(addr, c, size);
+  return p ? p : (char *)addr + size;
+}
+
+/// Replaces every instance of `c` with `x`.
+///
+/// @warning Will read past `str + strlen(str)` if `c == NUL`.
+///
+/// @param str A NUL-terminated string.
+/// @param c   The unwanted byte.
+/// @param x   The replacement.
+void strchrsub(char *str, char c, char x)
+  FUNC_ATTR_NONNULL_ALL
+{
+  assert(c != '\0');
+  while ((str = strchr(str, c))) {
+    *str++ = x;
+  }
+}
+
+/// Replaces every instance of `c` with `x`.
+///
+/// @param data An object in memory. May contain NULs.
+/// @param c    The unwanted byte.
+/// @param x    The replacement.
+/// @param len  The length of data.
+void memchrsub(void *data, char c, char x, size_t len)
+  FUNC_ATTR_NONNULL_ALL
+{
+  char *p = data, *end = (char *)data + len;
+  while ((p = memchr(p, c, (size_t)(end - p)))) {
+    *p++ = x;
+  }
+}
+
 /// The xstpcpy() function shall copy the string pointed to by src (including
 /// the terminating NUL character) into the array pointed to by dst.
 ///

src/nvim/message.c

@@ -3173,8 +3173,7 @@ int vim_vsnprintf(char *str, size_t str_m, char *fmt, va_list ap, typval_T *tvs)
     p = "";
   while (*p != NUL) {
     if (*p != '%') {
-      char    *q = strchr(p + 1, '%');
-      size_t n = (q == NULL) ? STRLEN(p) : (size_t)(q - p);
+      size_t n = xstrchrnul(p + 1, '%') - p;
 
       /* Copy up to the next '%' or NUL without any changes. */
       if (str_l < str_m) {

src/nvim/misc1.c

@@ -3401,13 +3401,16 @@ void fast_breakcheck(void)
 
 /*
  * Get the stdout of an external command.
+ * If "ret_len" is NULL replace NUL characters with NL.  When "ret_len" is not
+ * NULL store the length there.
  * Returns an allocated string, or NULL for error.
  */
 char_u *
 get_cmd_output (
     char_u *cmd,
     char_u *infile,            /* optional input file name */
-    int flags                      /* can be SHELL_SILENT */
+    int flags,                 // can be kShellOptSilent
+    size_t *ret_len
 )
 {
   char_u      *tempname;
@@ -3463,13 +3466,15 @@ get_cmd_output (
     EMSG2(_(e_notread), tempname);
     free(buffer);
     buffer = NULL;
-  } else {
+  } else if (ret_len == NULL) {
     /* Change NUL into SOH, otherwise the string is truncated. */
     for (i = 0; i < len; ++i)
       if (buffer[i] == NUL)
         buffer[i] = 1;
 
     buffer[len] = NUL;          /* make sure the buffer is terminated */
+  } else {
+    *ret_len = len;
   }
 
 done:

src/nvim/ops.c

@@ -4903,10 +4903,9 @@ static void str_to_reg(struct yankreg *y_ptr,
    * Find the end of each line and save it into the array.
    */
   for (start = 0; start < len + extraline; start += i + 1) {
-    for (i = start; i < len; ++i)       /* find the end of the line */
-      if (str[i] == '\n')
-        break;
-    i -= start;                         /* i is now length of line */
+    // Let i represent the length of one line.
+    const char_u *p = str + start;
+    i = (char_u *)xmemscan(p, '\n', len - start) - p;
     if (i > maxlen)
       maxlen = i;
     if (append) {

src/nvim/os/fs.c

@@ -134,10 +134,7 @@ static bool is_executable_in_path(const char_u *name, char_u **abspath)
   // Walk through all entries in $PATH to check if "name" exists there and
   // is an executable file.
   for (;; ) {
-    const char *e = strchr(path, ':');
-    if (e == NULL) {
-      e = path + STRLEN(path);
-    }
+    const char *e = xstrchrnul(path, ':');
 
     // Glue together the given directory from $PATH with name and save into
     // buf.

src/nvim/path.c

@@ -1172,7 +1172,7 @@ expand_backtick (
     buffer = eval_to_string(cmd + 1, &p, TRUE);
   else
     buffer = get_cmd_output(cmd, NULL,
-        (flags & EW_SILENT) ? kShellOptSilent : 0);
+        (flags & EW_SILENT) ? kShellOptSilent : 0, NULL);
   free(cmd);
   if (buffer == NULL)
     return 0;

src/nvim/testdir/Makefile

@@ -35,7 +35,8 @@ SCRIPTS := test_autoformat_join.out                                    \
            test_listlbr.out test_listlbr_utf8.out                      \
            test_changelist.out                                         \
            test_breakindent.out                                        \
-           test_insertcount.out
+           test_insertcount.out                                        \
+           test_systen.in
 
 SCRIPTS_GUI := test16.out
 

src/nvim/testdir/test_system.ok

@@ -0,0 +1,3 @@
+
+ab
cd
+['ab
cd']

src/nvim/version.c

@@ -385,7 +385,7 @@ static int included_patches[] = {
   //259 NA
   //258 NA
   //257 NA
-  //256,
+  256,
   //255,
   //254,
   253,
@@ -393,8 +393,8 @@ static int included_patches[] = {
   251,
   //250 NA
   //249,
-  //248,
-  //247,
+  248,
+  247,
   //246,
   245,
   //244,