From 8cf28f66b0db9126e9962a7f1eed3fa271d55b4c Mon Sep 17 00:00:00 2001 From: Beena352 Date: Mon, 22 Dec 2025 17:37:50 -0800 Subject: [PATCH 1/4] Refactor wsladiag CLI parsing and fix shell PTY handling --- src/shared/inc/CommandLine.h | 12 +++- src/windows/wsladiag/wsladiag.cpp | 112 +++++++++++++++++++++--------- 2 files changed, 88 insertions(+), 36 deletions(-) diff --git a/src/shared/inc/CommandLine.h b/src/shared/inc/CommandLine.h index 9966c5e65..d53991846 100644 --- a/src/shared/inc/CommandLine.h +++ b/src/shared/inc/CommandLine.h @@ -307,7 +307,8 @@ class ArgumentParser public: #ifdef WIN32 - ArgumentParser(const std::wstring& CommandLine, LPCWSTR Name, int StartIndex = 1) : m_startIndex(StartIndex), m_name(Name) + ArgumentParser(const std::wstring& CommandLine, LPCWSTR Name, int StartIndex = 1, bool IgnoreUnknownArgs = false) : + m_startIndex(StartIndex), m_name(Name), m_ignoreUnknownArgs(IgnoreUnknownArgs) { m_argv.reset(CommandLineToArgvW(std::wstring(CommandLine).c_str(), &m_argc)); THROW_LAST_ERROR_IF(!m_argv); @@ -315,7 +316,8 @@ class ArgumentParser #else - ArgumentParser(int argc, const char* const* argv) : m_argc(argc), m_argv(argv), m_startIndex(1) + ArgumentParser(int argc, const char* const* argv, bool IgnoreUnknownArgs = false) : + m_argc(argc), m_argv(argv), m_startIndex(1), m_ignoreUnknownArgs(IgnoreUnknownArgs) { } @@ -429,6 +431,11 @@ class ArgumentParser if (!foundMatch) { + if (m_ignoreUnknownArgs) + { + break; + } + THROW_USER_ERROR(wsl::shared::Localization::MessageInvalidCommandLine(m_argv[i], m_name ? m_name : m_argv[0])); } @@ -535,6 +542,7 @@ class ArgumentParser int m_startIndex{}; const TChar* m_name{}; + bool m_ignoreUnknownArgs{false}; }; } // namespace wsl::shared diff --git a/src/windows/wsladiag/wsladiag.cpp b/src/windows/wsladiag/wsladiag.cpp index eef2f3150..23a00cce5 100644 --- a/src/windows/wsladiag/wsladiag.cpp +++ b/src/windows/wsladiag/wsladiag.cpp @@ -8,7 +8,7 @@ Module Name: Abstract: - Entry point for the wsladiag tool, performs WSL runtime initialization and parses --list/--help. + Entry point for the wsladiag tool, performs WSL runtime initialization and parses list/shell/help. --*/ @@ -42,9 +42,35 @@ static int ReportError(const std::wstring& context, HRESULT hr) return 1; } -// Handler for `wsladiag shell ` (TTY-backed interactive shell). -static int RunShellCommand(const std::wstring& sessionName, bool verbose) +// Handler for `wsladiag shell [--verbose]` command - launches TTY-backed interactive shell. +static int RunShellCommand(std::wstring_view commandLine) { + std::wstring sessionName; + bool verbose = false; + + ArgumentParser parser(std::wstring{commandLine}, L"wsladiag", 2); // Skip "wsladiag.exe shell" to parse shell-specific args + parser.AddPositionalArgument(sessionName, 0); + parser.AddArgument(verbose, L"--verbose", L'v'); + + try + { + parser.Parse(); + } + catch (...) + { + const auto hr = wil::ResultFromCaughtException(); + if (hr == E_INVALIDARG) + { + return 1; + } + throw; + } + + if (sessionName.empty()) + { + return 1; + } + const auto log = [&](std::wstring_view msg) { if (verbose) { @@ -85,9 +111,11 @@ static int RunShellCommand(const std::wstring& sessionName, bool verbose) wsl::windows::common::WSLAProcessLauncher launcher{ shell, {shell, "--login"}, {"TERM=xterm-256color"}, wsl::windows::common::ProcessFlags::None}; - launcher.AddFd(WSLA_PROCESS_FD{.Fd = 0, .Type = WSLAFdTypeTerminalInput}); - launcher.AddFd(WSLA_PROCESS_FD{.Fd = 1, .Type = WSLAFdTypeTerminalOutput}); - launcher.AddFd(WSLA_PROCESS_FD{.Fd = 2, .Type = WSLAFdTypeTerminalControl}); + launcher.AddFd(WSLA_PROCESS_FD{.Fd = 0, .Type = WSLAFdTypeTerminalInput, .Path = nullptr}); + launcher.AddFd(WSLA_PROCESS_FD{.Fd = 1, .Type = WSLAFdTypeTerminalOutput, .Path = nullptr}); + launcher.AddFd(WSLA_PROCESS_FD{.Fd = 2, .Type = WSLAFdTypeTerminalControl, .Path = nullptr}); + + log(std::format(L"[diag] tty rows={} cols={}", rows, cols)); launcher.SetTtySize(rows, cols); log(L"[diag] launching shell process..."); @@ -96,7 +124,6 @@ static int RunShellCommand(const std::wstring& sessionName, bool verbose) auto ttyIn = process.GetStdHandle(0); auto ttyOut = process.GetStdHandle(1); - auto ttyControl = process.GetStdHandle(2); // Console handles. wil::unique_hfile conin{ @@ -119,11 +146,11 @@ static int RunShellCommand(const std::wstring& sessionName, bool verbose) const UINT originalOutCP = GetConsoleOutputCP(); const UINT originalInCP = GetConsoleCP(); - auto restoreConsole = wil::scope_exit([&] { - SetConsoleMode(consoleIn, originalInMode); - SetConsoleMode(consoleOut, originalOutMode); - SetConsoleOutputCP(originalOutCP); - SetConsoleCP(originalInCP); + auto restoreConsole = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&] { + LOG_IF_WIN32_BOOL_FALSE(SetConsoleMode(consoleIn, originalInMode)); + LOG_IF_WIN32_BOOL_FALSE(SetConsoleMode(consoleOut, originalOutMode)); + LOG_IF_WIN32_BOOL_FALSE(SetConsoleOutputCP(originalOutCP)); + LOG_IF_WIN32_BOOL_FALSE(SetConsoleCP(originalInCP)); }); // Console mode for interactive terminal. @@ -139,23 +166,46 @@ static int RunShellCommand(const std::wstring& sessionName, bool verbose) THROW_LAST_ERROR_IF(!SetConsoleOutputCP(CP_UTF8)); THROW_LAST_ERROR_IF(!SetConsoleCP(CP_UTF8)); - // Keep terminal control socket alive. auto exitEvent = wil::unique_event(wil::EventOptions::ManualReset); - wsl::shared::SocketChannel controlChannel{wil::unique_socket{(SOCKET)ttyControl.release()}, "TerminalControl", exitEvent.get()}; + + std::optional controlChannel; + try + { + auto ttyControl = process.GetStdHandle(2); // only once + controlChannel.emplace(wil::unique_socket{(SOCKET)ttyControl.release()}, "TerminalControl", exitEvent.get()); + } + catch (const wil::ResultException& e) + { + const HRESULT hr = e.GetErrorCode(); + if (hr == HRESULT_FROM_WIN32(ERROR_NOT_FOUND)) + { + log(L"[diag] TerminalControl not available; live resize disabled"); + } + else + { + throw; // or ReportError + return + } + } auto updateTerminalSize = [&]() { + if (!controlChannel.has_value()) + { + return; + } + CONSOLE_SCREEN_BUFFER_INFOEX infoEx{}; infoEx.cbSize = sizeof(infoEx); THROW_IF_WIN32_BOOL_FALSE(GetConsoleScreenBufferInfoEx(consoleOut, &infoEx)); WSLA_TERMINAL_CHANGED message{}; - message.Columns = infoEx.srWindow.Right - infoEx.srWindow.Left + 1; - message.Rows = infoEx.srWindow.Bottom - infoEx.srWindow.Top + 1; + message.Columns = static_cast(infoEx.srWindow.Right - infoEx.srWindow.Left + 1); + message.Rows = static_cast(infoEx.srWindow.Bottom - infoEx.srWindow.Top + 1); - controlChannel.SendMessage(message); + controlChannel->SendMessage(message); }; - // Relay console -> tty input. + // Start input relay thread to forward console input to TTY + // Runs in parallel with output relay (main thread) std::thread inputThread([&] { try { @@ -178,16 +228,19 @@ static int RunShellCommand(const std::wstring& sessionName, bool verbose) // Relay tty output -> console (blocks until output ends). wsl::windows::common::relay::InterruptableRelay(ttyOut.get(), consoleOut, exitEvent.get()); + // Output relay finished - signal input thread to stop and wait for process exit + exitEvent.SetEvent(); process.GetExitEvent().wait(); - auto [code, signalled] = process.GetExitState(); + + auto [exitCode, signalled] = process.GetExitState(); std::wstring shellWide(shell.begin(), shell.end()); - wslutil::PrintMessage(std::format(L"{} exited with: {}{}", shellWide, code, signalled ? L" (signalled)" : L""), stdout); + wslutil::PrintMessage(std::format(L"{} exited with: {}{}", shellWide, exitCode, signalled ? L" (signalled)" : L""), stdout); return 0; } -static int RunListCommand(bool /*verbose*/) +static int RunListCommand() { wil::com_ptr userSession; THROW_IF_FAILED(CoCreateInstance(__uuidof(WSLAUserSession), nullptr, CLSCTX_LOCAL_SERVER, IID_PPV_ARGS(&userSession))); @@ -257,17 +310,13 @@ int wsladiag_main(std::wstring_view commandLine) THROW_IF_WIN32_ERROR(WSAStartup(MAKEWORD(2, 2), &data)); auto wsaCleanup = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, []() { WSACleanup(); }); - ArgumentParser parser(std::wstring{commandLine}, L"wsladiag"); + ArgumentParser parser(std::wstring{commandLine}, L"wsladiag", 1, true); bool help = false; - bool verbose = false; std::wstring verb; - std::wstring shellSession; - parser.AddPositionalArgument(verb, 0); // "list" or "shell" - parser.AddPositionalArgument(shellSession, 1); // session name for "shell" + parser.AddPositionalArgument(verb, 0); parser.AddArgument(help, L"--help", L'h'); - parser.AddArgument(verbose, L"--verbose", L'v'); auto printUsage = []() { wslutil::PrintMessage( @@ -301,16 +350,11 @@ int wsladiag_main(std::wstring_view commandLine) } else if (verb == L"list") { - return RunListCommand(verbose); + return RunListCommand(); } else if (verb == L"shell") { - if (shellSession.empty()) - { - printUsage(); - return 1; - } - return RunShellCommand(shellSession, verbose); + return RunShellCommand(commandLine); } else { From 5ec6d87aa134c67550268e2f549776ad855a38ef Mon Sep 17 00:00:00 2001 From: Beena352 Date: Tue, 23 Dec 2025 10:22:20 -0800 Subject: [PATCH 2/4] Rename ArgumentParser flag to stopOnUnknownArgs --- src/shared/inc/CommandLine.h | 8 +-- src/windows/wsladiag/wsladiag.cpp | 90 +++++++++---------------------- 2 files changed, 28 insertions(+), 70 deletions(-) diff --git a/src/shared/inc/CommandLine.h b/src/shared/inc/CommandLine.h index d53991846..bc242ecd5 100644 --- a/src/shared/inc/CommandLine.h +++ b/src/shared/inc/CommandLine.h @@ -308,7 +308,7 @@ class ArgumentParser #ifdef WIN32 ArgumentParser(const std::wstring& CommandLine, LPCWSTR Name, int StartIndex = 1, bool IgnoreUnknownArgs = false) : - m_startIndex(StartIndex), m_name(Name), m_ignoreUnknownArgs(IgnoreUnknownArgs) + m_startIndex(StartIndex), m_name(Name), m_stopUnknownArgs(IgnoreUnknownArgs) { m_argv.reset(CommandLineToArgvW(std::wstring(CommandLine).c_str(), &m_argc)); THROW_LAST_ERROR_IF(!m_argv); @@ -317,7 +317,7 @@ class ArgumentParser #else ArgumentParser(int argc, const char* const* argv, bool IgnoreUnknownArgs = false) : - m_argc(argc), m_argv(argv), m_startIndex(1), m_ignoreUnknownArgs(IgnoreUnknownArgs) + m_argc(argc), m_argv(argv), m_startIndex(1), m_stopUnknownArgs(IgnoreUnknownArgs) { } @@ -431,7 +431,7 @@ class ArgumentParser if (!foundMatch) { - if (m_ignoreUnknownArgs) + if (m_stopUnknownArgs) { break; } @@ -542,7 +542,7 @@ class ArgumentParser int m_startIndex{}; const TChar* m_name{}; - bool m_ignoreUnknownArgs{false}; + bool m_stopUnknownArgs{false}; }; } // namespace wsl::shared diff --git a/src/windows/wsladiag/wsladiag.cpp b/src/windows/wsladiag/wsladiag.cpp index 23a00cce5..d785d6435 100644 --- a/src/windows/wsladiag/wsladiag.cpp +++ b/src/windows/wsladiag/wsladiag.cpp @@ -52,23 +52,11 @@ static int RunShellCommand(std::wstring_view commandLine) parser.AddPositionalArgument(sessionName, 0); parser.AddArgument(verbose, L"--verbose", L'v'); - try - { - parser.Parse(); - } - catch (...) - { - const auto hr = wil::ResultFromCaughtException(); - if (hr == E_INVALIDARG) - { - return 1; - } - throw; - } + parser.Parse(); if (sessionName.empty()) { - return 1; + THROW_HR(E_INVALIDARG); } const auto log = [&](std::wstring_view msg) { @@ -168,31 +156,10 @@ static int RunShellCommand(std::wstring_view commandLine) auto exitEvent = wil::unique_event(wil::EventOptions::ManualReset); - std::optional controlChannel; - try - { - auto ttyControl = process.GetStdHandle(2); // only once - controlChannel.emplace(wil::unique_socket{(SOCKET)ttyControl.release()}, "TerminalControl", exitEvent.get()); - } - catch (const wil::ResultException& e) - { - const HRESULT hr = e.GetErrorCode(); - if (hr == HRESULT_FROM_WIN32(ERROR_NOT_FOUND)) - { - log(L"[diag] TerminalControl not available; live resize disabled"); - } - else - { - throw; // or ReportError + return - } - } + auto ttyControl = process.GetStdHandle(2); // TerminalControl + wsl::shared::SocketChannel controlChannel{wil::unique_socket{(SOCKET)ttyControl.release()}, "TerminalControl", exitEvent.get()}; auto updateTerminalSize = [&]() { - if (!controlChannel.has_value()) - { - return; - } - CONSOLE_SCREEN_BUFFER_INFOEX infoEx{}; infoEx.cbSize = sizeof(infoEx); THROW_IF_WIN32_BOOL_FALSE(GetConsoleScreenBufferInfoEx(consoleOut, &infoEx)); @@ -201,7 +168,7 @@ static int RunShellCommand(std::wstring_view commandLine) message.Columns = static_cast(infoEx.srWindow.Right - infoEx.srWindow.Left + 1); message.Rows = static_cast(infoEx.srWindow.Bottom - infoEx.srWindow.Top + 1); - controlChannel->SendMessage(message); + controlChannel.SendMessage(message); }; // Start input relay thread to forward console input to TTY @@ -228,8 +195,6 @@ static int RunShellCommand(std::wstring_view commandLine) // Relay tty output -> console (blocks until output ends). wsl::windows::common::relay::InterruptableRelay(ttyOut.get(), consoleOut, exitEvent.get()); - // Output relay finished - signal input thread to stop and wait for process exit - exitEvent.SetEvent(); process.GetExitEvent().wait(); auto [exitCode, signalled] = process.GetExitState(); @@ -293,6 +258,17 @@ static int RunListCommand() return 0; } +static void PrintUsage() +{ + wslutil::PrintMessage( + L"wsladiag - WSLA diagnostics tool\n" + L"Usage:\n" + L" wsladiag list\n" + L" wsladiag shell [--verbose]\n" + L" wsladiag --help\n", + stderr); +} + int wsladiag_main(std::wstring_view commandLine) { wslutil::ConfigureCrt(); @@ -318,34 +294,11 @@ int wsladiag_main(std::wstring_view commandLine) parser.AddPositionalArgument(verb, 0); parser.AddArgument(help, L"--help", L'h'); - auto printUsage = []() { - wslutil::PrintMessage( - L"wsladiag - WSLA diagnostics tool\n" - L"Usage:\n" - L" wsladiag list\n" - L" wsladiag shell [--verbose]\n" - L" wsladiag --help\n", - stderr); - }; - - try - { - parser.Parse(); - } - catch (...) - { - const auto hr = wil::ResultFromCaughtException(); - if (hr == E_INVALIDARG) - { - printUsage(); - return 1; - } - throw; - } + parser.Parse(); // Let exceptions propagate to wmain for centralized handling if (help || verb.empty()) { - printUsage(); + PrintUsage(); return 0; } else if (verb == L"list") @@ -359,7 +312,7 @@ int wsladiag_main(std::wstring_view commandLine) else { wslutil::PrintMessage(std::format(L"Unknown command: '{}'", verb), stderr); - printUsage(); + PrintUsage(); return 1; } } @@ -373,6 +326,11 @@ int wmain(int, wchar_t**) catch (...) { const auto hr = wil::ResultFromCaughtException(); + if (hr == E_INVALIDARG) + { + PrintUsage(); + return 1; + } return ReportError(L"wsladiag failed", hr); } } From dbc865fa5683aba6e91106fe2d5077abe688e658 Mon Sep 17 00:00:00 2001 From: Beena352 Date: Wed, 24 Dec 2025 10:35:20 -0800 Subject: [PATCH 3/4] Rename ArgumentParser constructor flag to stopOnUnknownArgs --- src/shared/inc/CommandLine.h | 10 +++++----- src/windows/wsladiag/wsladiag.cpp | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/shared/inc/CommandLine.h b/src/shared/inc/CommandLine.h index bc242ecd5..3ef367ed4 100644 --- a/src/shared/inc/CommandLine.h +++ b/src/shared/inc/CommandLine.h @@ -307,8 +307,8 @@ class ArgumentParser public: #ifdef WIN32 - ArgumentParser(const std::wstring& CommandLine, LPCWSTR Name, int StartIndex = 1, bool IgnoreUnknownArgs = false) : - m_startIndex(StartIndex), m_name(Name), m_stopUnknownArgs(IgnoreUnknownArgs) + ArgumentParser(const std::wstring& CommandLine, LPCWSTR Name, int StartIndex = 1, bool stopUnknownArgs = false) : + m_startIndex(StartIndex), m_name(Name), m_stopUnknownArgs(stopUnknownArgs) { m_argv.reset(CommandLineToArgvW(std::wstring(CommandLine).c_str(), &m_argc)); THROW_LAST_ERROR_IF(!m_argv); @@ -316,8 +316,8 @@ class ArgumentParser #else - ArgumentParser(int argc, const char* const* argv, bool IgnoreUnknownArgs = false) : - m_argc(argc), m_argv(argv), m_startIndex(1), m_stopUnknownArgs(IgnoreUnknownArgs) + ArgumentParser(int argc, const char* const* argv, bool stopUnknownArgs = false) : + m_argc(argc), m_argv(argv), m_startIndex(1), m_stopUnknownArgs(stopUnknownArgs) { } @@ -402,7 +402,7 @@ class ArgumentParser const TChar* value = nullptr; if (e.Positional) { - value = m_argv[i]; // Positional arguments directly receive arvg[i] + value = m_argv[i]; // Positional arguments directly receive argv[i] } else if (i + 1 < m_argc) { diff --git a/src/windows/wsladiag/wsladiag.cpp b/src/windows/wsladiag/wsladiag.cpp index d785d6435..36fa5db7e 100644 --- a/src/windows/wsladiag/wsladiag.cpp +++ b/src/windows/wsladiag/wsladiag.cpp @@ -165,8 +165,8 @@ static int RunShellCommand(std::wstring_view commandLine) THROW_IF_WIN32_BOOL_FALSE(GetConsoleScreenBufferInfoEx(consoleOut, &infoEx)); WSLA_TERMINAL_CHANGED message{}; - message.Columns = static_cast(infoEx.srWindow.Right - infoEx.srWindow.Left + 1); - message.Rows = static_cast(infoEx.srWindow.Bottom - infoEx.srWindow.Top + 1); + message.Columns = infoEx.srWindow.Right - infoEx.srWindow.Left + 1; + message.Rows = infoEx.srWindow.Bottom - infoEx.srWindow.Top + 1; controlChannel.SendMessage(message); }; From 2700c392d0e8069aaf7e2373a6253a48132809fe Mon Sep 17 00:00:00 2001 From: Beena352 Date: Fri, 2 Jan 2026 13:38:47 -0800 Subject: [PATCH 4/4] Remove E_INVALIDARG special-casing in wmain error handling --- src/windows/wsladiag/wsladiag.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/windows/wsladiag/wsladiag.cpp b/src/windows/wsladiag/wsladiag.cpp index 36fa5db7e..48af792a7 100644 --- a/src/windows/wsladiag/wsladiag.cpp +++ b/src/windows/wsladiag/wsladiag.cpp @@ -326,11 +326,6 @@ int wmain(int, wchar_t**) catch (...) { const auto hr = wil::ResultFromCaughtException(); - if (hr == E_INVALIDARG) - { - PrintUsage(); - return 1; - } return ReportError(L"wsladiag failed", hr); } }