Revert "Make pidfile writing "more" atomic."
Xiyuan correctly points out that writing a pidfile by using rename(2)
doesn't work e.g. on cgroupfs. Given that we routinely write pids to
cgroupfs, revert this commit. The sync issues that this CL was supposed
to fix are covered by
https://android-review.googlesource.com/c/platform/external/minijail/+/950197.
Bug: crbug.com/970215
Test: Without the revert:
Test: 'minijail0 -f /sys/fs/cgroup/freezer/tasks -- /bin/true' fails.
Test: With the revert:
Test: 'minijail0 -f /sys/fs/cgroup/freezer/tasks -- /bin/true' succeeds.
This reverts commit 255a8e2879edb49f395f819e2f8590b40a8284ed.
Change-Id: I545c67a8b241a48466bab348b4e3f778600da7ab
diff --git a/system.c b/system.c
index eb37fdd..2dcf152 100644
--- a/system.c
+++ b/system.c
@@ -3,7 +3,6 @@
* found in the LICENSE file.
*/
-#define _GNU_SOURCE
#include "system.h"
#include <errno.h>
@@ -13,7 +12,6 @@
#include <pwd.h>
#include <stdbool.h>
#include <stdio.h>
-#include <stdlib.h>
#include <string.h>
#include <sys/ioctl.h>
#include <sys/prctl.h>
@@ -236,56 +234,23 @@
int write_pid_to_path(pid_t pid, const char *path)
{
- char *temp_path;
- int ret;
+ FILE *fp = fopen(path, "we");
- if (asprintf(&temp_path, "%s.XXXXXX", path) < 0) {
- warn("asprintf(temp_path) failed");
- return -ENOMEM;
- }
- int fd = mkstemp(temp_path);
- if (fd < 0) {
- pwarn("mkstemp(%s) failed", temp_path);
- ret = -errno;
- goto done;
- }
-
- FILE *fp = fdopen(fd, "we");
if (!fp) {
- pwarn("fdopen(fd, \"we\") failed");
- ret = -errno;
- close(fd);
- goto unlink_temp;
+ pwarn("failed to open '%s'", path);
+ return -errno;
}
if (fprintf(fp, "%d\n", (int)pid) < 0) {
/* fprintf(3) does not set errno on failure. */
warn("fprintf(%s) failed", path);
- ret = -1;
- fclose(fp);
- goto unlink_temp;
+ return -1;
}
if (fclose(fp)) {
pwarn("fclose(%s) failed", path);
- ret = -errno;
- /* Best effort. */
- close(fd);
- goto unlink_temp;
+ return -errno;
}
- if (rename(temp_path, path) < 0) {
- pwarn("rename(%s, %s) failed", temp_path, path);
- ret = -errno;
- goto unlink_temp;
- }
- ret = 0;
- goto done;
-
-unlink_temp:
- /* Best effort. */
- unlink(temp_path);
-done:
- free(temp_path);
- return ret;
+ return 0;
}
/*
diff --git a/system_unittest.cc b/system_unittest.cc
index dea3556..36d8dc3 100644
--- a/system_unittest.cc
+++ b/system_unittest.cc
@@ -203,7 +203,7 @@
TemporaryFile tmp;
ASSERT_TRUE(tmp.is_valid());
- ASSERT_EQ(0, write_pid_to_path(1234, tmp.path.c_str()));
+ EXPECT_EQ(0, write_pid_to_path(1234, tmp.path.c_str()));
FILE *fp = fopen(tmp.path.c_str(), "re");
EXPECT_NE(nullptr, fp);
char data[6] = {};
