Minor dumpstate refactorings:
- Make some Dumpstate functions const.
- Renamed suffix_ to name_.
BUG: 26379932
Test: DumpstateTest passes
Change-Id: I324a6d63393f51ce27b2e95ce0e23296d97c328d
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index a48f112..43fc5d7 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -672,7 +672,7 @@
/* End copy from system/core/logd/LogBuffer.cpp */
// TODO: move to utils.cpp
-void Dumpstate::PrintHeader() {
+void Dumpstate::PrintHeader() const {
std::string build, fingerprint, radio, bootloader, network;
char date[80];
@@ -696,7 +696,7 @@
printf("Network: %s\n", network.c_str());
printf("Kernel: ");
- DumpFile("", "/proc/version");
+ JustDumpFile("", "/proc/version");
printf("Command line: %s\n", strtok(cmdline_buf, "\n"));
printf("Bugreport format version: %s\n", version_.c_str());
printf("Dumpstate info: id=%lu pid=%d dryRun=%d args=%s extraOptions=%s\n", id_, getpid(),
@@ -1430,9 +1430,9 @@
if (do_add_date) {
char date[80];
strftime(date, sizeof(date), "%Y-%m-%d-%H-%M-%S", localtime(&ds.now_));
- ds.suffix_ = date;
+ ds.name_ = date;
} else {
- ds.suffix_ = "undated";
+ ds.name_ = "undated";
}
std::string buildId = android::base::GetProperty("ro.build.id", "UNKNOWN_BUILD");
ds.baseName_ = ds.baseName_ + "-" + buildId;
@@ -1449,7 +1449,7 @@
"Log path: %s\n"
"Temporary path: %s\n"
"Screenshot path: %s\n",
- ds.bugreportDir_.c_str(), ds.baseName_.c_str(), ds.suffix_.c_str(), log_path.c_str(),
+ ds.bugreportDir_.c_str(), ds.baseName_.c_str(), ds.name_.c_str(), log_path.c_str(),
tmp_path.c_str(), ds.screenshotPath_.c_str());
if (do_zip_file) {
@@ -1471,7 +1471,7 @@
// clang-format off
std::vector<std::string> am_args = {
"--receiver-permission", "android.permission.DUMP", "--receiver-foreground",
- "--es", "android.intent.extra.NAME", ds.suffix_,
+ "--es", "android.intent.extra.NAME", ds.name_,
"--ei", "android.intent.extra.ID", std::to_string(ds.id_),
"--ei", "android.intent.extra.PID", std::to_string(getpid()),
"--ei", "android.intent.extra.MAX", std::to_string(WEIGHT_TOTAL),
@@ -1604,8 +1604,8 @@
}
}
if (change_suffix) {
- MYLOGI("changing suffix from %s to %s\n", ds.suffix_.c_str(), name.c_str());
- ds.suffix_ = name;
+ MYLOGI("changing suffix from %s to %s\n", ds.name_.c_str(), name.c_str());
+ ds.name_ = name;
if (!ds.screenshotPath_.empty()) {
std::string newScreenshotPath = ds.GetPath(".png");
if (rename(ds.screenshotPath_.c_str(), newScreenshotPath.c_str())) {
@@ -1619,7 +1619,7 @@
bool do_text_file = true;
if (do_zip_file) {
- std::string entry_name = ds.baseName_ + "-" + ds.suffix_ + ".txt";
+ std::string entry_name = ds.baseName_ + "-" + ds.name_ + ".txt";
MYLOGD("Adding main entry (%s) to .zip bugreport\n", entry_name.c_str());
if (!finish_zip_file(entry_name, tmp_path, log_path)) {
MYLOGE("Failed to finish zip file; sending text bugreport instead\n");
diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h
index 54d91f7..7e236f1 100644
--- a/cmds/dumpstate/dumpstate.h
+++ b/cmds/dumpstate/dumpstate.h
@@ -218,12 +218,12 @@
*
* Dry-run mode is enabled by setting the system property dumpstate.dry_run to true.
*/
- bool IsDryRun();
+ bool IsDryRun() const;
/*
* Gets whether device is running a `user` build.
*/
- bool IsUserBuild();
+ bool IsUserBuild() const;
/*
* Forks a command, waits for it to finish, and returns its status.
@@ -277,10 +277,10 @@
void UpdateProgress(int delta);
/* Prints the dumpstate header on `stdout`. */
- void PrintHeader();
+ void PrintHeader() const;
/* Gets the path of a bugreport file with the given suffix. */
- std::string GetPath(const std::string& suffix);
+ std::string GetPath(const std::string& suffix) const;
// TODO: initialize fields on constructor
@@ -319,17 +319,25 @@
time_t now_;
- // Suffix of the bugreport files - it's typically the date (when invoked with -d),
- // although it could be changed by the user using a system property.
- std::string suffix_;
-
- // Base name (without suffix or extensions) of the bugreport files.
+ // Base name (without suffix or extensions) of the bugreport files, typically
+ // `bugreport-BUILD_ID`.
std::string baseName_;
+ // Name is the suffix part of the bugreport files - it's typically the date (when invoked with
+ // `-d`), but it could be changed by the user..
+ std::string name_;
+
private:
// Used by GetInstance() only.
Dumpstate(bool dryRun = false, const std::string& buildType = "user");
+ // Internal version of RunCommand that just runs it, without updating progress.
+ int JustRunCommand(const char* command, const char* path, std::vector<const char*>& args,
+ const CommandOptions& options) const;
+
+ // Internal version of RunCommand that just dumps it, without updating progress.
+ int JustDumpFile(const std::string& title, const std::string& path) const;
+
// Whether this is a dry run.
bool dryRun_;
diff --git a/cmds/dumpstate/utils.cpp b/cmds/dumpstate/utils.cpp
index 83fcf21..fc9c037 100644
--- a/cmds/dumpstate/utils.cpp
+++ b/cmds/dumpstate/utils.cpp
@@ -40,6 +40,7 @@
#include <android-base/file.h>
#include <android-base/properties.h>
+#include <android-base/stringprintf.h>
#include <cutils/debugger.h>
#include <cutils/log.h>
#include <cutils/properties.h>
@@ -200,16 +201,17 @@
return (uint64_t) ts.tv_sec * NANOS_PER_SEC + ts.tv_nsec;
}
-bool Dumpstate::IsDryRun() {
+bool Dumpstate::IsDryRun() const {
return dryRun_;
}
-bool Dumpstate::IsUserBuild() {
+bool Dumpstate::IsUserBuild() const {
return "user" == buildType_;
}
-std::string Dumpstate::GetPath(const std::string& suffix) {
- return bugreportDir_ + "/" + baseName_ + "-" + suffix_ + suffix;
+std::string Dumpstate::GetPath(const std::string& suffix) const {
+ return android::base::StringPrintf("%s/%s-%s%s", bugreportDir_.c_str(), baseName_.c_str(),
+ name_.c_str(), suffix.c_str());
}
void for_each_userid(void (*func)(int), const char *header) {
@@ -525,6 +527,7 @@
RunCommand(title, {"showmap", "-q", arg}, CommandOptions::AS_ROOT_10);
}
+// TODO: when converted to a Dumpstate function, it should be const
static int _dump_file_from_fd(const std::string& title, const char* path, int fd) {
if (!title.empty()) {
printf("------ %s (%s", title.c_str(), path);
@@ -603,7 +606,10 @@
UpdateProgress(WEIGHT_FILE);
return 0;
}
+ return JustDumpFile(title, path);
+}
+int Dumpstate::JustDumpFile(const std::string& title, const std::string& path) const {
int fd = TEMP_FAILURE_RETRY(open(path.c_str(), O_RDONLY | O_NONBLOCK | O_CLOEXEC));
if (fd < 0) {
int err = errno;
@@ -833,7 +839,7 @@
return 0;
}
- bool silent = (options.StdoutMode() == REDIRECT_TO_STDERR);
+ int status = JustRunCommand(command, path, args, options);
/* TODO: for now we're simplifying the progress calculation by using the
* timeout as the weight. It's a good approximation for most cases, except when calling dumpsys,
@@ -841,6 +847,17 @@
* Ideally, it should use a options.EstimatedDuration() instead...*/
int weight = options.Timeout();
+ if (weight > 0) {
+ UpdateProgress(weight);
+ }
+
+ return status;
+}
+
+int Dumpstate::JustRunCommand(const char* command, const char* path, std::vector<const char*>& args,
+ const CommandOptions& options) const {
+ bool silent = (options.StdoutMode() == REDIRECT_TO_STDERR);
+
uint64_t start = DurationReporter::Nanotime();
pid_t pid = fork();
@@ -925,9 +942,6 @@
MYLOGE("*** command '%s' failed: exit code %d\n", command, status);
}
- if (weight > 0) {
- UpdateProgress(weight);
- }
return status;
}