view_compiler.cpp: clean up file descriptor handling
For file descriptors which are explicitly passed across an exec()
boundary, mark them as // NOLINT(android-cloexec-open). This suppresses
clang-tidy link checks (intended to be added as part of b/129350825)
Don't call close(STDOUT_FILENO). The dup2() syscall atomically closes
this for us, so we don't need to do it ourselves. Additionally, this
also fixes a race condition where another thread may call open() and get
an FD corresponding to STDOUT_FILENO, which we will then close as part
of the dup2() call. Removing the close() makes file descriptor handling
atomic.
Set O_CLOEXEC on outfd. The pre-duped file descriptor should have
O_CLOEXEC set on it. Calling dup2() will dup the file descriptor, but
NOT preserve the O_CLOEXEC flag. Quoting "man dup2":
The two file descriptors do not share file descriptor flags
(the close-on-exec flag). The close-on-exec flag (FD_CLOEXEC;
see fcntl(2)) for the duplicate descriptor is off.
If we don't set the O_CLOEXEC flag on outfd, we'll be leaking the file
descriptor twice, once as STDOUT_FILENO, and once as outfd. The second
leak is undesirable although harmless.
Additional cleanup: Add missing newlines at end of file.
Bug: 129350825
Test: compiles
Change-Id: Ic83ad72ef8a38106ad95ec0202c5c09c61fcf3e7
diff --git a/cmds/installd/view_compiler.cpp b/cmds/installd/view_compiler.cpp
index f1ac717..60d6492 100644
--- a/cmds/installd/view_compiler.cpp
+++ b/cmds/installd/view_compiler.cpp
@@ -45,7 +45,7 @@
// and pass file descriptors.
// Open input file
- unique_fd infd{open(apk_path, 0)};
+ unique_fd infd{open(apk_path, O_RDONLY)}; // NOLINT(android-cloexec-open)
if (infd.get() < 0) {
PLOG(ERROR) << "Could not open input file: " << apk_path;
return false;
@@ -53,7 +53,7 @@
// Set up output file. viewcompiler can't open outputs by fd, but it can write to stdout, so
// we close stdout and open it towards the right output.
- unique_fd outfd{open(out_dex_file, O_CREAT | O_TRUNC | O_WRONLY, 0644)};
+ unique_fd outfd{open(out_dex_file, O_CREAT | O_TRUNC | O_WRONLY | O_CLOEXEC, 0644)};
if (outfd.get() < 0) {
PLOG(ERROR) << "Could not open output file: " << out_dex_file;
return false;
@@ -62,10 +62,6 @@
PLOG(ERROR) << "Could not change output file permissions";
return false;
}
- if (close(STDOUT_FILENO) != 0) {
- PLOG(ERROR) << "Could not close stdout";
- return false;
- }
if (dup2(outfd, STDOUT_FILENO) < 0) {
PLOG(ERROR) << "Could not duplicate output file descriptor";
return false;
@@ -96,4 +92,4 @@
}
} // namespace installd
-} // namespace android
\ No newline at end of file
+} // namespace android
diff --git a/cmds/installd/view_compiler.h b/cmds/installd/view_compiler.h
index f7c6e57..aa141ca 100644
--- a/cmds/installd/view_compiler.h
+++ b/cmds/installd/view_compiler.h
@@ -26,4 +26,4 @@
} // namespace installd
} // namespace android
-#endif // VIEW_COMPILER_H_
\ No newline at end of file
+#endif // VIEW_COMPILER_H_