dumpstate: enforce oneshot
Ensure the service exits after errors as well as after successful
finish as a oneshot service should.
While at it also move remove(file) to unlink(file) and fix an error
in checking the return value of unlink.
BUG: 123571915
Test: adb shell /data/nativetest64/dumpstate_smoke_test/dumpstate_smoke_test --gtest_filter=DumpstateBinderTest.*
Change-Id: I772b7981cd3b2f7c285ab980495d5539d57ebf46
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index 8bd834d..ae12ad9 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -157,7 +157,7 @@
}
static bool CopyFileToFd(const std::string& input_file, int out_fd) {
- MYLOGD("Going to copy bugreport file (%s) to %d\n", ds.path_.c_str(), out_fd);
+ MYLOGD("Going to copy file (%s) to %d\n", input_file.c_str(), out_fd);
// Obtain a handle to the source file.
android::base::unique_fd in_fd(OpenForRead(input_file));
@@ -165,14 +165,14 @@
if (CopyFile(in_fd.get(), out_fd)) {
return true;
}
- MYLOGE("Failed to copy zip file: %s\n", strerror(errno));
+ MYLOGE("Failed to copy file: %s\n", strerror(errno));
}
return false;
}
static bool UnlinkAndLogOnError(const std::string& file) {
- if (unlink(file.c_str()) != -1) {
- MYLOGE("Failed to remove file (%s): %s\n", file.c_str(), strerror(errno));
+ if (unlink(file.c_str())) {
+ MYLOGE("Failed to unlink file (%s): %s\n", file.c_str(), strerror(errno));
return false;
}
return true;
@@ -460,9 +460,7 @@
if (!ds.AddZipEntry("anrd_trace.txt", path)) {
MYLOGE("Unable to add anrd_trace file %s to zip file\n", path);
} else {
- if (remove(path)) {
- MYLOGE("Error removing anrd_trace file %s: %s", path, strerror(errno));
- }
+ android::os::UnlinkAndLogOnError(path);
return true;
}
} else {
@@ -1582,13 +1580,8 @@
for (int i = 0; i < NUM_OF_DUMPS; i++) {
paths.emplace_back(StringPrintf("%s/%s", ds.bugreport_internal_dir_.c_str(),
kDumpstateBoardFiles[i].c_str()));
- remover.emplace_back(android::base::make_scope_guard(std::bind(
- [](std::string path) {
- if (remove(path.c_str()) != 0 && errno != ENOENT) {
- MYLOGE("Could not remove(%s): %s\n", path.c_str(), strerror(errno));
- }
- },
- paths[i])));
+ remover.emplace_back(android::base::make_scope_guard(
+ std::bind([](std::string path) { android::os::UnlinkAndLogOnError(path); }, paths[i])));
}
sp<IDumpstateDevice> dumpstate_device(IDumpstateDevice::getService());
@@ -1749,9 +1742,7 @@
ds.zip_file.reset(nullptr);
MYLOGD("Removing temporary file %s\n", tmp_path_.c_str())
- if (remove(tmp_path_.c_str()) != 0) {
- MYLOGE("Failed to remove temporary file (%s): %s\n", tmp_path_.c_str(), strerror(errno));
- }
+ android::os::UnlinkAndLogOnError(tmp_path_);
return true;
}
@@ -2335,6 +2326,7 @@
register_sig_handler();
+ // TODO(b/111441001): maybe skip if already started?
if (options_->do_start_service) {
MYLOGI("Starting 'dumpstate' service\n");
android::status_t ret;
@@ -2481,15 +2473,24 @@
// Do an early return if there were errors. We make an exception for consent
// timing out because it's possible the user got distracted. In this case the
// bugreport is not shared but made available for manual retrieval.
+ MYLOGI("User denied consent. Returning\n");
return status;
}
- if (options_->screenshot_fd.get() != -1) {
+ if (options_->do_fb && options_->screenshot_fd.get() != -1) {
bool copy_succeeded = android::os::CopyFileToFd(screenshot_path_,
options_->screenshot_fd.get());
if (copy_succeeded) {
android::os::UnlinkAndLogOnError(screenshot_path_);
}
}
+ if (status == Dumpstate::RunStatus::USER_CONSENT_TIMED_OUT) {
+ MYLOGI(
+ "Did not receive user consent yet."
+ " Will not copy the bugreport artifacts to caller.\n");
+ // TODO(b/111441001):
+ // 1. cancel outstanding requests
+ // 2. check for result more frequently
+ }
}
/* vibrate a few but shortly times to let user know it's finished */