MaskRay accepted this revision.
MaskRay added a comment.
LGTM. Some nits
================
Comment at: lldb/source/Host/posix/ProcessLauncherPosixFork.cpp:161
+ // stdin/stdout/stderr
+ if ((fd > 2) && !info.GetFileActionForFD(fd) && fd != error_fd)
+ files_to_close.push_back(fd);
----------------
drop parens around `fd > 2`
================
Comment at: lldb/source/Host/posix/ProcessLauncherPosixFork.cpp:164
+ }
+ for (auto &file_to_close : files_to_close)
+ close(file_to_close);
----------------
`auto &` -> int
================
Comment at: lldb/source/Host/posix/ProcessLauncherPosixFork.cpp:168
+ // /proc/self/fd didn't work - trying the slow way instead
+ for (int fd = 3; fd < sysconf(_SC_OPEN_MAX); ++fd)
+ if (!info.GetFileActionForFD(fd) && fd != error_fd) {
----------------
Cache `sysconf(_SC_OPEN_MAX)` to avoid repeated calls.
================
Comment at: lldb/source/Host/posix/ProcessLauncherPosixFork.cpp:169
+ for (int fd = 3; fd < sysconf(_SC_OPEN_MAX); ++fd)
+ if (!info.GetFileActionForFD(fd) && fd != error_fd) {
+ close(fd);
----------------
one-line simple statements don't need braces.
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105732/new/
https://reviews.llvm.org/D105732
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
