-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[NFC][lldb][windows] refactor the referencing of STARTUPINFOW #170669
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[NFC][lldb][windows] refactor the referencing of STARTUPINFOW #170669
Conversation
|
@llvm/pr-subscribers-lldb Author: Charles Zablit (charles-zablit) ChangesThis NFC patch refactors the referencing of the
This patch also removes C89 style variable declarations, one of which was unused. Full diff: https://github.com/llvm/llvm-project/pull/170669.diff 1 Files Affected:
diff --git a/lldb/source/Host/windows/ProcessLauncherWindows.cpp b/lldb/source/Host/windows/ProcessLauncherWindows.cpp
index f22a5ffefc19a..ac5383f20f58e 100644
--- a/lldb/source/Host/windows/ProcessLauncherWindows.cpp
+++ b/lldb/source/Host/windows/ProcessLauncherWindows.cpp
@@ -86,13 +86,9 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info,
Status &error) {
error.Clear();
- std::string executable;
STARTUPINFOEXW startupinfoex = {};
- STARTUPINFOW &startupinfo = startupinfoex.StartupInfo;
- PROCESS_INFORMATION pi = {};
-
- startupinfo.cb = sizeof(startupinfoex);
- startupinfo.dwFlags |= STARTF_USESTDHANDLES;
+ startupinfoex.StartupInfo.cb = sizeof(startupinfoex);
+ startupinfoex.StartupInfo.dwFlags |= STARTF_USESTDHANDLES;
HANDLE stdin_handle = GetStdioHandle(launch_info, STDIN_FILENO);
HANDLE stdout_handle = GetStdioHandle(launch_info, STDOUT_FILENO);
@@ -136,8 +132,8 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info,
getenv("LLDB_LAUNCH_INFERIORS_WITHOUT_CONSOLE");
if (hide_console_var &&
llvm::StringRef(hide_console_var).equals_insensitive("true")) {
- startupinfo.dwFlags |= STARTF_USESHOWWINDOW;
- startupinfo.wShowWindow = SW_HIDE;
+ startupinfoex.StartupInfo.dwFlags |= STARTF_USESHOWWINDOW;
+ startupinfoex.StartupInfo.wShowWindow = SW_HIDE;
}
DWORD flags = CREATE_NEW_CONSOLE | CREATE_UNICODE_ENVIRONMENT |
@@ -169,6 +165,8 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info,
llvm::ConvertUTF8toWide(launch_info.GetWorkingDirectory().GetPath(),
wworkingDirectory);
+ PROCESS_INFORMATION pi = {};
+
BOOL result = ::CreateProcessW(
wexecutable.c_str(), pwcommandLine, NULL, NULL,
/*bInheritHandles=*/!inherited_handles.empty(), flags, environment.data(),
@@ -198,21 +196,20 @@ llvm::ErrorOr<std::vector<HANDLE>> ProcessLauncherWindows::GetInheritedHandles(
const ProcessLaunchInfo &launch_info, STARTUPINFOEXW &startupinfoex,
HANDLE stdout_handle, HANDLE stderr_handle, HANDLE stdin_handle) {
std::vector<HANDLE> inherited_handles;
- STARTUPINFOW &startupinfo = startupinfoex.StartupInfo;
- startupinfo.hStdError =
+ startupinfoex.StartupInfo.hStdError =
stderr_handle ? stderr_handle : GetStdHandle(STD_ERROR_HANDLE);
- startupinfo.hStdInput =
+ startupinfoex.StartupInfo.hStdInput =
stdin_handle ? stdin_handle : GetStdHandle(STD_INPUT_HANDLE);
- startupinfo.hStdOutput =
+ startupinfoex.StartupInfo.hStdOutput =
stdout_handle ? stdout_handle : GetStdHandle(STD_OUTPUT_HANDLE);
- if (startupinfo.hStdError)
- inherited_handles.push_back(startupinfo.hStdError);
- if (startupinfo.hStdInput)
- inherited_handles.push_back(startupinfo.hStdInput);
- if (startupinfo.hStdOutput)
- inherited_handles.push_back(startupinfo.hStdOutput);
+ if (startupinfoex.StartupInfo.hStdError)
+ inherited_handles.push_back(startupinfoex.StartupInfo.hStdError);
+ if (startupinfoex.StartupInfo.hStdInput)
+ inherited_handles.push_back(startupinfoex.StartupInfo.hStdInput);
+ if (startupinfoex.StartupInfo.hStdOutput)
+ inherited_handles.push_back(startupinfoex.StartupInfo.hStdOutput);
for (size_t i = 0; i < launch_info.GetNumFileActions(); ++i) {
const FileAction *act = launch_info.GetFileActionAtIndex(i);
|
compnerd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! This feels a lot more cleaner to read.
|
Ran the full Windows test suite at desk, everything works ✅ |
This NFC patch refactors the referencing of the
STARTUPINFOWattribute inProcessLauncherWindows::LaunchProcess, making intent more explicit.STARTUPINFOW startupinfois now explicitly referenced from its parent to make it clear we are mutating it.This patch also removes C89 style variable declarations, one of which was unused.