Skip to content

fix(OSQUERY-004): 2 review findings in process.cpp #44

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.

Sign up for GitHub

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

Draft
flamingo wants to merge 1 commit into
base: master
Choose a base branch
from
+5 −4
Draft

fix(OSQUERY-004): 2 review findings in process.cpp #44

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions osquery/process/posix/process.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,8 @@ std::shared_ptr<PlatformProcess> PlatformProcess::launchExtension(
::execve(arguments[0], argv, ::environ);

// Code should never reach this point
std::cerr << "Could not start extension process: " << exec_path
<< ", error: " << errno << std::endl;
LOG(ERROR) << "Could not start extension process: " << exec_path
<< ", error: " << errno;
::exit(EXIT_FAILURE);
return std::shared_ptr<PlatformProcess>();
}
Comment on lines 186 to 193

@flamingo flamingo Bot Aug 24, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 std::cerr used for diagnostic output instead of LOG() macros in process.cpp

In launchExtension() (osquery/process/posix/process.cpp), replaced std::cerr << "Could not start extension process: " << exec_path << ", error: " << errno << std::endl; with LOG(ERROR) << "Could not start extension process: " << exec_path << ", error: " << errno;, matching the existing LOG(ERROR) style used in launchWorker(). No other logic changed.

🤖 Prompt for AI agents 
In osquery/process/posix/process.cpp around line 195, review and complete this code-review fix: std::cerr used for diagnostic output instead of LOG() macros in process.cpp.
What the draft fix changed: In launchExtension() (osquery/process/posix/process.cpp), replaced `std::cerr << "Could not start extension process: " << exec_path << ", error: " << errno << std::endl;` with `LOG(ERROR) << "Could not start extension process: " << exec_path << ", error: " << errno;`, matching the existing LOG(ERROR) style used in launchWorker(). No other logic changed.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer

Expand Down Expand Up @@ -233,8 +233,8 @@ std::shared_ptr<PlatformProcess> PlatformProcess::launchTestPythonScript(
std::string args = boost::algorithm::join(args_array, " ");

@flamingo flamingo Bot Aug 24, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 std::cerr used for diagnostic output in launchTestPythonScript

In launchTestPythonScript() (osquery/process/posix/process.cpp), replaced std::cerr << "Could not start process at " << osquery_python_path << ", args: " << args << ", error: " << errno << std::endl; with LOG(ERROR) << "Could not start process at " << osquery_python_path << ", args: " << args << ", error: " << errno;. The <iostream> include was left in place since it is not strictly required to be removed by the finding and removing it would be a drive-by change beyond scope.

🤖 Prompt for AI agents 
In osquery/process/posix/process.cpp around line 233, review and complete this code-review fix: std::cerr used for diagnostic output in launchTestPythonScript.
What the draft fix changed: In launchTestPythonScript() (osquery/process/posix/process.cpp), replaced `std::cerr << "Could not start process at " << osquery_python_path << ", args: " << args << ", error: " << errno << std::endl;` with `LOG(ERROR) << "Could not start process at " << osquery_python_path << ", args: " << args << ", error: " << errno;`. The `<iostream>` include was left in place since it is not strictly required to be removed by the finding and removing it would be a drive-by change beyond scope.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer


// Should not reach this if everything executes correctly
std::cerr << "Could not start process at " << osquery_python_path
<< ", args: " << args << ", error: " << errno << std::endl;
LOG(ERROR) << "Could not start process at " << osquery_python_path
<< ", args: " << args << ", error: " << errno;
::exit(EXIT_FAILURE);
} else if (process_pid > 0) {
process.reset(new PlatformProcess(process_pid));
Expand All @@ -243,3 +243,4 @@ std::shared_ptr<PlatformProcess> PlatformProcess::launchTestPythonScript(
return process;
}
} // namespace osquery