Refactor get_env() to respect const qualifier

Without the casts*, the compiler rightly warns about the os_getenv
losing the qualifier.  This refactor adds a variable to manage this
properly, and renames the original variables to increase clarity.

src/nvim/eval.c

@@ -1751,13 +1751,11 @@ ex_let_one (
         name[len] = NUL;
         p = get_tv_string_chk(tv);
         if (p != NULL && op != NULL && *op == '.') {
-          bool mustfree = false;
+          char *s = vim_getenv((char *)name);
-          char *s = vim_getenv((char *)name, &mustfree);
 
           if (s != NULL) {
             p = tofree = concat_str((char_u *)s, p);
-            if (mustfree)
+            xfree(s);
-              xfree(s);
           }
         }
         if (p != NULL) {
@@ -6357,7 +6355,6 @@ static int get_env_tv(char_u **arg, typval_T *rettv, int evaluate)
 {
   char_u *name;
   char_u *string = NULL;
-  bool    mustfree = false;
   int     len;
   int     cc;
 
@@ -6372,15 +6369,9 @@ static int get_env_tv(char_u **arg, typval_T *rettv, int evaluate)
     cc = name[len];
     name[len] = NUL;
     // First try vim_getenv(), fast for normal environment vars.
-    string = (char_u *)vim_getenv((char *)name, &mustfree);
+    string = (char_u *)vim_getenv((char *)name);
-    if (string != NULL && *string != NUL) {
+    if (string == NULL || *string == NUL) {
-      if (!mustfree) {
+      xfree(string);
-        string = vim_strsave(string);
-      }
-    } else {
-      if (mustfree) {
-        xfree(string);
-      }
 
       // Next try expanding things like $VIM and ${HOME}.
       string = expand_env_save(name - 1);

src/nvim/ex_cmds.c

@@ -5061,7 +5061,6 @@ void fix_help_buffer(void)
   char_u      *fname;
   char_u      *p;
   char_u      *rt;
-  bool mustfree;
 
   /* set filetype to "help". */
   set_option_value((char_u *)"ft", 0L, (char_u *)"help", OPT_LOCAL);
@@ -5116,8 +5115,7 @@ void fix_help_buffer(void)
       p = p_rtp;
       while (*p != NUL) {
         copy_option_part(&p, NameBuff, MAXPATHL, ",");
-        mustfree = FALSE;
+        rt = (char_u *)vim_getenv("VIMRUNTIME");
-        rt = (char_u *)vim_getenv("VIMRUNTIME", &mustfree);
         if (path_full_compare(rt, NameBuff, FALSE) != kEqualFiles) {
           int fcount;
           char_u      **fnames;
@@ -5242,8 +5240,7 @@ void fix_help_buffer(void)
             FreeWild(fcount, fnames);
           }
         }
-        if (mustfree)
+        xfree(rt);
-          xfree(rt);
       }
       break;
     }

src/nvim/ex_getln.c

@@ -3825,7 +3825,6 @@ static void expand_shellcmd(char_u *filepat, int *num_file, char_u ***file,
   char_u      *pat;
   int i;
   char_u      *path;
-  bool mustfree = false;
   garray_T ga;
   char_u *buf = xmalloc(MAXPATHL);
   size_t l;
@@ -3849,7 +3848,7 @@ static void expand_shellcmd(char_u *filepat, int *num_file, char_u ***file,
                               || (pat[1] == '.' && vim_ispathsep(pat[2])))))
     path = (char_u *)".";
   else {
-    path = (char_u *)vim_getenv("PATH", &mustfree);
+    path = (char_u *)vim_getenv("PATH");
     if (path == NULL)
       path = (char_u *)"";
   }
@@ -3900,8 +3899,7 @@ static void expand_shellcmd(char_u *filepat, int *num_file, char_u ***file,
 
   xfree(buf);
   xfree(pat);
-  if (mustfree)
+  xfree(path);
-    xfree(path);
 }
 
 /*

src/nvim/main.c

@@ -829,18 +829,16 @@ static void init_locale(void)
 # endif
 
   {
-    bool mustfree = false;
     char_u  *p;
 
     /* expand_env() doesn't work yet, because chartab[] is not initialized
      * yet, call vim_getenv() directly */
-    p = (char_u *)vim_getenv("VIMRUNTIME", &mustfree);
+    p = (char_u *)vim_getenv("VIMRUNTIME");
     if (p != NULL && *p != NUL) {
       vim_snprintf((char *)NameBuff, MAXPATHL, "%s/lang", p);
       bindtextdomain(VIMPACKAGE, (char *)NameBuff);
     }
-    if (mustfree)
+    xfree(p);
-      xfree(p);
     textdomain(VIMPACKAGE);
   }
   TIME_MSG("locale set");

src/nvim/option.c

@@ -1804,19 +1804,20 @@ void set_init_1(void)
 # endif
     int len;
     garray_T ga;
-    bool mustfree;
 
     ga_init(&ga, 1, 100);
     for (size_t n = 0; n < ARRAY_SIZE(names); ++n) {
-      mustfree = FALSE;
+      bool mustfree = true;
 # ifdef UNIX
-      if (*names[n] == NUL)
+      if (*names[n] == NUL) {
         p = (char_u *)"/tmp";
+        mustfree = false;
+      }
       else
 # endif
-      p = (char_u *)vim_getenv(names[n], &mustfree);
+      p = (char_u *)vim_getenv(names[n]);
       if (p != NULL && *p != NUL) {
-        /* First time count the NUL, otherwise count the ','. */
+        // First time count the NUL, otherwise count the ','.
         len = (int)STRLEN(p) + 3;
         ga_grow(&ga, len);
         if (!GA_EMPTY(&ga))
@@ -1826,8 +1827,9 @@ void set_init_1(void)
         STRCAT(ga.ga_data, "*");
         ga.ga_len += len;
       }
-      if (mustfree)
+      if(mustfree) {
         xfree(p);
+      }
     }
     if (ga.ga_data != NULL) {
       set_string_default("bsk", ga.ga_data);
@@ -1861,10 +1863,9 @@ void set_init_1(void)
     char_u  *buf;
     int i;
     int j;
-    bool mustfree = false;
 
     /* Initialize the 'cdpath' option's default value. */
-    cdpath = (char_u *)vim_getenv("CDPATH", &mustfree);
+    cdpath = (char_u *)vim_getenv("CDPATH");
     if (cdpath != NULL) {
       buf = xmalloc(2 * STRLEN(cdpath) + 2);
       {
@@ -1887,8 +1888,7 @@ void set_init_1(void)
         } else
           xfree(buf);           /* cannot happen */
       }
-      if (mustfree)
+      xfree(cdpath);
-        xfree(cdpath);
     }
   }
 
@@ -7428,11 +7428,10 @@ static void paste_option_changed(void)
 /// When "fname" is not NULL, use it to set $"envname" when it wasn't set yet.
 void vimrc_found(char_u *fname, char_u *envname)
 {
-  bool dofree = false;
   char_u      *p;
 
   if (fname != NULL) {
-    p = (char_u *)vim_getenv((char *)envname, &dofree);
+    p = (char_u *)vim_getenv((char *)envname);
     if (p == NULL) {
       /* Set $MYVIMRC to the first vimrc file found. */
       p = FullName_save(fname, FALSE);
@@ -7440,8 +7439,9 @@ void vimrc_found(char_u *fname, char_u *envname)
         vim_setenv((char *)envname, (char *)p);
         xfree(p);
       }
-    } else if (dofree)
+    } else {
       xfree(p);
+    }
   }
 }
 

src/nvim/os/env.c

@@ -215,31 +215,29 @@ void expand_env_esc(char_u *srcp, char_u *dst, int dstlen, bool esc, bool one,
   int c;
   char_u      *var;
   bool copy_char;
-  bool mustfree;                 /* var was allocated, need to free it later */
+  bool mustfree;  // var was allocated, need to free it later
-  bool at_start = true;           /* at start of a name */
+  bool at_start = true;  // at start of a name
   int startstr_len = 0;
 
   if (startstr != NULL)
     startstr_len = (int)STRLEN(startstr);
 
   src = skipwhite(srcp);
-  --dstlen;                 /* leave one char space for "\," */
+  --dstlen;  // leave one char space for "\,"
   while (*src && dstlen > 0) {
     copy_char = true;
     if ((*src == '$') || (*src == '~' && at_start)) {
       mustfree = false;
 
-      /*
+      // The variable name is copied into dst temporarily, because it may
-       * The variable name is copied into dst temporarily, because it may
+      // be a string in read-only memory and a NUL needs to be appended.
-       * be a string in read-only memory and a NUL needs to be appended.
+      if (*src != '~') {  // environment var
-       */
-      if (*src != '~') {                                /* environment var */
         tail = src + 1;
         var = dst;
         c = dstlen - 1;
 
 #ifdef UNIX
-        /* Unix has ${var-name} type environment vars */
+        // Unix has ${var-name} type environment vars
         if (*tail == '{' && !vim_isIDc('{')) {
           tail++;               /* ignore '{' */
           while (c-- > 0 && *tail && *tail != '}')
@@ -262,7 +260,8 @@ void expand_env_esc(char_u *srcp, char_u *dst, int dstlen, bool esc, bool one,
           }
 #endif
         *var = NUL;
-        var = (char_u *)vim_getenv((char *)dst, &mustfree);
+        var = (char_u *)vim_getenv((char *)dst);
+        mustfree = true;
 #if defined(UNIX)
         }
 #endif
@@ -271,11 +270,9 @@ void expand_env_esc(char_u *srcp, char_u *dst, int dstlen, bool esc, bool one,
                  || vim_strchr((char_u *)" ,\t\n", src[1]) != NULL) {
         var = homedir;
         tail = src + 1;
-      } else {                                        /* user directory */
+      } else {  // user directory
 #if defined(UNIX)
-        /*
+        // Copy ~user to dst[], so we can put a NUL after it.
-         * Copy ~user to dst[], so we can put a NUL after it.
-         */
         tail = src;
         var = dst;
         c = dstlen - 1;
@@ -285,12 +282,10 @@ void expand_env_esc(char_u *srcp, char_u *dst, int dstlen, bool esc, bool one,
                    && !vim_ispathsep(*tail))
           *var++ = *tail++;
         *var = NUL;
-        /*
+        // Use os_get_user_directory() to get the user directory.
-         * Use os_get_user_directory() to get the user directory.
+        // If this function fails, the shell is used to
-         * If this function fails, the shell is used to
+        // expand ~user. This is slower and may fail if the shell
-         * expand ~user. This is slower and may fail if the shell
+        // does not support ~user (old versions of /bin/sh).
-         * does not support ~user (old versions of /bin/sh).
-         */
         var = (char_u *)os_get_user_directory((char *)dst + 1);
         mustfree = true;
         if (var == NULL)
@@ -304,15 +299,15 @@ void expand_env_esc(char_u *srcp, char_u *dst, int dstlen, bool esc, bool one,
           mustfree = true;
         }
 #else
-        /* cannot expand user's home directory, so don't try */
+        // cannot expand user's home directory, so don't try
         var = NULL;
-        tail = (char_u *)"";            /* for gcc */
+        tail = (char_u *)"";  // for gcc
-#endif /* UNIX */
+#endif  // UNIX
       }
 
 #ifdef BACKSLASH_IN_FILENAME
-      /* If 'shellslash' is set change backslashes to forward slashes.
+      // If 'shellslash' is set change backslashes to forward slashes.
-       * Can't use slash_adjust(), p_ssl may be set temporarily. */
+      // Can't use slash_adjust(), p_ssl may be set temporarily.
       if (p_ssl && var != NULL && vim_strchr(var, '\\') != NULL) {
         char_u  *p = vim_strsave(var);
 
@@ -325,8 +320,8 @@ void expand_env_esc(char_u *srcp, char_u *dst, int dstlen, bool esc, bool one,
       }
 #endif
 
-      /* If "var" contains white space, escape it with a backslash.
+      // If "var" contains white space, escape it with a backslash.
-       * Required for ":e ~/tt" when $HOME includes a space. */
+      // Required for ":e ~/tt" when $HOME includes a space.
       if (esc && var != NULL && vim_strpbrk(var, (char_u *)" \t") != NULL) {
         char_u  *p = vim_strsave_escaped(var, (char_u *)" \t");
 
@@ -341,8 +336,8 @@ void expand_env_esc(char_u *srcp, char_u *dst, int dstlen, bool esc, bool one,
         STRCPY(dst, var);
         dstlen -= (int)STRLEN(var);
         c = (int)STRLEN(var);
-        /* if var[] ends in a path separator and tail[] starts
+        // if var[] ends in a path separator and tail[] starts
-         * with it, skip a character */
+        // with it, skip a character
         if (*var != NUL && after_pathsep((char *)dst, (char *)dst + c)
 #if defined(BACKSLASH_IN_FILENAME)
             && dst[-1] != ':'
@@ -357,12 +352,10 @@ void expand_env_esc(char_u *srcp, char_u *dst, int dstlen, bool esc, bool one,
         xfree(var);
     }
 
-    if (copy_char) {        /* copy at least one char */
+    if (copy_char) {  // copy at least one char
-      /*
+      // Recognize the start of a new name, for '~'.
-       * Recognize the start of a new name, for '~'.
+      // Don't do this when "one" is true, to avoid expanding "~" in
-       * Don't do this when "one" is true, to avoid expanding "~" in
+      // ":edit foo ~ foo".
-       * ":edit foo ~ foo".
-       */
       at_start = false;
       if (src[0] == '\\' && src[1] != NUL) {
         *dst++ = *src++;
@@ -384,7 +377,7 @@ void expand_env_esc(char_u *srcp, char_u *dst, int dstlen, bool esc, bool one,
 /// Check if the directory "vimdir/<version>" or "vimdir/runtime" exists.
 /// Return NULL if not, return its name in allocated memory otherwise.
 /// @param vimdir directory to test
-static char *vim_version_dir(char *vimdir)
+static char *vim_version_dir(const char *vimdir)
 {
   char_u      *p;
 
@@ -418,124 +411,112 @@ static char *remove_tail(char *p, char *pend, char *name)
 /// Vim's version of getenv().
 /// Special handling of $HOME, $VIM and $VIMRUNTIME, allowing the user to
 /// override the vim runtime directory at runtime.  Also does ACP to 'enc'
-/// conversion for Win32.
+/// conversion for Win32.  Results must be freed by the calling function.
 /// @param name Name of environment variable to expand
-/// @param[out] mustfree Ouput parameter for the caller to determine if they are
+char *vim_getenv(const char *name)
-///           responsible for releasing memory. Must be initialized to false
-///           by the caller.
-char *vim_getenv(const char *name, bool *mustfree)
 {
-  char *p;
+  const char *kos_env_path = os_getenv(name);
-  char *pend;
+  if (kos_env_path != NULL
-  int vimruntime;
+      && *kos_env_path == NUL) {  // empty is the same as not set
-
+    kos_env_path = NULL;
-
-  p = (char *)os_getenv(name);
-  if (p != NULL && *p == NUL)       /* empty is the same as not set */
-    p = NULL;
-
-  if (p != NULL) {
-    return p;
   }
 
-  vimruntime = (STRCMP(name, "VIMRUNTIME") == 0);
+  if (kos_env_path != NULL) {
-  if (!vimruntime && STRCMP(name, "VIM") != 0)
+    return xstrdup(kos_env_path);
-    return NULL;
+  }
 
-  /*
+  bool vimruntime = (strcmp(name, "VIMRUNTIME") == 0);
-   * When expanding $VIMRUNTIME fails, try using $VIM/vim<version> or $VIM.
+  if (!vimruntime && strcmp(name, "VIM") != 0) {
-   * Don't do this when default_vimruntime_dir is non-empty.
+    return NULL;
-   */
+  }
+
+  // When expanding $VIMRUNTIME fails, try using $VIM/vim<version> or $VIM.
+  // Don't do this when default_vimruntime_dir is non-empty.
+  char *vim_path = NULL;
   if (vimruntime
 #ifdef HAVE_PATHDEF
       && *default_vimruntime_dir == NUL
 #endif
       ) {
-    p = (char *)os_getenv("VIM");
+    kos_env_path = os_getenv("VIM");
-    if (p != NULL && *p == NUL)             /* empty is the same as not set */
+    if (kos_env_path != NULL
-      p = NULL;
+        && *kos_env_path == NUL) {  // empty is the same as not set
-    if (p != NULL) {
+      kos_env_path = NULL;
-      p = vim_version_dir(p);
+    }
-      if (p != NULL)
+    if (kos_env_path != NULL) {
-        *mustfree = true;
+      vim_path = vim_version_dir(kos_env_path);
-      else
+      if (vim_path == NULL) {
-        p = (char *)os_getenv("VIM");
+        vim_path = xstrdup(kos_env_path);
+      }
     }
   }
 
-  /*
+  // When expanding $VIM or $VIMRUNTIME fails, try using:
-   * When expanding $VIM or $VIMRUNTIME fails, try using:
+  // - the directory name from 'helpfile' (unless it contains '$')
-   * - the directory name from 'helpfile' (unless it contains '$')
+  // - the executable name from argv[0]
-   * - the executable name from argv[0]
+  if (vim_path == NULL) {
-   */
+    if (p_hf != NULL && vim_strchr(p_hf, '$') == NULL) {
-  if (p == NULL) {
+      vim_path = (char *)p_hf;
-    if (p_hf != NULL && vim_strchr(p_hf, '$') == NULL)
+    }
-      p = (char *)p_hf;
+    if (vim_path != NULL) {
-    if (p != NULL) {
+      // remove the file name
-      /* remove the file name */
+      char *vim_path_end = (char *)path_tail((char_u *)vim_path);
-      pend = (char *)path_tail((char_u *)p);
 
-      /* remove "doc/" from 'helpfile', if present */
+      // remove "doc/" from 'helpfile', if present
-      if (p == (char *)p_hf)
+      if (vim_path == (char *)p_hf) {
-        pend = remove_tail(p, pend, "doc");
+        vim_path_end = remove_tail(vim_path, vim_path_end, "doc");
-
-      /* for $VIM, remove "runtime/" or "vim54/", if present */
-      if (!vimruntime) {
-        pend = remove_tail(p, pend, RUNTIME_DIRNAME);
-        pend = remove_tail(p, pend, VIM_VERSION_NODOT);
       }
 
-      /* remove trailing path separator */
+      // for $VIM, remove "runtime/" or "vim54/", if present
-      if (pend > p && after_pathsep(p, pend))
+      if (!vimruntime) {
-        --pend;
+        vim_path_end = remove_tail(vim_path, vim_path_end, RUNTIME_DIRNAME);
+        vim_path_end = remove_tail(vim_path, vim_path_end, VIM_VERSION_NODOT);
+      }
+
+      // remove trailing path separator
+      if (vim_path_end > vim_path && after_pathsep(vim_path, vim_path_end)) {
+        vim_path_end--;
+      }
 
       // check that the result is a directory name
-      assert(pend >= p);
+      assert(vim_path_end >= vim_path);
-      p = xstrndup(p, (size_t)(pend - p));
+      vim_path = xstrndup(vim_path, (size_t)(vim_path_end - vim_path));
 
-      if (!os_isdir((char_u *)p)) {
+      if (!os_isdir((char_u *)vim_path)) {
-        xfree(p);
+        xfree(vim_path);
-        p = NULL;
+        vim_path = NULL;
-      } else {
-        *mustfree = true;
       }
     }
   }
 
 #ifdef HAVE_PATHDEF
-  /* When there is a pathdef.c file we can use default_vim_dir and
+  // When there is a pathdef.c file we can use default_vim_dir and
-   * default_vimruntime_dir */
+  // default_vimruntime_dir
-  if (p == NULL) {
+  if (vim_path == NULL) {
-    /* Only use default_vimruntime_dir when it is not empty */
+    // Only use default_vimruntime_dir when it is not empty
     if (vimruntime && *default_vimruntime_dir != NUL) {
-      p = default_vimruntime_dir;
+      vim_path = xstrdup(default_vimruntime_dir);
-      *mustfree = false;
     } else if (*default_vim_dir != NUL) {
       if (vimruntime
-          && (p = vim_version_dir(default_vim_dir)) != NULL) {
+          && (vim_path = vim_version_dir(default_vim_dir)) == NULL) {
-        *mustfree = true;
+        vim_path = xstrdup(default_vim_dir);
-      } else {
-        p = default_vim_dir;
-        *mustfree = false;
       }
     }
   }
 #endif
 
-  /*
+  // Set the environment variable, so that the new value can be found fast
-   * Set the environment variable, so that the new value can be found fast
+  // next time, and others can also use it (e.g. Perl).
-   * next time, and others can also use it (e.g. Perl).
+  if (vim_path != NULL) {
-   */
-  if (p != NULL) {
     if (vimruntime) {
-      vim_setenv("VIMRUNTIME", p);
+      vim_setenv("VIMRUNTIME", vim_path);
       didset_vimruntime = true;
     } else {
-      vim_setenv("VIM", p);
+      vim_setenv("VIM", vim_path);
       didset_vim = true;
     }
   }
-  return p;
+  return vim_path;
 }
 
 /// Replace home directory by "~" in each space or comma separated file name in