Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(508)

Unified Diff: base/files/file_util_posix.cc

Issue 2687713003: Stop base::OpenFile from leaking fds/handles into child procs. (Closed)
Patch Set: addressed review comments Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: base/files/file_util_posix.cc
diff --git a/base/files/file_util_posix.cc b/base/files/file_util_posix.cc
index 34a1c4d1e6e2481a953ad2fac5d30ddb124ca3cd..c2ccd39c5fb2803ef55b5ac7e577ddf3ba007fff 100644
--- a/base/files/file_util_posix.cc
+++ b/base/files/file_util_posix.cc
@@ -185,6 +185,20 @@ bool DetermineDevShmExecutable() {
#endif // defined(OS_LINUX)
#endif // !defined(OS_NACL_NONSFI)
+#if !defined(OS_MACOSX)
+// Appends |mode_char| to |mode| before the optional character set encoding; see
+// https://www.gnu.org/software/libc/manual/html_node/Opening-Streams.html for
+// details.
+std::string AppendModeCharacter(StringPiece mode, char mode_char) {
+ // Add |mode_char| immediately before the comma or at the end of the string.
Mark Mentovai 2017/02/11 00:52:29 This comment says basically the same thing as the
grt (UTC plus 2) 2017/02/13 07:53:28 Done.
+ std::string result(mode.as_string());
+ size_t comma_pos = result.find(',');
+ result.insert(comma_pos == std::string::npos ? result.length() : comma_pos, 1,
+ mode_char);
+ return result;
+}
+#endif
+
} // namespace
#if !defined(OS_NACL_NONSFI)
@@ -710,11 +724,29 @@ bool GetFileInfo(const FilePath& file_path, File::Info* results) {
#endif // !defined(OS_NACL_NONSFI)
FILE* OpenFile(const FilePath& filename, const char* mode) {
+ // 'e' is unconditionally added below, so be sure there is not one already
+ // present before a comma in |mode|.
+ DCHECK(
+ strchr(mode, 'e') == nullptr ||
+ (strchr(mode, ',') != nullptr && strchr(mode, 'e') > strchr(mode, ',')));
ThreadRestrictions::AssertIOAllowed();
FILE* result = NULL;
+#if defined(OS_MACOSX)
+ // macOS does not provide a mode character to set O_CLOEXEC; see
+ // https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man3/fopen.3.html.
+ const char* the_mode = mode;
+#else
+ std::string mode_with_e(AppendModeCharacter(mode, 'e'));
+ const char* the_mode = mode_with_e.c_str();
+#endif
do {
- result = fopen(filename.value().c_str(), mode);
+ result = fopen(filename.value().c_str(), the_mode);
} while (!result && errno == EINTR);
+#if defined(OS_MACOSX)
+ // Mark the descriptor as close-on-exec.
+ if (result)
+ SetCloseOnExec(fileno(result));
+#endif
return result;
}

Powered by Google App Engine
This is Rietveld 408576698