From f46857d1df3419972344b005a1be32273749e91b Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Tue, 30 Jun 2026 15:31:35 -0400 Subject: [PATCH] refactor(os): deduplicate console/fd boilerplate #40509 Problem: Scattered, redundant boilerplate for Windows console/fd plumbing. Solution: - Add os_dup_cloexec() - Add os_reattach_console_stdio(). - Use os_open_conin_fd(). --- src/nvim/channel.c | 10 +++------- src/nvim/fileio.c | 11 ++++++----- src/nvim/main.c | 5 +---- src/nvim/os/fs.c | 16 +++++++++++++++- src/nvim/os/os_win_console.c | 18 +++++++++++++----- 5 files changed, 38 insertions(+), 22 deletions(-) diff --git a/src/nvim/channel.c b/src/nvim/channel.c index 2f9fddc7f7..6481b5da27 100644 --- a/src/nvim/channel.c +++ b/src/nvim/channel.c @@ -556,10 +556,8 @@ uint64_t channel_from_stdio(bool rpc, CallbackReader on_output, const char **err // Strangely, ConPTY doesn't work if stdin and stdout are pipes. So replace // stdin and stdout with CONIN$ and CONOUT$, respectively. if (embedded_mode && os_has_conpty_working()) { - stdin_dup_fd = os_dup(STDIN_FILENO); - os_set_cloexec(stdin_dup_fd); - stdout_dup_fd = os_dup(STDOUT_FILENO); - os_set_cloexec(stdout_dup_fd); + stdin_dup_fd = os_dup_cloexec(STDIN_FILENO); + stdout_dup_fd = os_dup_cloexec(STDOUT_FILENO); // :restart spawns a replacement server that must not borrow the parent // Nvim process console, because that parent process will soon exit. const bool restart_alloc_console = os_env_exists(ENV_RESTART_ALLOC_CONSOLE, true); @@ -577,9 +575,7 @@ uint64_t channel_from_stdio(bool rpc, CallbackReader on_output, const char **err ShowWindow(GetConsoleWindow(), SW_HIDE); } } - os_enable_ctrl_c(); - os_replace_stdin_to_conin(); - os_replace_stdout_and_stderr_to_conout(); + os_reattach_console_stdio(); } #else if (embedded_mode) { diff --git a/src/nvim/fileio.c b/src/nvim/fileio.c index 9aa2097665..77fdf5e0b8 100644 --- a/src/nvim/fileio.c +++ b/src/nvim/fileio.c @@ -84,6 +84,10 @@ # include #endif +#ifdef MSWIN +# include "nvim/os/os_win_console.h" +#endif + #ifdef OPEN_CHR_FILES # include "nvim/charset.h" #endif @@ -1696,11 +1700,8 @@ failed: // On Unix, use stderr for stdin, makes shell commands work. vim_ignored = dup(2); #else - // On Windows, use the console input handle for stdin. - HANDLE conin = CreateFile("CONIN$", GENERIC_READ | GENERIC_WRITE, - FILE_SHARE_READ, (LPSECURITY_ATTRIBUTES)NULL, - OPEN_EXISTING, 0, (HANDLE)NULL); - vim_ignored = _open_osfhandle((intptr_t)conin, _O_RDONLY); + // On Windows, use the console input handle (CONIN$) for stdin. + vim_ignored = os_open_conin_fd(); #endif } } diff --git a/src/nvim/main.c b/src/nvim/main.c index e975d1bc8a..39f986d86c 100644 --- a/src/nvim/main.c +++ b/src/nvim/main.c @@ -303,10 +303,7 @@ int main(int argc, char **argv) #ifdef MSWIN int startup_stderr_fd = -1; if (embedded_mode) { - startup_stderr_fd = os_dup(STDERR_FILENO); - if (startup_stderr_fd >= 0) { - os_set_cloexec(startup_stderr_fd); - } + startup_stderr_fd = os_dup_cloexec(STDERR_FILENO); } #endif diff --git a/src/nvim/os/fs.c b/src/nvim/os/fs.c index 77438ff8aa..39feb93786 100644 --- a/src/nvim/os/fs.c +++ b/src/nvim/os/fs.c @@ -554,6 +554,20 @@ os_dup_dup: return ret; } +/// Duplicates file descriptor `fd` and marks the copy close-on-exec (Unix) / non-inheritable +/// (Windows), so the duped fd is not leaked to spawned child processes. +/// +/// @return New file descriptor, or libuv error code (< 0) if the dup failed. +int os_dup_cloexec(const int fd) + FUNC_ATTR_WARN_UNUSED_RESULT +{ + int newfd = os_dup(fd); + if (newfd >= 0) { + os_set_cloexec(newfd); + } + return newfd; +} + /// Open the file descriptor for stdin. int os_open_stdin_fd(void) { @@ -564,7 +578,7 @@ int os_open_stdin_fd(void) stdin_dup_fd = os_dup(STDIN_FILENO); #ifdef MSWIN // Replace the original stdin with the console input handle. - os_replace_stdin_to_conin(); + os_redirect_stdin_to_conin(); #endif } return stdin_dup_fd; diff --git a/src/nvim/os/os_win_console.c b/src/nvim/os/os_win_console.c index 19870ec2b0..834a556e35 100644 --- a/src/nvim/os/os_win_console.c +++ b/src/nvim/os/os_win_console.c @@ -42,14 +42,14 @@ void os_clear_hwnd(void) hWnd = NULL; } -void os_replace_stdin_to_conin(void) +void os_redirect_stdin_to_conin(void) { close(STDIN_FILENO); const int conin_fd = os_open_conin_fd(); assert(conin_fd == STDIN_FILENO); } -void os_replace_stdout_and_stderr_to_conout(void) +void os_redirect_stdout_stderr_to_conout(void) { const HANDLE conout_handle = CreateFile("CONOUT$", @@ -66,6 +66,16 @@ void os_replace_stdout_and_stderr_to_conout(void) assert(conerr_fd == STDERR_FILENO); } +/// Points this process's stdio at the current console: enables Ctrl-C handling and replaces +/// stdin/stdout/stderr with CONIN$/CONOUT$. The caller must have already acquired a console +/// (via AttachConsole, AllocConsole, etc.). +void os_reattach_console_stdio(void) +{ + os_enable_ctrl_c(); + os_redirect_stdin_to_conin(); + os_redirect_stdout_stderr_to_conout(); +} + /// Detach from the current console and switch stdio to a hidden private one. /// /// Used when an embedded server must outlive its parent console, while keeping @@ -75,9 +85,7 @@ void os_swap_to_hidden_console(void) FreeConsole(); AllocConsole(); ShowWindow(GetConsoleWindow(), SW_HIDE); - os_enable_ctrl_c(); - os_replace_stdin_to_conin(); - os_replace_stdout_and_stderr_to_conout(); + os_reattach_console_stdio(); } /// Resets Windows console icon if we got an original one on startup.