commit fd973c0a4ec0246708d0fcc46e66e38dd1f89a26
parent 7432781e71842a595365c351b105971ac3662dff
Author: Judit Novak <judit.novak@gmail.com>
Date: Wed, 16 Apr 2025 12:36:07 +0200
fix(env.c): drop envmap, free os_getenv() result #32683
Problem:
vim.uv.os_setenv gets "stuck" per-key. #32550
Caused by the internal `envmap` cache. #7920
:echo $FOO <-- prints nothing
:lua vim.uv.os_setenv("FOO", "bar")
:echo $FOO <-- prints bar (as expected)
:lua vim.uv.os_setenv("FOO", "fizz")
:echo $FOO <-- prints bar, still (not expected. Should be "fizz")
:lua vim.uv.os_unsetenv("FOO")
:echo $FOO <-- prints bar, still (not expected. Should be nothing)
:lua vim.uv.os_setenv("FOO", "buzz")
:echo $FOO <-- prints bar, still (not expected. Should be "buzz")
Solution:
- Remove the `envmap` cache.
- Callers to `os_getenv` must free the result.
- Update all call-sites.
- Introduce `os_getenv_noalloc`.
- Extend `os_env_exists()` the `nonempty` parameter.
Diffstat:
26 files changed, 281 insertions(+), 146 deletions(-)
diff --git a/src/nvim/diff.c b/src/nvim/diff.c
@@ -1143,7 +1143,7 @@ static int diff_file(diffio_T *dio)
char *const cmd = xmalloc(len);
// We don't want $DIFF_OPTIONS to get in the way.
- if (os_getenv("DIFF_OPTIONS")) {
+ if (os_env_exists("DIFF_OPTIONS", true)) {
os_unsetenv("DIFF_OPTIONS");
}
diff --git a/src/nvim/eval/funcs.c b/src/nvim/eval/funcs.c
@@ -1556,7 +1556,7 @@ static void f_exists(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
const char *p = tv_get_string(&argvars[0]);
if (*p == '$') { // Environment variable.
// First try "normal" environment variables (fast).
- if (os_env_exists(p + 1)) {
+ if (os_env_exists(p + 1, false)) {
n = true;
} else {
// Try expanding things like $VIM and ${HOME}.
@@ -3881,9 +3881,9 @@ dict_T *create_environment(const dictitem_T *job_env, const bool clear_env, cons
size_t len = strlen(required_env_vars[i]);
dictitem_T *dv = tv_dict_find(env, required_env_vars[i], (ptrdiff_t)len);
if (!dv) {
- const char *env_var = os_getenv(required_env_vars[i]);
+ char *env_var = os_getenv(required_env_vars[i]);
if (env_var) {
- tv_dict_add_str(env, required_env_vars[i], len, env_var);
+ tv_dict_add_allocated_str(env, required_env_vars[i], len, env_var);
}
}
}
diff --git a/src/nvim/ex_docmd.c b/src/nvim/ex_docmd.c
@@ -7810,8 +7810,8 @@ static void ex_checkhealth(exarg_T *eap)
return;
}
- const char *vimruntime_env = os_getenv("VIMRUNTIME");
- if (vimruntime_env == NULL) {
+ char *vimruntime_env = os_getenv_noalloc("VIMRUNTIME");
+ if (!vimruntime_env) {
emsg(_("E5009: $VIMRUNTIME is empty or unset"));
} else {
bool rtp_ok = NULL != strstr(p_rtp, vimruntime_env);
diff --git a/src/nvim/fileio.c b/src/nvim/fileio.c
@@ -3278,7 +3278,7 @@ static void vim_mktempdir(void)
expand_env((char *)temp_dirs[i], tmp, TEMP_FILE_PATH_MAXLEN - 64);
if (!os_isdir(tmp)) {
if (strequal("$TMPDIR", temp_dirs[i])) {
- if (!os_getenv("TMPDIR")) {
+ if (!os_env_exists("TMPDIR", true)) {
DLOG("$TMPDIR is unset");
} else {
WLOG("$TMPDIR tempdir not a directory (or does not exist): \"%s\"", tmp);
diff --git a/src/nvim/log.c b/src/nvim/log.c
@@ -336,7 +336,7 @@ static bool v_do_log_to_file(FILE *log_file, int log_level, const char *context,
// TODO(justinmk): expose this as v:name ?
if (regen) {
// Parent servername ($NVIM).
- const char *parent = path_tail(os_getenv(ENV_NVIM));
+ const char *parent = path_tail(os_getenv_noalloc(ENV_NVIM));
// Servername. Empty until starting=false.
const char *serv = path_tail(get_vim_var_str(VV_SEND_SERVER));
if (parent[0] != NUL) {
diff --git a/src/nvim/lua/executor.c b/src/nvim/lua/executor.c
@@ -831,8 +831,7 @@ static bool nlua_state_init(lua_State *const lstate) FUNC_ATTR_NONNULL_ALL
void nlua_init(char **argv, int argc, int lua_arg0)
{
#ifdef NLUA_TRACK_REFS
- const char *env = os_getenv("NVIM_LUA_NOTRACK");
- if (!env || !*env) {
+ if (os_env_exists("NVIM_LUA_NOTRACK", true)) {
nlua_track_refs = true;
}
#endif
@@ -1283,7 +1282,7 @@ static int nlua_empty_dict_tostring(lua_State *lstate)
/// @param lstate Lua interpreter state.
static int nlua_getenv(lua_State *lstate)
{
- lua_pushstring(lstate, os_getenv(luaL_checkstring(lstate, 1)));
+ lua_pushstring(lstate, os_getenv_noalloc(luaL_checkstring(lstate, 1)));
return 1;
}
#endif
diff --git a/src/nvim/main.c b/src/nvim/main.c
@@ -938,12 +938,11 @@ static void remote_request(mparm_T *params, int remote_args, char *server_addr,
if (!chan) {
fprintf(stderr, "Remote ui failed to start: %s\n", connect_error);
os_exit(1);
- } else if (strequal(server_addr, os_getenv("NVIM"))) {
+ } else if (strequal(server_addr, os_getenv_noalloc("NVIM"))) {
fprintf(stderr, "%s", "Cannot attach UI of :terminal child to its parent. ");
fprintf(stderr, "%s\n", "(Unset $NVIM to skip this check)");
os_exit(1);
}
-
ui_client_channel_id = chan;
return;
}
@@ -2118,7 +2117,7 @@ static void source_startup_scripts(const mparm_T *const parmp)
static int execute_env(char *env)
FUNC_ATTR_NONNULL_ALL
{
- const char *initstr = os_getenv(env);
+ char *initstr = os_getenv(env);
if (initstr == NULL) {
return FAIL;
}
@@ -2132,6 +2131,8 @@ static int execute_env(char *env)
estack_pop();
current_sctx = save_current_sctx;
+
+ xfree(initstr);
return OK;
}
diff --git a/src/nvim/mbyte.c b/src/nvim/mbyte.c
@@ -2412,14 +2412,15 @@ char *enc_locale(void)
char buf[50];
const char *s;
+
#ifdef HAVE_NL_LANGINFO_CODESET
if (!(s = nl_langinfo(CODESET)) || *s == NUL)
#endif
{
if (!(s = setlocale(LC_CTYPE, NULL)) || *s == NUL) {
- if ((s = os_getenv("LC_ALL"))) {
- if ((s = os_getenv("LC_CTYPE"))) {
- s = os_getenv("LANG");
+ if ((s = os_getenv_noalloc("LC_ALL"))) {
+ if ((s = os_getenv_noalloc("LC_CTYPE"))) {
+ s = os_getenv_noalloc("LANG");
}
}
}
diff --git a/src/nvim/memory.c b/src/nvim/memory.c
@@ -788,7 +788,6 @@ void free_all_mem(void)
free_all_marks();
alist_clear(&global_alist);
free_homedir();
- free_envmap();
free_users();
free_search_patterns();
free_old_sub();
diff --git a/src/nvim/msgpack_rpc/server.c b/src/nvim/msgpack_rpc/server.c
@@ -41,28 +41,24 @@ bool server_init(const char *listen_addr)
ga_init(&watchers, sizeof(SocketWatcher *), 1);
// $NVIM_LISTEN_ADDRESS (deprecated)
- if ((!listen_addr || listen_addr[0] == '\0') && os_env_exists(ENV_LISTEN)) {
- user_arg = kFalse; // User-provided env var.
- listen_addr = os_getenv(ENV_LISTEN);
- }
-
if (!listen_addr || listen_addr[0] == '\0') {
- user_arg = kNone; // Autogenerated server address.
- listen_addr = server_address_new(NULL);
+ if (os_env_exists(ENV_LISTEN, true)) {
+ user_arg = kFalse; // User-provided env var.
+ listen_addr = os_getenv(ENV_LISTEN);
+ } else {
+ user_arg = kNone; // Autogenerated server address.
+ listen_addr = server_address_new(NULL);
+ }
must_free = true;
}
int rv = server_start(listen_addr);
// TODO(justinmk): this is for log_spec. Can remove this after nvim_log #7062 is merged.
- if (os_env_exists("__NVIM_TEST_LOG")) {
+ if (os_env_exists("__NVIM_TEST_LOG", false)) {
ELOG("test log message");
}
- if (must_free) {
- xfree((char *)listen_addr);
- }
-
if (rv == 0 || user_arg == kNone) {
// The autogenerated servername can fail if the user has a broken $XDG_RUNTIME_DIR. #30282
// But that is not fatal (startup will continue, logged in $NVIM_LOGFILE, empty v:servername).
@@ -78,12 +74,16 @@ bool server_init(const char *listen_addr)
ok = false;
end:
- if (os_env_exists(ENV_LISTEN)) {
+ if (os_env_exists(ENV_LISTEN, false)) {
// Unset $NVIM_LISTEN_ADDRESS, it's a liability hereafter. It is "input only", it must not be
// leaked to child jobs or :terminal.
os_unsetenv(ENV_LISTEN);
}
+ if (must_free) {
+ xfree((char *)listen_addr);
+ }
+
return ok;
}
@@ -120,12 +120,12 @@ char *server_address_new(const char *name)
static uint32_t count = 0;
char fmt[ADDRESS_MAX_SIZE];
#ifdef MSWIN
- (void)get_appname(true);
+ (void)get_appname(true); // Writes appname to NameBuf.
int r = snprintf(fmt, sizeof(fmt), "\\\\.\\pipe\\%s.%" PRIu64 ".%" PRIu32,
name ? name : NameBuff, os_get_pid(), count++);
#else
char *dir = stdpaths_get_xdg_var(kXDGRuntimeDir);
- (void)get_appname(true);
+ (void)get_appname(true); // Writes appname to NameBuf.
int r = snprintf(fmt, sizeof(fmt), "%s/%s.%" PRIu64 ".%" PRIu32,
dir, name ? name : NameBuff, os_get_pid(), count++);
xfree(dir);
diff --git a/src/nvim/option.c b/src/nvim/option.c
@@ -190,7 +190,7 @@ static void set_init_default_shell(void)
{
// Find default value for 'shell' option.
// Don't use it if it is empty.
- const char *shell = os_getenv("SHELL");
+ char *shell = os_getenv("SHELL");
if (shell != NULL) {
if (vim_strchr(shell, ' ') != NULL) {
const size_t len = strlen(shell) + 3; // two quotes and a trailing NUL
@@ -198,8 +198,9 @@ static void set_init_default_shell(void)
snprintf(cmd, len, "\"%s\"", shell);
set_string_default(kOptShell, cmd, true);
} else {
- set_string_default(kOptShell, (char *)shell, false);
+ set_string_default(kOptShell, shell, false);
}
+ xfree(shell);
}
}
@@ -413,7 +414,7 @@ void set_init_1(bool clean_arg)
// abilities (bidi namely).
// NOTE: mlterm's author is being asked to 'set' a variable
// instead of an environment variable due to inheritance.
- if (os_env_exists("MLTERM")) {
+ if (os_env_exists("MLTERM", false)) {
set_option_value_give_err(kOptTermbidi, BOOLEAN_OPTVAL(true), 0);
}
diff --git a/src/nvim/os/env.c b/src/nvim/os/env.c
@@ -52,13 +52,11 @@
# include "os/env.c.generated.h"
#endif
-// Because `uv_os_getenv` requires allocating, we must manage a map to maintain
-// the behavior of `os_getenv`.
-static PMap(cstr_t) envmap = MAP_INIT;
-
/// Like getenv(), but returns NULL if the variable is empty.
+/// Result must be freed by the caller.
/// @see os_env_exists
-const char *os_getenv(const char *name)
+/// @see os_getenv_noalloc
+char *os_getenv(const char *name)
FUNC_ATTR_NONNULL_ALL
{
char *e = NULL;
@@ -66,17 +64,6 @@ const char *os_getenv(const char *name)
return NULL;
}
int r = 0;
- if (map_has(cstr_t, &envmap, name)
- && !!(e = (char *)pmap_get(cstr_t)(&envmap, name))) {
- if (e[0] != NUL) {
- // Found non-empty cached env var.
- // NOTE: This risks incoherence if an in-process library changes the
- // environment without going through our os_setenv() wrapper. If
- // that turns out to be a problem, we can just remove this codepath.
- goto end;
- }
- pmap_del2(&envmap, name);
- }
#define INIT_SIZE 64
size_t size = INIT_SIZE;
char buf[INIT_SIZE];
@@ -96,7 +83,6 @@ const char *os_getenv(const char *name)
// except when it does not include the NUL-terminator.
e = xmemdupz(buf, size);
}
- pmap_put(cstr_t)(&envmap, xstrdup(name), e);
end:
if (r != 0 && r != UV_ENOENT && r != UV_UNKNOWN) {
ELOG("uv_os_getenv(%s) failed: %d %s", name, r, uv_err_name(r));
@@ -104,9 +90,43 @@ end:
return e;
}
+/// Like getenv(), but returns a pointer to `NameBuff` instead of allocating, or NULL on failure.
+/// Value is truncated if it exceeds sizeof(NameBuff).
+/// @see os_env_exists
+char *os_getenv_noalloc(const char *name)
+ FUNC_ATTR_NONNULL_ALL
+{
+ if (name[0] == NUL) {
+ return NULL;
+ }
+
+ size_t size = sizeof(NameBuff);
+ int r = uv_os_getenv(name, NameBuff, &size);
+ if (r == UV_ENOBUFS) {
+ char *e = xmalloc(size);
+ r = uv_os_getenv(name, e, &size);
+ if (r == 0 && size != 0 && e[0] != NUL) {
+ xmemcpyz(NameBuff, e, sizeof(NameBuff) - 1);
+ }
+ xfree(e);
+ }
+
+ if (r != 0 || size == 0 || NameBuff[0] == NUL) {
+ if (r != 0 && r != UV_ENOENT && r != UV_UNKNOWN) {
+ ELOG("uv_os_getenv(%s) failed: %d %s", name, r, uv_err_name(r));
+ }
+ return NULL;
+ }
+ return NameBuff;
+}
+
/// Returns true if environment variable `name` is defined (even if empty).
/// Returns false if not found (UV_ENOENT) or other failure.
-bool os_env_exists(const char *name)
+///
+/// @param name the environment variable in question
+/// @param nonempty Require a non-empty value. Treat empty as "does not exist".
+/// @return whether the variable exists
+bool os_env_exists(const char *name, bool nonempty)
FUNC_ATTR_NONNULL_ALL
{
if (name[0] == NUL) {
@@ -114,14 +134,14 @@ bool os_env_exists(const char *name)
}
// Use a tiny buffer because we don't care about the value: if uv_os_getenv()
// returns UV_ENOBUFS, the env var was found.
- char buf[1];
+ char buf[2];
size_t size = sizeof(buf);
int r = uv_os_getenv(name, buf, &size);
assert(r != UV_EINVAL);
if (r != 0 && r != UV_ENOENT && r != UV_ENOBUFS) {
ELOG("uv_os_getenv(%s) failed: %d %s", name, r, uv_err_name(r));
}
- return (r == 0 || r == UV_ENOBUFS);
+ return ((r == 0 && (!nonempty || size > 0)) || r == UV_ENOBUFS);
}
/// Sets an environment variable.
@@ -137,7 +157,7 @@ int os_setenv(const char *name, const char *value, int overwrite)
return -1;
}
#ifdef MSWIN
- if (!overwrite && os_getenv(name) != NULL) {
+ if (!overwrite && !os_env_exists(name, true)) {
return 0;
}
if (value[0] == NUL) {
@@ -145,7 +165,7 @@ int os_setenv(const char *name, const char *value, int overwrite)
return os_unsetenv(name);
}
#else
- if (!overwrite && os_env_exists(name)) {
+ if (!overwrite && os_env_exists(name, false)) {
return 0;
}
#endif
@@ -160,9 +180,6 @@ int os_setenv(const char *name, const char *value, int overwrite)
#endif
r = uv_os_setenv(name, value);
assert(r != UV_EINVAL);
- // Destroy the old map item. Do this AFTER uv_os_setenv(), because `value`
- // could be a previous os_getenv() result.
- pmap_del2(&envmap, name);
if (r != 0) {
ELOG("uv_os_setenv(%s) failed: %d %s", name, r, uv_err_name(r));
}
@@ -176,7 +193,6 @@ int os_unsetenv(const char *name)
if (name[0] == NUL) {
return -1;
}
- pmap_del2(&envmap, name);
int r = uv_os_unsetenv(name);
if (r != 0) {
ELOG("uv_os_unsetenv(%s) failed: %d %s", name, r, uv_err_name(r));
@@ -428,7 +444,8 @@ void init_homedir(void)
xfree(homedir);
homedir = NULL;
- const char *var = os_getenv("HOME");
+ char *var = os_getenv("HOME");
+ char *tofree = var;
#ifdef MSWIN
// Typically, $HOME is not defined on Windows, unless the user has
@@ -436,10 +453,10 @@ void init_homedir(void)
// platforms, $HOMEDRIVE and $HOMEPATH are automatically defined for
// each user. Try constructing $HOME from these.
if (var == NULL) {
- const char *homedrive = os_getenv("HOMEDRIVE");
- const char *homepath = os_getenv("HOMEPATH");
+ char *homedrive = os_getenv("HOMEDRIVE");
+ char *homepath = os_getenv("HOMEPATH");
if (homepath == NULL) {
- homepath = "\\";
+ homepath = xstrdup("\\");
}
if (homedrive != NULL
&& strlen(homedrive) + strlen(homepath) < MAXPATHL) {
@@ -448,6 +465,8 @@ void init_homedir(void)
var = os_buf;
}
}
+ xfree(homepath);
+ xfree(homedrive);
}
if (var == NULL) {
var = os_uv_homedir();
@@ -461,11 +480,13 @@ void init_homedir(void)
if (p != NULL) {
vim_snprintf(os_buf, (size_t)(p - var), "%s", var + 1);
var = NULL;
- const char *exp = os_getenv(os_buf);
- if (exp != NULL && *exp != NUL
- && strlen(exp) + strlen(p) < MAXPATHL) {
- vim_snprintf(os_buf, MAXPATHL, "%s%s", exp, p + 1);
- var = os_buf;
+ char *exp = os_getenv(os_buf);
+ if (exp != NULL) {
+ if (*exp != NUL && strlen(exp) + strlen(p) < MAXPATHL) {
+ vim_snprintf(os_buf, MAXPATHL, "%s%s", exp, p + 1);
+ var = os_buf;
+ }
+ xfree(exp);
}
}
}
@@ -498,6 +519,7 @@ void init_homedir(void)
if (var != NULL) {
homedir = xstrdup(var);
}
+ xfree(tofree);
}
static char homedir_buf[MAXPATHL];
@@ -523,17 +545,6 @@ void free_homedir(void)
xfree(homedir);
}
-void free_envmap(void)
-{
- cstr_t name;
- ptr_t e;
- map_foreach(&envmap, name, e, {
- xfree((char *)name);
- xfree(e);
- });
- map_destroy(cstr_t, &envmap);
-}
-
#endif
/// Call expand_env() and store the result in an allocated string.
@@ -917,9 +928,9 @@ char *vim_getenv(const char *name)
}
#endif
- const char *kos_env_path = os_getenv(name);
+ char *kos_env_path = os_getenv(name);
if (kos_env_path != NULL) {
- return xstrdup(kos_env_path);
+ return kos_env_path;
}
bool vimruntime = (strcmp(name, "VIMRUNTIME") == 0);
@@ -932,11 +943,13 @@ char *vim_getenv(const char *name)
char *vim_path = NULL;
if (vimruntime
&& *default_vimruntime_dir == NUL) {
- kos_env_path = os_getenv("VIM");
+ kos_env_path = os_getenv("VIM"); // kos_env_path was NULL.
if (kos_env_path != NULL) {
vim_path = vim_version_dir(kos_env_path);
if (vim_path == NULL) {
- vim_path = xstrdup(kos_env_path);
+ vim_path = kos_env_path;
+ } else {
+ xfree(kos_env_path);
}
}
}
@@ -1059,13 +1072,13 @@ size_t home_replace(const buf_T *const buf, const char *src, char *const dst, si
dirlen = strlen(homedir);
}
- const char *homedir_env = os_getenv("HOME");
+ char *homedir_env = os_getenv("HOME");
#ifdef MSWIN
if (homedir_env == NULL) {
homedir_env = os_getenv("USERPROFILE");
}
#endif
- char *homedir_env_mod = (char *)homedir_env;
+ char *homedir_env_mod = homedir_env;
bool must_free = false;
if (homedir_env_mod != NULL && *homedir_env_mod == '~') {
@@ -1142,6 +1155,8 @@ size_t home_replace(const buf_T *const buf, const char *src, char *const dst, si
*dst_p = NUL;
+ xfree(homedir_env);
+
if (must_free) {
xfree(homedir_env_mod);
}
@@ -1199,9 +1214,10 @@ bool os_setenv_append_path(const char *fname)
size_t dirlen = (size_t)(tail - fname);
assert(tail >= fname && dirlen + 1 < sizeof(os_buf));
xmemcpyz(os_buf, fname, dirlen);
- const char *path = os_getenv("PATH");
+ char *path = os_getenv("PATH");
const size_t pathlen = path ? strlen(path) : 0;
const size_t newlen = pathlen + dirlen + 2;
+ bool retval = false;
if (newlen < MAX_ENVPATHLEN) {
char *temp = xmalloc(newlen);
if (pathlen == 0) {
@@ -1215,9 +1231,10 @@ bool os_setenv_append_path(const char *fname)
xstrlcat(temp, os_buf, newlen);
os_setenv("PATH", temp, 1);
xfree(temp);
- return true;
+ retval = true;
}
- return false;
+ xfree(path);
+ return retval;
}
/// Returns true if `sh` looks like it resolves to "cmd.exe".
@@ -1228,7 +1245,7 @@ bool os_shell_is_cmdexe(const char *sh)
return false;
}
if (striequal(sh, "$COMSPEC")) {
- const char *comspec = os_getenv("COMSPEC");
+ char *comspec = os_getenv_noalloc("COMSPEC");
return striequal("cmd.exe", path_tail(comspec));
}
if (striequal(sh, "cmd.exe") || striequal(sh, "cmd")) {
diff --git a/src/nvim/os/fs.c b/src/nvim/os/fs.c
@@ -303,7 +303,7 @@ static bool is_executable_ext(const char *name, char **abspath)
size_t nameext_len = nameext ? strlen(nameext) : 0;
xstrlcpy(os_buf, name, sizeof(os_buf));
char *buf_end = xstrchrnul(os_buf, NUL);
- const char *pathext = os_getenv("PATHEXT");
+ const char *pathext = os_getenv_noalloc("PATHEXT");
if (!pathext) {
pathext = ".com;.exe;.bat;.cmd";
}
@@ -350,14 +350,14 @@ static bool is_executable_ext(const char *name, char **abspath)
static bool is_executable_in_path(const char *name, char **abspath)
FUNC_ATTR_NONNULL_ARG(1)
{
- const char *path_env = os_getenv("PATH");
+ char *path_env = os_getenv("PATH");
if (path_env == NULL) {
return false;
}
#ifdef MSWIN
char *path = NULL;
- if (!os_env_exists("NoDefaultCurrentDirectoryInExePath")) {
+ if (!os_env_exists("NoDefaultCurrentDirectoryInExePath", false)) {
// Prepend ".;" to $PATH.
size_t pathlen = strlen(path_env);
path = xmallocz(pathlen + 2);
@@ -404,6 +404,7 @@ static bool is_executable_in_path(const char *name, char **abspath)
end:
xfree(buf);
xfree(path);
+ xfree(path_env);
return rv;
}
diff --git a/src/nvim/os/lang.c b/src/nvim/os/lang.c
@@ -70,6 +70,7 @@ char *get_mess_lang(void)
}
/// Get the language used for messages from the environment.
+/// The function may be using NameBuff.
///
/// This uses LC_MESSAGES when available, which it is for most systems we build for
/// except for windows. Then fallback to get the value from the environment
@@ -79,17 +80,17 @@ static char *get_mess_env(void)
#ifdef LC_MESSAGES
return get_locale_val(LC_MESSAGES);
#else
- char *p = (char *)os_getenv("LC_ALL");
+ char *p = os_getenv_noalloc("LC_ALL");
if (p != NULL) {
return p;
}
- p = (char *)os_getenv("LC_MESSAGES");
+ p = os_getenv_noalloc("LC_MESSAGES");
if (p != NULL) {
return p;
}
- p = (char *)os_getenv("LANG");
+ p = os_getenv_noalloc("LANG");
if (p != NULL && ascii_isdigit(*p)) {
p = NULL; // ignore something like "1043"
}
@@ -340,7 +341,7 @@ char *get_locales(expand_T *xp, int idx)
void lang_init(void)
{
#ifdef __APPLE__
- if (os_getenv("LANG") == NULL) {
+ if (!os_env_exists("LANG", true)) {
char buf[50] = { 0 };
// $LANG is not set, either because it was unset or Nvim was started
diff --git a/src/nvim/os/os_win_console.c b/src/nvim/os/os_win_console.c
@@ -82,7 +82,7 @@ void os_icon_init(void)
hOrigIconSmall = (HICON)SendMessage(hWnd, WM_GETICON, (WPARAM)ICON_SMALL, (LPARAM)0);
hOrigIcon = (HICON)SendMessage(hWnd, WM_GETICON, (WPARAM)ICON_BIG, (LPARAM)0);
- const char *vimruntime = os_getenv("VIMRUNTIME");
+ char *vimruntime = os_getenv("VIMRUNTIME");
if (vimruntime != NULL) {
snprintf(NameBuff, MAXPATHL, "%s/neovim.ico", vimruntime);
if (!os_path_exists(NameBuff)) {
@@ -92,6 +92,7 @@ void os_icon_init(void)
LR_LOADFROMFILE | LR_LOADMAP3DCOLORS);
os_icon_set(hVimIcon, hVimIcon);
}
+ xfree(vimruntime);
}
}
@@ -117,7 +118,7 @@ void os_title_reset(void)
/// @param out_fd stdout file descriptor
void os_tty_guess_term(const char **term, int out_fd)
{
- bool conemu_ansi = strequal(os_getenv("ConEmuANSI"), "ON");
+ bool conemu_ansi = strequal(os_getenv_noalloc("ConEmuANSI"), "ON");
bool vtp = false;
HANDLE handle = (HANDLE)_get_osfhandle(out_fd);
diff --git a/src/nvim/os/stdpaths.c b/src/nvim/os/stdpaths.c
@@ -70,19 +70,19 @@ static const char *const xdg_defaults[] = {
/// @return $NVIM_APPNAME value
const char *get_appname(bool namelike)
{
- const char *env_val = os_getenv("NVIM_APPNAME");
- if (env_val == NULL || *env_val == NUL) {
- env_val = "nvim";
+ const char *env_val = os_getenv_noalloc("NVIM_APPNAME");
+
+ if (!env_val) {
+ xstrlcpy(NameBuff, "nvim", sizeof(NameBuff));
}
if (namelike) {
// Appname may be a relative path, replace slashes to make it name-like.
- xstrlcpy(NameBuff, env_val, sizeof(NameBuff));
memchrsub(NameBuff, '/', '-', sizeof(NameBuff));
memchrsub(NameBuff, '\\', '-', sizeof(NameBuff));
}
- return env_val;
+ return NameBuff;
}
/// Ensure that APPNAME is valid. Must be a name or relative path.
@@ -157,21 +157,21 @@ char *stdpaths_get_xdg_var(const XDGVarType idx)
const char *const env = xdg_env_vars[idx];
const char *const fallback = xdg_defaults[idx];
- const char *env_val = os_getenv(env);
+ char *env_val = os_getenv(env);
#ifdef MSWIN
if (env_val == NULL && xdg_defaults_env_vars[idx] != NULL) {
env_val = os_getenv(xdg_defaults_env_vars[idx]);
}
#else
- if (env_val == NULL && os_env_exists(env)) {
- env_val = "";
+ if (env_val == NULL && os_env_exists(env, false)) {
+ env_val = xstrdup("");
}
#endif
char *ret = NULL;
if (env_val != NULL) {
- ret = xstrdup(env_val);
+ ret = env_val;
} else if (fallback) {
ret = expand_env_save((char *)fallback);
} else if (idx == kXDGRuntimeDir) {
diff --git a/src/nvim/os/time.c b/src/nvim/os/time.c
@@ -98,8 +98,8 @@ struct tm *os_localtime_r(const time_t *restrict clock,
// Call tzset(3) to update the global timezone variables if it has.
// POSIX standard doesn't require localtime_r() implementations to do that
// as it does with localtime(), and we don't want to call tzset() every time.
- const char *tz = os_getenv("TZ");
- if (tz == NULL) {
+ const char *tz = os_getenv_noalloc("TZ");
+ if (!tz) {
tz = "";
}
if (strncmp(tz_cache, tz, sizeof(tz_cache) - 1) != 0) {
diff --git a/src/nvim/os/users.c b/src/nvim/os/users.c
@@ -89,7 +89,7 @@ int os_get_usernames(garray_T *users)
#endif
#ifdef HAVE_PWD_FUNCS
{
- const char *user_env = os_getenv("USER");
+ char *user_env = os_getenv_noalloc("USER");
// The $USER environment variable may be a valid remote user name (NIS,
// LDAP) not already listed by getpwent(), as getpwent() only lists
diff --git a/src/nvim/path.c b/src/nvim/path.c
@@ -2392,7 +2392,7 @@ bool path_is_absolute(const char *fname)
void path_guess_exepath(const char *argv0, char *buf, size_t bufsize)
FUNC_ATTR_NONNULL_ALL
{
- const char *path = os_getenv("PATH");
+ char *path = os_getenv("PATH");
if (path == NULL || path_is_absolute(argv0)) {
xstrlcpy(buf, argv0, bufsize);
@@ -2427,4 +2427,5 @@ void path_guess_exepath(const char *argv0, char *buf, size_t bufsize)
// Not found in $PATH, fall back to argv0.
xstrlcpy(buf, argv0, bufsize);
}
+ xfree(path);
}
diff --git a/src/nvim/runtime.c b/src/nvim/runtime.c
@@ -1746,8 +1746,7 @@ char *runtimepath_default(bool clean_arg)
size_t config_len = 0;
size_t vimruntime_len = 0;
size_t libdir_len = 0;
- const char *appname = get_appname(false);
- size_t appname_len = strlen(appname);
+ size_t appname_len = strlen(get_appname(false));
if (data_home != NULL) {
data_len = strlen(data_home);
size_t nvim_data_size = appname_len;
diff --git a/src/nvim/tui/input.c b/src/nvim/tui/input.c
@@ -139,7 +139,8 @@ void tinput_init(TermInput *input, Loop *loop)
input->in_fd = STDIN_FILENO;
- const char *term = os_getenv("TERM");
+ const char *term = os_getenv_noalloc("TERM");
+
if (!term) {
term = ""; // termkey_new_abstract assumes non-null (#2745)
}
diff --git a/src/nvim/tui/terminfo.c b/src/nvim/tui/terminfo.c
@@ -49,7 +49,7 @@ bool terminfo_is_bsd_console(const char *term)
# if defined(__FreeBSD__)
// FreeBSD console sets TERM=xterm, but it does not support xterm features
// like cursor-shaping. Assume that TERM=xterm is degraded. #8644
- return strequal(term, "xterm") && !!os_getenv("XTERM_VERSION");
+ return strequal(term, "xterm") && os_env_exists("XTERM_VERSION", true);
# endif
#endif
return false;
diff --git a/src/nvim/tui/tui.c b/src/nvim/tui/tui.c
@@ -379,7 +379,7 @@ static void terminfo_start(TUIData *tui)
tui->out_isatty = os_isatty(tui->out_fd);
tui->input.tui_data = tui;
- const char *term = os_getenv("TERM");
+ char *term = os_getenv("TERM");
#ifdef MSWIN
os_tty_guess_term(&term, tui->out_fd);
os_setenv("TERM", term, 1);
@@ -401,23 +401,25 @@ static void terminfo_start(TUIData *tui)
}
// None of the following work over SSH; see :help TERM .
- const char *colorterm = os_getenv("COLORTERM");
- const char *termprg = os_getenv("TERM_PROGRAM");
- const char *vte_version_env = os_getenv("VTE_VERSION");
+ char *colorterm = os_getenv("COLORTERM");
+ char *termprg = os_getenv("TERM_PROGRAM");
+ char *vte_version_env = os_getenv("VTE_VERSION");
+ char *konsolev_env = os_getenv("KONSOLE_VERSION");
+ char *term_program_version_env = os_getenv("TERM_PROGRAM_VERSION");
+
int vtev = vte_version_env ? (int)strtol(vte_version_env, NULL, 10) : 0;
bool iterm_env = termprg && strstr(termprg, "iTerm.app");
bool nsterm = (termprg && strstr(termprg, "Apple_Terminal"))
|| terminfo_is_term_family(term, "nsterm");
bool konsole = terminfo_is_term_family(term, "konsole")
- || os_getenv("KONSOLE_PROFILE_NAME")
- || os_getenv("KONSOLE_DBUS_SESSION");
- const char *konsolev_env = os_getenv("KONSOLE_VERSION");
+ || os_env_exists("KONSOLE_PROFILE_NAME", true)
+ || os_env_exists("KONSOLE_DBUS_SESSION", true);
int konsolev = konsolev_env ? (int)strtol(konsolev_env, NULL, 10)
: (konsole ? 1 : 0);
bool wezterm = strequal(termprg, "WezTerm");
- const char *weztermv = wezterm ? os_getenv("TERM_PROGRAM_VERSION") : NULL;
+ const char *weztermv = wezterm ? term_program_version_env : NULL;
bool screen = terminfo_is_term_family(term, "screen");
- bool tmux = terminfo_is_term_family(term, "tmux") || !!os_getenv("TMUX");
+ bool tmux = terminfo_is_term_family(term, "tmux") || os_env_exists("TMUX", true);
// truecolor support must be checked before patching/augmenting terminfo
tui->rgb = term_has_truecolor(tui, colorterm);
@@ -503,6 +505,13 @@ static void terminfo_start(TUIData *tui)
}
}
flush_buf(tui);
+
+ xfree(term);
+ xfree(colorterm);
+ xfree(termprg);
+ xfree(vte_version_env);
+ xfree(konsolev_env);
+ xfree(term_program_version_env);
}
/// Disable the alternate screen and prepare for the TUI to close.
@@ -1772,6 +1781,8 @@ void tui_guess_size(TUIData *tui)
{
int width = 0;
int height = 0;
+ char *lines = NULL;
+ char *columns = NULL;
// 1 - try from a system call (ioctl/TIOCGWINSZ on unix)
if (tui->out_isatty
@@ -1782,9 +1793,9 @@ void tui_guess_size(TUIData *tui)
// 2 - use $LINES/$COLUMNS if available
const char *val;
int advance;
- if ((val = os_getenv("LINES"))
+ if ((val = os_getenv_noalloc("LINES"))
&& sscanf(val, "%d%n", &height, &advance) != EOF && advance
- && (val = os_getenv("COLUMNS"))
+ && (val = os_getenv_noalloc("COLUMNS"))
&& sscanf(val, "%d%n", &width, &advance) != EOF && advance) {
goto end;
}
@@ -1804,6 +1815,9 @@ void tui_guess_size(TUIData *tui)
// Redraw on SIGWINCH event if size didn't change. #23411
ui_client_set_size(width, height);
+
+ xfree(lines);
+ xfree(columns);
}
static void unibi_goto(TUIData *tui, int row, int col)
@@ -1965,7 +1979,7 @@ static void patch_terminfo_bugs(TUIData *tui, const char *term, const char *colo
int vte_version, int konsolev, bool iterm_env, bool nsterm)
{
unibi_term *ut = tui->ut;
- const char *xterm_version = os_getenv("XTERM_VERSION");
+ char *xterm_version = os_getenv("XTERM_VERSION");
bool xterm = terminfo_is_term_family(term, "xterm")
// Treat Terminal.app as generic xterm-like, for now.
|| nsterm;
@@ -1977,7 +1991,7 @@ static void patch_terminfo_bugs(TUIData *tui, const char *term, const char *colo
bool teraterm = terminfo_is_term_family(term, "teraterm");
bool putty = terminfo_is_term_family(term, "putty");
bool screen = terminfo_is_term_family(term, "screen");
- bool tmux = terminfo_is_term_family(term, "tmux") || !!os_getenv("TMUX");
+ bool tmux = terminfo_is_term_family(term, "tmux") || os_env_exists("TMUX", true);
bool st = terminfo_is_term_family(term, "st");
bool gnome = terminfo_is_term_family(term, "gnome")
|| terminfo_is_term_family(term, "vte");
@@ -2276,6 +2290,8 @@ static void patch_terminfo_bugs(TUIData *tui, const char *term, const char *colo
"\x1b]50;\x07");
}
}
+
+ xfree(xterm_version);
}
/// This adds stuff that is not in standard terminfo as extended unibilium
@@ -2284,6 +2300,7 @@ static void augment_terminfo(TUIData *tui, const char *term, int vte_version, in
const char *weztermv, bool iterm_env, bool nsterm)
{
unibi_term *ut = tui->ut;
+ char *xterm_version = os_getenv("XTERM_VERSION");
bool xterm = terminfo_is_term_family(term, "xterm")
// Treat Terminal.app as generic xterm-like, for now.
|| nsterm;
@@ -2294,7 +2311,7 @@ static void augment_terminfo(TUIData *tui, const char *term, int vte_version, in
bool teraterm = terminfo_is_term_family(term, "teraterm");
bool putty = terminfo_is_term_family(term, "putty");
bool screen = terminfo_is_term_family(term, "screen");
- bool tmux = terminfo_is_term_family(term, "tmux") || !!os_getenv("TMUX");
+ bool tmux = terminfo_is_term_family(term, "tmux") || os_env_exists("TMUX", true);
bool st = terminfo_is_term_family(term, "st");
bool iterm = terminfo_is_term_family(term, "iterm")
|| terminfo_is_term_family(term, "iterm2")
@@ -2305,7 +2322,6 @@ static void augment_terminfo(TUIData *tui, const char *term, int vte_version, in
// None of the following work over SSH; see :help TERM .
bool iterm_pretending_xterm = xterm && iterm_env;
- const char *xterm_version = os_getenv("XTERM_VERSION");
bool true_xterm = xterm && !!xterm_version && !bsdvt;
// Only define this capability for terminal types that we know understand it.
@@ -2468,6 +2484,8 @@ static void augment_terminfo(TUIData *tui, const char *term, int vte_version, in
// Kitty keyboard protocol
tui->input.key_encoding = kKeyEncodingXterm;
}
+
+ xfree(xterm_version);
}
static bool should_invisible(TUIData *tui)
diff --git a/src/nvim/ui_client.c b/src/nvim/ui_client.c
@@ -63,7 +63,7 @@ uint64_t ui_client_start_server(int argc, char **argv)
#ifdef MSWIN
// TODO(justinmk): detach breaks `tt.setup_child_nvim` tests on Windows?
- bool detach = os_env_exists("__NVIM_DETACH");
+ bool detach = os_env_exists("__NVIM_DETACH", true);
#else
bool detach = true;
#endif
@@ -172,7 +172,7 @@ void ui_client_run(bool remote_ui)
ui_client_attach(width, height, term, rgb);
// TODO(justinmk): this is for log_spec. Can remove this after nvim_log #7062 is merged.
- if (os_env_exists("__NVIM_TEST_LOG")) {
+ if (os_env_exists("__NVIM_TEST_LOG", true)) {
ELOG("test log message");
}
diff --git a/test/functional/core/env_spec.lua b/test/functional/core/env_spec.lua
@@ -0,0 +1,27 @@
+local t = require('test.testutil')
+local n = require('test.functional.testnvim')()
+
+local eq = t.eq
+local clear = n.clear
+local exec_capture = n.exec_capture
+local command = n.command
+
+describe('vim.uv', function()
+ before_each(function()
+ clear()
+ end)
+
+ -- Subsequential env var assignment consistency
+ -- see: issue 32550
+ it('vim.uv.os_setenv(), vim.uv.os_unsetenv() consistency', function()
+ eq('', exec_capture('echo $FOO'))
+ command('lua vim.uv.os_setenv("FOO", "bar")')
+ eq('bar', exec_capture('echo $FOO'))
+ command('lua vim.uv.os_setenv("FOO", "fizz")')
+ eq('fizz', exec_capture('echo $FOO'))
+ command('lua vim.uv.os_unsetenv("FOO")')
+ eq('', exec_capture('echo $FOO'))
+ command('lua vim.uv.os_setenv("FOO", "buzz")')
+ eq('buzz', exec_capture('echo $FOO'))
+ end)
+end)
diff --git a/test/unit/os/env_spec.lua b/test/unit/os/env_spec.lua
@@ -13,8 +13,8 @@ local OK = 0
local cimp = cimport('./src/nvim/os/os.h')
describe('env.c', function()
- local function os_env_exists(name)
- return cimp.os_env_exists(to_cstr(name))
+ local function os_env_exists(name, nonempty)
+ return cimp.os_env_exists(to_cstr(name), nonempty)
end
local function os_setenv(name, value, override)
@@ -34,17 +34,45 @@ describe('env.c', function()
end
end
- itp('os_env_exists', function()
- eq(false, os_env_exists(''))
- eq(false, os_env_exists(' '))
- eq(false, os_env_exists('\t'))
- eq(false, os_env_exists('\n'))
- eq(false, os_env_exists('AaあB <= very weird name...'))
+ local function os_getenv_noalloc(name)
+ local rval = cimp.os_getenv_noalloc(to_cstr(name))
+ if rval ~= NULL then
+ return ffi.string(rval)
+ else
+ return NULL
+ end
+ end
+
+ itp('os_env_exists(..., false)', function()
+ eq(false, os_env_exists('', false))
+ eq(false, os_env_exists(' ', false))
+ eq(false, os_env_exists('\t', false))
+ eq(false, os_env_exists('\n', false))
+ eq(false, os_env_exists('AaあB <= very weird name...', false))
local varname = 'NVIM_UNIT_TEST_os_env_exists'
- eq(false, os_env_exists(varname))
+ eq(false, os_env_exists(varname, false))
+ eq(OK, os_setenv(varname, 'foo bar baz ...', 1))
+ eq(true, os_env_exists(varname, false))
+ eq(OK, os_setenv(varname, 'f', 1))
+ eq(true, os_env_exists(varname, true))
+ end)
+
+ itp('os_env_exists(..., true)', function()
+ eq(false, os_env_exists('', true))
+ eq(false, os_env_exists(' ', true))
+ eq(false, os_env_exists('\t', true))
+ eq(false, os_env_exists('\n', true))
+ eq(false, os_env_exists('AaあB <= very weird name...', true))
+
+ local varname = 'NVIM_UNIT_TEST_os_env_defined'
+ eq(false, os_env_exists(varname, true))
+ eq(OK, os_setenv(varname, '', 1))
+ eq(false, os_env_exists(varname, true))
eq(OK, os_setenv(varname, 'foo bar baz ...', 1))
- eq(true, os_env_exists(varname))
+ eq(true, os_env_exists(varname, true))
+ eq(OK, os_setenv(varname, 'f', 1))
+ eq(true, os_env_exists(varname, true))
end)
describe('os_setenv', function()
@@ -130,6 +158,10 @@ describe('env.c', function()
os_setenv(name, value, 1)
eq(value, os_getenv(name))
+ -- Shortest non-empty value
+ os_setenv(name, 'z', 1)
+ eq('z', os_getenv(name))
+
-- Get a big value.
local bigval = ('x'):rep(256)
eq(OK, os_setenv(name, bigval, 1))
@@ -147,6 +179,42 @@ describe('env.c', function()
end)
end)
+ describe('os_getenv_noalloc', function()
+ itp('reads an env var without memory allocation', function()
+ local name = 'NVIM_UNIT_TEST_GETENV_1N'
+ local value = 'NVIM_UNIT_TEST_GETENV_1V'
+ eq(NULL, os_getenv_noalloc(name))
+ -- Use os_setenv because Lua doesn't have setenv.
+ os_setenv(name, value, 1)
+ eq(value, os_getenv_noalloc(name))
+
+ -- Shortest non-empty value
+ os_setenv(name, 'z', 1)
+ eq('z', os_getenv(name))
+
+ local bigval = ('x'):rep(256)
+ eq(OK, os_setenv(name, bigval, 1))
+ eq(bigval, os_getenv_noalloc(name))
+
+ -- Variable size above NameBuff size gets truncated
+ -- This assumes MAXPATHL is 4096 bytes.
+ local verybigval = ('y'):rep(4096)
+ local trunc = string.sub(verybigval, 0, 4095)
+ eq(OK, os_setenv(name, verybigval, 1))
+ eq(trunc, os_getenv_noalloc(name))
+
+ -- Set non-empty, then set empty.
+ eq(OK, os_setenv(name, 'non-empty', 1))
+ eq('non-empty', os_getenv(name))
+ eq(OK, os_setenv(name, '', 1))
+ eq(NULL, os_getenv_noalloc(name))
+ end)
+
+ itp('returns NULL if the env var is not found', function()
+ eq(NULL, os_getenv_noalloc('NVIM_UNIT_TEST_GETENV_NOTFOUND'))
+ end)
+ end)
+
itp('os_unsetenv', function()
local name = 'TEST_UNSETENV'
local value = 'TESTVALUE'
@@ -156,7 +224,7 @@ describe('env.c', function()
-- Depending on the platform the var might be unset or set as ''
assert.True(os_getenv(name) == nil or os_getenv(name) == '')
if os_getenv(name) == nil then
- eq(false, os_env_exists(name))
+ eq(false, os_env_exists(name, false))
end
end)
