-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[lldb-dap] Do not take ownership of stdin. #133811
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
Conversation
There isn't any benefit to taking ownership of stdin and it may cause issues if `Transport` is dealloced.
@llvm/pr-subscribers-lldb Author: John Harrison (ashgti) ChangesThere isn't any benefit to taking ownership of stdin and it may cause issues if Full diff: https://github.com/llvm/llvm-project/pull/133811.diff 1 Files Affected:
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 062c3a5f989f3..a1d4f4f92e625 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -571,7 +571,7 @@ int main(int argc, char *argv[]) {
}
lldb::IOObjectSP input = std::make_shared<NativeFile>(
- fileno(stdin), File::eOpenOptionReadOnly, true);
+ fileno(stdin), File::eOpenOptionReadOnly, false);
lldb::IOObjectSP output = std::make_shared<NativeFile>(
stdout_fd, File::eOpenOptionWriteOnly, false);
|
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.
LGTM
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/15220 Here is the relevant piece of the build log for the reference
|
There isn't any benefit to taking ownership of stdin and it may cause issues if
Transport
is dealloced.