[Lldb-commits] [PATCH] D105732: [lldb] Update logic to close inherited file descriptors.

[Fangrui Song via Phabricator via lldb-commits]

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