fix(logging): don't overwrite NameBuff (#38004)

src/nvim/log.c

@@ -333,8 +333,9 @@ static bool v_do_log_to_file(FILE *log_file, int log_level, const char *context,
   // Get a name for this Nvim instance.
   // TODO(justinmk): expose this as v:name ?
   if (regen) {
+    char parent_buf[MAXPATHL];
     // Parent servername ($NVIM).
-    const char *parent = path_tail(os_getenv_noalloc(ENV_NVIM));
+    const char *parent = path_tail(os_getenv_buf(ENV_NVIM, parent_buf, sizeof(parent_buf)));
     // Servername. Empty until starting=false.
     const char *serv = path_tail(get_vim_var_str(VV_SEND_SERVER));
     if (parent[0] != NUL) {

src/nvim/os/env.c

@@ -100,34 +100,49 @@ 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).
+/// Like getenv(), but stores the value in `buf` instead of allocating.
+/// Value is truncated if it exceeds `bufsize`.
+///
+/// @return `buf` on success, NULL on failure
 /// @see os_env_exists
-char *os_getenv_noalloc(const char *name)
+/// @see os_getenv_noalloc
+char *os_getenv_buf(const char *const name, char *const buf, const size_t bufsize)
   FUNC_ATTR_NONNULL_ALL
 {
   if (name[0] == NUL) {
     return NULL;
   }
 
-  size_t size = sizeof(NameBuff);
-  int r = uv_os_getenv(name, NameBuff, &size);
+  size_t size = bufsize;
+  int r = uv_os_getenv(name, buf, &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);
+      xmemcpyz(buf, e, MIN(bufsize, size) - 1);
     }
     xfree(e);
   }
 
-  if (r != 0 || size == 0 || NameBuff[0] == NUL) {
+  if (r != 0 || size == 0 || buf[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;
+  return buf;
+}
+
+/// Like getenv(), but use `NameBuff` instead of allocating.
+/// Value is truncated if it exceeds sizeof(NameBuff).
+///
+/// @return pointer to `NameBuff` on success, NULL on failure
+/// @see os_env_exists
+/// @see os_getenv_buf
+char *os_getenv_noalloc(const char *name)
+  FUNC_ATTR_NONNULL_ALL
+{
+  return os_getenv_buf(name, NameBuff, sizeof(NameBuff));
 }
 
 /// Returns true if environment variable `name` is defined (even if empty).

test/unit/os/env_spec.lua

@@ -34,6 +34,15 @@ describe('env.c', function()
     end
   end
 
+  local function os_getenv_buf(name, buf, bufsize)
+    local rval = cimp.os_getenv_buf(to_cstr(name), buf, bufsize)
+    if rval ~= NULL then
+      return ffi.string(rval)
+    else
+      return NULL
+    end
+  end
+
   local function os_getenv_noalloc(name)
     local rval = cimp.os_getenv_noalloc(to_cstr(name))
     if rval ~= NULL then
@@ -179,6 +188,29 @@ describe('env.c', function()
     end)
   end)
 
+  describe('os_getenv_buf', function()
+    itp('reads an env var into given buffer', function()
+      local name = 'NVIM_UNIT_TEST_GETENV_1N'
+      local value = 'NVIM_UNIT_TEST_GETENV_1V'
+      local bufsize = 200
+      local buf = cstr(bufsize, '')
+      eq(NULL, os_getenv_buf(name, buf, bufsize))
+      -- Use os_setenv because Lua doesn't have setenv.
+      os_setenv(name, value, 1)
+      eq(value, os_getenv_buf(name, buf, bufsize))
+
+      -- Shortest non-empty value
+      os_setenv(name, 'z', 1)
+      eq('z', os_getenv_buf(name, buf, bufsize))
+
+      -- Variable size above `bufsize` gets truncated
+      local verybigval = ('y'):rep(bufsize + 10)
+      local trunc = string.sub(verybigval, 0, bufsize - 1)
+      eq(OK, os_setenv(name, verybigval, 1))
+      eq(trunc, os_getenv_buf(name, buf, bufsize))
+    end)
+  end)
+
   describe('os_getenv_noalloc', function()
     itp('reads an env var without memory allocation', function()
       local name = 'NVIM_UNIT_TEST_GETENV_1N'
@@ -190,7 +222,7 @@ describe('env.c', function()
 
       -- Shortest non-empty value
       os_setenv(name, 'z', 1)
-      eq('z', os_getenv(name))
+      eq('z', os_getenv_noalloc(name))
 
       local bigval = ('x'):rep(256)
       eq(OK, os_setenv(name, bigval, 1))
@@ -205,7 +237,7 @@ describe('env.c', function()
 
       -- Set non-empty, then set empty.
       eq(OK, os_setenv(name, 'non-empty', 1))
-      eq('non-empty', os_getenv(name))
+      eq('non-empty', os_getenv_noalloc(name))
       eq(OK, os_setenv(name, '', 1))
       eq(NULL, os_getenv_noalloc(name))
     end)