Merge "Separate options parsing from running dumpstate"
diff --git a/cmds/dumpstate/DumpstateService.cpp b/cmds/dumpstate/DumpstateService.cpp
index 260ea4b..71658d8 100644
--- a/cmds/dumpstate/DumpstateService.cpp
+++ b/cmds/dumpstate/DumpstateService.cpp
@@ -87,27 +87,27 @@
status_t DumpstateService::dump(int fd, const Vector<String16>&) {
dprintf(fd, "id: %d\n", ds_.id_);
dprintf(fd, "pid: %d\n", ds_.pid_);
- dprintf(fd, "update_progress: %s\n", ds_.update_progress_ ? "true" : "false");
+ dprintf(fd, "update_progress: %s\n", ds_.options_->do_progress_updates ? "true" : "false");
dprintf(fd, "update_progress_threshold: %d\n", ds_.update_progress_threshold_);
dprintf(fd, "last_updated_progress: %d\n", ds_.last_updated_progress_);
dprintf(fd, "progress:\n");
ds_.progress_->Dump(fd, " ");
- dprintf(fd, "args: %s\n", ds_.args_.c_str());
- dprintf(fd, "extra_options: %s\n", ds_.extra_options_.c_str());
+ dprintf(fd, "args: %s\n", ds_.options_->args.c_str());
+ dprintf(fd, "extra_options: %s\n", ds_.options_->extra_options.c_str());
dprintf(fd, "version: %s\n", ds_.version_.c_str());
dprintf(fd, "bugreport_dir: %s\n", ds_.bugreport_dir_.c_str());
dprintf(fd, "screenshot_path: %s\n", ds_.screenshot_path_.c_str());
dprintf(fd, "log_path: %s\n", ds_.log_path_.c_str());
dprintf(fd, "tmp_path: %s\n", ds_.tmp_path_.c_str());
dprintf(fd, "path: %s\n", ds_.path_.c_str());
- dprintf(fd, "extra_options: %s\n", ds_.extra_options_.c_str());
+ dprintf(fd, "extra_options: %s\n", ds_.options_->extra_options.c_str());
dprintf(fd, "base_name: %s\n", ds_.base_name_.c_str());
dprintf(fd, "name: %s\n", ds_.name_.c_str());
dprintf(fd, "now: %ld\n", ds_.now_);
dprintf(fd, "is_zipping: %s\n", ds_.IsZipping() ? "true" : "false");
dprintf(fd, "listener: %s\n", ds_.listener_name_.c_str());
- dprintf(fd, "notification title: %s\n", ds_.notification_title.c_str());
- dprintf(fd, "notification description: %s\n", ds_.notification_description.c_str());
+ dprintf(fd, "notification title: %s\n", ds_.options_->notification_title.c_str());
+ dprintf(fd, "notification description: %s\n", ds_.options_->notification_description.c_str());
return NO_ERROR;
}
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index 17bb7c3..0b9bca0 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -682,7 +682,7 @@
CommandOptions::WithTimeout(1).Always().Build());
printf("Bugreport format version: %s\n", version_.c_str());
printf("Dumpstate info: id=%d pid=%d dry_run=%d args=%s extra_options=%s\n", id_, pid_,
- PropertiesHelper::IsDryRun(), args_.c_str(), extra_options_.c_str());
+ PropertiesHelper::IsDryRun(), options_->args.c_str(), options_->extra_options.c_str());
printf("\n");
}
@@ -1623,7 +1623,7 @@
printf("*** See dumpstate-board.txt entry ***\n");
}
-static void ShowUsageAndExit(int exitCode = 1) {
+static void ShowUsageAndExit(int exit_code = 1) {
fprintf(stderr,
"usage: dumpstate [-h] [-b soundfile] [-e soundfile] [-o file] [-d] [-p] "
"[-z]] [-s] [-S] [-q] [-B] [-P] [-R] [-V version]\n"
@@ -1643,7 +1643,7 @@
" -R: take bugreport in remote mode (requires -o, -z, -d and -B, "
"shouldn't be used with -P)\n"
" -v: prints the dumpstate header and exit\n");
- exit(exitCode);
+ exit(exit_code);
}
static void ExitOnInvalidArgs() {
@@ -1769,13 +1769,13 @@
* if we are writing zip files and adds the version file.
*/
static void PrepareToWriteToFile() {
- const Dumpstate::DumpOptions& options = ds.options_;
- ds.bugreport_dir_ = dirname(options.use_outfile.c_str());
+ ds.bugreport_dir_ = dirname(ds.options_->use_outfile.c_str());
std::string build_id = android::base::GetProperty("ro.build.id", "UNKNOWN_BUILD");
std::string device_name = android::base::GetProperty("ro.product.name", "UNKNOWN_DEVICE");
- ds.base_name_ = android::base::StringPrintf("%s-%s-%s", basename(options.use_outfile.c_str()),
- device_name.c_str(), build_id.c_str());
- if (options.do_add_date) {
+ ds.base_name_ =
+ android::base::StringPrintf("%s-%s-%s", basename(ds.options_->use_outfile.c_str()),
+ device_name.c_str(), build_id.c_str());
+ if (ds.options_->do_add_date) {
char date[80];
strftime(date, sizeof(date), "%Y-%m-%d-%H-%M-%S", localtime(&ds.now_));
ds.name_ = date;
@@ -1783,13 +1783,13 @@
ds.name_ = "undated";
}
- if (options.telephony_only) {
+ if (ds.options_->telephony_only) {
ds.base_name_ += "-telephony";
- } else if (options.wifi_only) {
+ } else if (ds.options_->wifi_only) {
ds.base_name_ += "-wifi";
}
- if (options.do_fb) {
+ if (ds.options_->do_fb) {
ds.screenshot_path_ = ds.GetPath(".png");
}
ds.tmp_path_ = ds.GetPath(".tmp");
@@ -1805,7 +1805,7 @@
ds.bugreport_dir_.c_str(), ds.base_name_.c_str(), ds.name_.c_str(), ds.log_path_.c_str(),
ds.tmp_path_.c_str(), ds.screenshot_path_.c_str());
- if (options.do_zip_file) {
+ if (ds.options_->do_zip_file) {
ds.path_ = ds.GetPath(".zip");
MYLOGD("Creating initial .zip file (%s)\n", ds.path_.c_str());
create_parent_dirs(ds.path_.c_str());
@@ -1824,7 +1824,6 @@
* printing zipped file status, etc.
*/
static void FinalizeFile() {
- const Dumpstate::DumpOptions& options = ds.options_;
/* check if user changed the suffix using system properties */
std::string name =
android::base::GetProperty(android::base::StringPrintf("dumpstate.%d.name", ds.pid_), "");
@@ -1853,7 +1852,7 @@
}
bool do_text_file = true;
- if (options.do_zip_file) {
+ if (ds.options_->do_zip_file) {
if (!ds.FinishZipFile()) {
MYLOGE("Failed to finish zip file; sending text bugreport instead\n");
do_text_file = true;
@@ -1880,7 +1879,7 @@
ds.path_.clear();
}
}
- if (options.use_control_socket) {
+ if (ds.options_->use_control_socket) {
if (do_text_file) {
dprintf(ds.control_socket_fd_,
"FAIL:could not create zip file, check %s "
@@ -1894,7 +1893,6 @@
/* Broadcasts that we are done with the bugreport */
static void SendBugreportFinishedBroadcast() {
- const Dumpstate::DumpOptions& options = ds.options_;
if (!ds.path_.empty()) {
MYLOGI("Final bugreport path: %s\n", ds.path_.c_str());
// clang-format off
@@ -1908,22 +1906,22 @@
"--es", "android.intent.extra.DUMPSTATE_LOG", ds.log_path_
};
// clang-format on
- if (options.do_fb) {
+ if (ds.options_->do_fb) {
am_args.push_back("--es");
am_args.push_back("android.intent.extra.SCREENSHOT");
am_args.push_back(ds.screenshot_path_);
}
- if (!ds.notification_title.empty()) {
+ if (ds.options_->notification_title.empty()) {
am_args.push_back("--es");
am_args.push_back("android.intent.extra.TITLE");
- am_args.push_back(ds.notification_title);
- if (!ds.notification_description.empty()) {
+ am_args.push_back(ds.options_->notification_title);
+ if (!ds.options_->notification_description.empty()) {
am_args.push_back("--es");
am_args.push_back("android.intent.extra.DESCRIPTION");
- am_args.push_back(ds.notification_description);
+ am_args.push_back(ds.options_->notification_description);
}
}
- if (options.is_remote_mode) {
+ if (ds.options_->is_remote_mode) {
am_args.push_back("--es");
am_args.push_back("android.intent.extra.REMOTE_BUGREPORT_HASH");
am_args.push_back(SHA256_file_hash(ds.path_));
@@ -1936,30 +1934,77 @@
}
}
-int Dumpstate::ParseCommandlineOptions(int argc, char* argv[]) {
- int ret = -1; // success
+// TODO: Move away from system properties when we have options passed via binder calls.
+/* Sets runtime options from the system properties and then clears those properties. */
+static void SetOptionsFromProperties(Dumpstate::DumpOptions* options) {
+ options->extra_options = android::base::GetProperty(PROPERTY_EXTRA_OPTIONS, "");
+ if (!options->extra_options.empty()) {
+ // Framework uses a system property to override some command-line args.
+ // Currently, it contains the type of the requested bugreport.
+ if (options->extra_options == "bugreportplus") {
+ // Currently, the dumpstate binder is only used by Shell to update progress.
+ options->do_start_service = true;
+ options->do_progress_updates = true;
+ options->do_fb = false;
+ } else if (options->extra_options == "bugreportremote") {
+ options->do_vibrate = false;
+ options->is_remote_mode = true;
+ options->do_fb = false;
+ } else if (options->extra_options == "bugreportwear") {
+ options->do_start_service = true;
+ options->do_progress_updates = true;
+ options->do_zip_file = true;
+ } else if (options->extra_options == "bugreporttelephony") {
+ options->telephony_only = true;
+ } else if (options->extra_options == "bugreportwifi") {
+ options->wifi_only = true;
+ options->do_zip_file = true;
+ } else {
+ MYLOGE("Unknown extra option: %s\n", options->extra_options.c_str());
+ }
+ // Reset the property
+ android::base::SetProperty(PROPERTY_EXTRA_OPTIONS, "");
+ }
+
+ options->notification_title = android::base::GetProperty(PROPERTY_EXTRA_TITLE, "");
+ if (!options->notification_title.empty()) {
+ // Reset the property
+ android::base::SetProperty(PROPERTY_EXTRA_TITLE, "");
+
+ options->extra_options = android::base::GetProperty(PROPERTY_EXTRA_DESCRIPTION, "");
+ if (!options->notification_description.empty()) {
+ // Reset the property
+ android::base::SetProperty(PROPERTY_EXTRA_DESCRIPTION, "");
+ }
+ MYLOGD("notification (title: %s, description: %s)\n", options->notification_title.c_str(),
+ options->notification_description.c_str());
+ }
+}
+
+Dumpstate::RunStatus Dumpstate::DumpOptions::Initialize(int argc, char* argv[]) {
+ RunStatus status = RunStatus::OK;
int c;
while ((c = getopt(argc, argv, "dho:svqzpPBRSV:")) != -1) {
switch (c) {
// clang-format off
- case 'd': options_.do_add_date = true; break;
- case 'z': options_.do_zip_file = true; break;
- case 'o': options_.use_outfile = optarg; break;
- case 's': options_.use_socket = true; break;
- case 'S': options_.use_control_socket = true; break;
- case 'v': options_.show_header_only = true; break;
- case 'q': options_.do_vibrate = false; break;
- case 'p': options_.do_fb = true; break;
- case 'P': update_progress_ = true; break;
- case 'R': options_.is_remote_mode = true; break;
- case 'B': options_.do_broadcast = true; break;
- case 'V': break; // compatibility no-op
+ case 'd': do_add_date = true; break;
+ case 'z': do_zip_file = true; break;
+ case 'o': use_outfile = optarg; break;
+ case 's': use_socket = true; break;
+ case 'S': use_control_socket = true; break;
+ case 'v': show_header_only = true; break;
+ case 'q': do_vibrate = false; break;
+ case 'p': do_fb = true; break;
+ case 'P': do_progress_updates = true; break;
+ case 'R': is_remote_mode = true; break;
+ case 'B': do_broadcast = true; break;
+ case 'V': break; // compatibility no-op
case 'h':
- ret = 0;
+ status = RunStatus::HELP;
break;
default:
fprintf(stderr, "Invalid option: %c\n", c);
- ret = 1;
+ status = RunStatus::INVALID_INPUT;
break;
// clang-format on
}
@@ -1967,87 +2012,44 @@
// TODO: use helper function to convert argv into a string
for (int i = 0; i < argc; i++) {
- args_ += argv[i];
+ args += argv[i];
if (i < argc - 1) {
- args_ += " ";
+ args += " ";
}
}
// Reset next index used by getopt so this can be called multiple times, for eg, in tests.
optind = 1;
- return ret;
+
+ SetOptionsFromProperties(this);
+ return status;
}
-// TODO: Move away from system properties when we have binder.
-void Dumpstate::SetOptionsFromProperties() {
- extra_options_ = android::base::GetProperty(PROPERTY_EXTRA_OPTIONS, "");
- if (!extra_options_.empty()) {
- // Framework uses a system property to override some command-line args.
- // Currently, it contains the type of the requested bugreport.
- if (extra_options_ == "bugreportplus") {
- // Currently, the dumpstate binder is only used by Shell to update progress.
- options_.do_start_service = true;
- update_progress_ = true;
- options_.do_fb = false;
- } else if (extra_options_ == "bugreportremote") {
- options_.do_vibrate = false;
- options_.is_remote_mode = true;
- options_.do_fb = false;
- } else if (extra_options_ == "bugreportwear") {
- options_.do_start_service = true;
- update_progress_ = true;
- options_.do_zip_file = true;
- } else if (extra_options_ == "bugreporttelephony") {
- options_.telephony_only = true;
- } else if (extra_options_ == "bugreportwifi") {
- options_.wifi_only = true;
- options_.do_zip_file = true;
- } else {
- MYLOGE("Unknown extra option: %s\n", extra_options_.c_str());
- }
- // Reset the property
- android::base::SetProperty(PROPERTY_EXTRA_OPTIONS, "");
- }
-
- notification_title = android::base::GetProperty(PROPERTY_EXTRA_TITLE, "");
- if (!notification_title.empty()) {
- // Reset the property
- android::base::SetProperty(PROPERTY_EXTRA_TITLE, "");
-
- notification_description = android::base::GetProperty(PROPERTY_EXTRA_DESCRIPTION, "");
- if (!notification_description.empty()) {
- // Reset the property
- android::base::SetProperty(PROPERTY_EXTRA_DESCRIPTION, "");
- }
- MYLOGD("notification (title: %s, description: %s)\n", notification_title.c_str(),
- notification_description.c_str());
- }
-}
-
-bool Dumpstate::ValidateOptions() {
- if ((options_.do_zip_file || options_.do_add_date || ds.update_progress_ ||
- options_.do_broadcast) &&
- options_.use_outfile.empty()) {
+bool Dumpstate::DumpOptions::ValidateOptions() const {
+ if ((do_zip_file || do_add_date || do_progress_updates || do_broadcast)
+ && use_outfile.empty()) {
return false;
}
- if (options_.use_control_socket && !options_.do_zip_file) {
+ if (use_control_socket && !do_zip_file) {
return false;
}
- if (ds.update_progress_ && !options_.do_broadcast) {
+ if (do_progress_updates && !do_broadcast) {
return false;
}
- if (options_.is_remote_mode && (ds.update_progress_ || !options_.do_broadcast ||
- !options_.do_zip_file || !options_.do_add_date)) {
+ if (is_remote_mode && (do_progress_updates || !do_broadcast || !do_zip_file || !do_add_date)) {
return false;
}
return true;
}
-/* Main entry point for dumpstate. */
-int run_main(int argc, char* argv[]) {
+Dumpstate::RunStatus Dumpstate::RunWithOptions(std::unique_ptr<DumpOptions> options) {
+ if (!options->ValidateOptions()) {
+ return RunStatus::INVALID_INPUT;
+ }
+ options_ = std::move(options);
/* set as high priority, and protect from OOM killer */
setpriority(PRIO_PROCESS, 0, -20);
@@ -2064,52 +2066,42 @@
}
}
- int status = ds.ParseCommandlineOptions(argc, argv);
- if (status != -1) {
- ShowUsageAndExit(status);
- }
- ds.SetOptionsFromProperties();
- if (!ds.ValidateOptions()) {
- ExitOnInvalidArgs();
+ if (version_ == VERSION_DEFAULT) {
+ version_ = VERSION_CURRENT;
}
- if (ds.version_ == VERSION_DEFAULT) {
- ds.version_ = VERSION_CURRENT;
- }
-
- if (ds.version_ != VERSION_CURRENT && ds.version_ != VERSION_SPLIT_ANR) {
+ if (version_ != VERSION_CURRENT && version_ != VERSION_SPLIT_ANR) {
MYLOGE("invalid version requested ('%s'); suppported values are: ('%s', '%s', '%s')\n",
- ds.version_.c_str(), VERSION_DEFAULT.c_str(), VERSION_CURRENT.c_str(),
+ version_.c_str(), VERSION_DEFAULT.c_str(), VERSION_CURRENT.c_str(),
VERSION_SPLIT_ANR.c_str());
- exit(1);
+ return RunStatus::INVALID_INPUT;
}
- const Dumpstate::DumpOptions& options = ds.options_;
- if (options.show_header_only) {
- ds.PrintHeader();
- exit(0);
+ if (options_->show_header_only) {
+ PrintHeader();
+ return RunStatus::OK;
}
// Redirect output if needed
- bool is_redirecting = !options.use_socket && !options.use_outfile.empty();
+ bool is_redirecting = !options_->use_socket && !options_->use_outfile.empty();
// TODO: temporarily set progress until it's part of the Dumpstate constructor
- std::string stats_path = is_redirecting
- ? android::base::StringPrintf("%s/dumpstate-stats.txt",
- dirname(options.use_outfile.c_str()))
- : "";
- ds.progress_.reset(new Progress(stats_path));
+ std::string stats_path =
+ is_redirecting ? android::base::StringPrintf("%s/dumpstate-stats.txt",
+ dirname(options_->use_outfile.c_str()))
+ : "";
+ progress_.reset(new Progress(stats_path));
/* gets the sequential id */
uint32_t last_id = android::base::GetIntProperty(PROPERTY_LAST_ID, 0);
- ds.id_ = ++last_id;
+ id_ = ++last_id;
android::base::SetProperty(PROPERTY_LAST_ID, std::to_string(last_id));
MYLOGI("begin\n");
register_sig_handler();
- if (options.do_start_service) {
+ if (options_->do_start_service) {
MYLOGI("Starting 'dumpstate' service\n");
android::status_t ret;
if ((ret = android::os::DumpstateService::Start()) != android::OK) {
@@ -2121,43 +2113,43 @@
MYLOGI("Running on dry-run mode (to disable it, call 'setprop dumpstate.dry_run false')\n");
}
- MYLOGI("dumpstate info: id=%d, args='%s', extra_options= %s)\n", ds.id_, ds.args_.c_str(),
- ds.extra_options_.c_str());
+ MYLOGI("dumpstate info: id=%d, args='%s', extra_options= %s)\n", id_, options_->args.c_str(),
+ options_->extra_options.c_str());
- MYLOGI("bugreport format version: %s\n", ds.version_.c_str());
+ MYLOGI("bugreport format version: %s\n", version_.c_str());
- ds.do_early_screenshot_ = ds.update_progress_;
+ do_early_screenshot_ = options_->do_progress_updates;
// If we are going to use a socket, do it as early as possible
// to avoid timeouts from bugreport.
- if (options.use_socket) {
+ if (options_->use_socket) {
redirect_to_socket(stdout, "dumpstate");
}
- if (options.use_control_socket) {
+ if (options_->use_control_socket) {
MYLOGD("Opening control socket\n");
- ds.control_socket_fd_ = open_socket("dumpstate");
- ds.update_progress_ = 1;
+ control_socket_fd_ = open_socket("dumpstate");
+ options_->do_progress_updates = 1;
}
if (is_redirecting) {
PrepareToWriteToFile();
- if (ds.update_progress_) {
- if (options.do_broadcast) {
+ if (options_->do_progress_updates) {
+ if (options_->do_broadcast) {
// clang-format off
std::vector<std::string> am_args = {
"--receiver-permission", "android.permission.DUMP",
- "--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(ds.pid_),
- "--ei", "android.intent.extra.MAX", std::to_string(ds.progress_->GetMax()),
+ "--es", "android.intent.extra.NAME", name_,
+ "--ei", "android.intent.extra.ID", std::to_string(id_),
+ "--ei", "android.intent.extra.PID", std::to_string(pid_),
+ "--ei", "android.intent.extra.MAX", std::to_string(progress_->GetMax()),
};
// clang-format on
SendBroadcast("com.android.internal.intent.action.BUGREPORT_STARTED", am_args);
}
- if (options.use_control_socket) {
- dprintf(ds.control_socket_fd_, "BEGIN:%s\n", ds.path_.c_str());
+ if (options_->use_control_socket) {
+ dprintf(control_socket_fd_, "BEGIN:%s\n", path_.c_str());
}
}
}
@@ -2169,23 +2161,23 @@
fclose(cmdline);
}
- if (options.do_vibrate) {
+ if (options_->do_vibrate) {
Vibrate(150);
}
- if (options.do_fb && ds.do_early_screenshot_) {
- if (ds.screenshot_path_.empty()) {
+ if (options_->do_fb && do_early_screenshot_) {
+ if (screenshot_path_.empty()) {
// should not have happened
MYLOGE("INTERNAL ERROR: skipping early screenshot because path was not set\n");
} else {
MYLOGI("taking early screenshot\n");
- ds.TakeScreenshot();
+ TakeScreenshot();
}
}
- if (options.do_zip_file && ds.zip_file != nullptr) {
- if (chown(ds.path_.c_str(), AID_SHELL, AID_SHELL)) {
- MYLOGE("Unable to change ownership of zip file %s: %s\n", ds.path_.c_str(),
+ if (options_->do_zip_file && zip_file != nullptr) {
+ if (chown(path_.c_str(), AID_SHELL, AID_SHELL)) {
+ MYLOGE("Unable to change ownership of zip file %s: %s\n", path_.c_str(),
strerror(errno));
}
}
@@ -2194,19 +2186,19 @@
int dup_stderr_fd;
if (is_redirecting) {
TEMP_FAILURE_RETRY(dup_stderr_fd = dup(fileno(stderr)));
- redirect_to_file(stderr, const_cast<char*>(ds.log_path_.c_str()));
- if (chown(ds.log_path_.c_str(), AID_SHELL, AID_SHELL)) {
- MYLOGE("Unable to change ownership of dumpstate log file %s: %s\n",
- ds.log_path_.c_str(), strerror(errno));
+ redirect_to_file(stderr, const_cast<char*>(log_path_.c_str()));
+ if (chown(log_path_.c_str(), AID_SHELL, AID_SHELL)) {
+ MYLOGE("Unable to change ownership of dumpstate log file %s: %s\n", log_path_.c_str(),
+ strerror(errno));
}
TEMP_FAILURE_RETRY(dup_stdout_fd = dup(fileno(stdout)));
/* TODO: rather than generating a text file now and zipping it later,
it would be more efficient to redirect stdout to the zip entry
directly, but the libziparchive doesn't support that option yet. */
- redirect_to_file(stdout, const_cast<char*>(ds.tmp_path_.c_str()));
- if (chown(ds.tmp_path_.c_str(), AID_SHELL, AID_SHELL)) {
+ redirect_to_file(stdout, const_cast<char*>(tmp_path_.c_str()));
+ if (chown(tmp_path_.c_str(), AID_SHELL, AID_SHELL)) {
MYLOGE("Unable to change ownership of temporary bugreport file %s: %s\n",
- ds.tmp_path_.c_str(), strerror(errno));
+ tmp_path_.c_str(), strerror(errno));
}
}
@@ -2216,18 +2208,18 @@
// NOTE: there should be no stdout output until now, otherwise it would break the header.
// In particular, DurationReport objects should be created passing 'title, NULL', so their
// duration is logged into MYLOG instead.
- ds.PrintHeader();
+ PrintHeader();
- if (options.telephony_only) {
+ if (options_->telephony_only) {
DumpstateTelephonyOnly();
- ds.DumpstateBoard();
- } else if (options.wifi_only) {
+ DumpstateBoard();
+ } else if (options_->wifi_only) {
DumpstateWifiOnly();
} else {
// Dump state for the default case. This also drops root.
if (!DumpstateDefault()) {
// Something went wrong.
- return -1;
+ return RunStatus::ERROR;
}
}
@@ -2237,12 +2229,12 @@
}
/* rename or zip the (now complete) .tmp file to its final location */
- if (!options.use_outfile.empty()) {
+ if (!options_->use_outfile.empty()) {
FinalizeFile();
}
/* vibrate a few but shortly times to let user know it's finished */
- if (options.do_vibrate) {
+ if (options_->do_vibrate) {
for (int i = 0; i < 3; i++) {
Vibrate(75);
usleep((75 + 50) * 1000);
@@ -2250,26 +2242,51 @@
}
/* tell activity manager we're done */
- if (options.do_broadcast) {
+ if (options_->do_broadcast) {
SendBugreportFinishedBroadcast();
}
- MYLOGD("Final progress: %d/%d (estimated %d)\n", ds.progress_->Get(), ds.progress_->GetMax(),
- ds.progress_->GetInitialMax());
- ds.progress_->Save();
- MYLOGI("done (id %d)\n", ds.id_);
+ MYLOGD("Final progress: %d/%d (estimated %d)\n", progress_->Get(), progress_->GetMax(),
+ progress_->GetInitialMax());
+ progress_->Save();
+ MYLOGI("done (id %d)\n", id_);
if (is_redirecting) {
TEMP_FAILURE_RETRY(dup2(dup_stderr_fd, fileno(stderr)));
}
- if (options.use_control_socket && ds.control_socket_fd_ != -1) {
+ if (options_->use_control_socket && control_socket_fd_ != -1) {
MYLOGD("Closing control socket\n");
- close(ds.control_socket_fd_);
+ close(control_socket_fd_);
}
- ds.tombstone_data_.clear();
- ds.anr_data_.clear();
+ tombstone_data_.clear();
+ anr_data_.clear();
+ return RunStatus::OK;
+}
+
+/* Main entry point for dumpstate. */
+int run_main(int argc, char* argv[]) {
+ std::unique_ptr<Dumpstate::DumpOptions> options = std::make_unique<Dumpstate::DumpOptions>();
+ Dumpstate::RunStatus status = options->Initialize(argc, argv);
+ if (status == Dumpstate::RunStatus::OK) {
+ status = ds.RunWithOptions(std::move(options));
+ }
+
+ switch (status) {
+ case Dumpstate::RunStatus::OK:
+ return 0;
+ break;
+ case Dumpstate::RunStatus::HELP:
+ ShowUsageAndExit(0 /* exit code */);
+ break;
+ case Dumpstate::RunStatus::INVALID_INPUT:
+ ExitOnInvalidArgs();
+ break;
+ case Dumpstate::RunStatus::ERROR:
+ exit(-1);
+ break;
+ }
return 0;
}
diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h
index 389cc2e..c2f7f6a 100644
--- a/cmds/dumpstate/dumpstate.h
+++ b/cmds/dumpstate/dumpstate.h
@@ -183,6 +183,8 @@
friend class DumpstateTest;
public:
+ enum RunStatus { OK, HELP, INVALID_INPUT, ERROR };
+
static android::os::dumpstate::CommandOptions DEFAULT_DUMPSYS;
static Dumpstate& GetInstance();
@@ -290,22 +292,12 @@
/* Returns true if the current version supports priority dump feature. */
bool CurrentVersionSupportsPriorityDumps() const;
- // TODO: revisit the return values later.
- /*
- * Parses commandline arguments and sets runtime options accordingly.
- *
- * Returns 0 or positive number if the caller should exit with returned value as
- * exit code, or returns -1 if caller should proceed with execution.
- */
- int ParseCommandlineOptions(int argc, char* argv[]);
+ struct DumpOptions;
- /* Sets runtime options from the system properties. */
- void SetOptionsFromProperties();
+ /* Main entry point for running a complete bugreport. Takes ownership of options. */
+ RunStatus RunWithOptions(std::unique_ptr<DumpOptions> options);
- /* Returns true if the options set so far are consistent. */
- bool ValidateOptions();
-
- // TODO: add update_progress_ & other options from DumpState.
+ // TODO: add other options from DumpState.
/*
* Structure to hold options that determine the behavior of dumpstate.
*/
@@ -322,7 +314,22 @@
bool do_start_service = false;
bool telephony_only = false;
bool wifi_only = false;
+ // Whether progress updates should be published.
+ bool do_progress_updates = false;
std::string use_outfile;
+ // Extra options passed as system property.
+ std::string extra_options;
+ // Command-line arguments as string
+ std::string args;
+ // Notification title and description
+ std::string notification_title;
+ std::string notification_description;
+
+ /* Initializes options from commandline arguments and system properties. */
+ RunStatus Initialize(int argc, char* argv[]);
+
+ /* Returns true if the options set so far are consistent. */
+ bool ValidateOptions() const;
};
// TODO: initialize fields on constructor
@@ -333,10 +340,7 @@
pid_t pid_;
// Runtime options.
- DumpOptions options_;
-
- // Whether progress updates should be published.
- bool update_progress_ = false;
+ std::unique_ptr<DumpOptions> options_;
// How frequently the progess should be updated;the listener will only be notificated when the
// delta from the previous update is more than the threshold.
@@ -356,12 +360,6 @@
// Bugreport format version;
std::string version_ = VERSION_CURRENT;
- // Command-line arguments as string
- std::string args_;
-
- // Extra options passed as system property.
- std::string extra_options_;
-
// Full path of the directory where the bugreport files will be written.
std::string bugreport_dir_;
@@ -398,10 +396,6 @@
std::string listener_name_;
bool report_section_;
- // Notification title and description
- std::string notification_title;
- std::string notification_description;
-
// List of open tombstone dump files.
std::vector<DumpData> tombstone_data_;
diff --git a/cmds/dumpstate/tests/dumpstate_test.cpp b/cmds/dumpstate/tests/dumpstate_test.cpp
index c57535a..b675c51 100644
--- a/cmds/dumpstate/tests/dumpstate_test.cpp
+++ b/cmds/dumpstate/tests/dumpstate_test.cpp
@@ -137,6 +137,175 @@
}
};
+class DumpOptionsTest : public Test {
+ public:
+ virtual ~DumpOptionsTest() {
+ }
+ virtual void SetUp() {
+ options_ = Dumpstate::DumpOptions();
+ }
+
+ Dumpstate::DumpOptions options_;
+};
+
+TEST_F(DumpOptionsTest, InitializeNone) {
+ // clang-format off
+ char* argv[] = {
+ const_cast<char*>("dumpstate")
+ };
+ // clang-format on
+
+ Dumpstate::DumpOptions options;
+ Dumpstate::RunStatus status = options_.Initialize(ARRAY_SIZE(argv), argv);
+
+ EXPECT_EQ(status, Dumpstate::RunStatus::OK);
+ EXPECT_FALSE(options_.do_add_date);
+ EXPECT_FALSE(options_.do_zip_file);
+ EXPECT_EQ("", options_.use_outfile);
+ EXPECT_FALSE(options_.use_socket);
+ EXPECT_FALSE(options_.use_control_socket);
+ EXPECT_FALSE(options_.show_header_only);
+ EXPECT_TRUE(options_.do_vibrate);
+ EXPECT_FALSE(options_.do_fb);
+ EXPECT_FALSE(options_.do_progress_updates);
+ EXPECT_FALSE(options_.is_remote_mode);
+ EXPECT_FALSE(options_.do_broadcast);
+}
+
+TEST_F(DumpOptionsTest, InitializePartial1) {
+ // clang-format off
+ char* argv[] = {
+ const_cast<char*>("dumpstate"),
+ const_cast<char*>("-d"),
+ const_cast<char*>("-z"),
+ const_cast<char*>("-o abc"),
+ const_cast<char*>("-s"),
+ const_cast<char*>("-S"),
+
+ };
+ // clang-format on
+
+ Dumpstate::RunStatus status = options_.Initialize(ARRAY_SIZE(argv), argv);
+
+ EXPECT_EQ(status, Dumpstate::RunStatus::OK);
+ EXPECT_TRUE(options_.do_add_date);
+ EXPECT_TRUE(options_.do_zip_file);
+ // TODO: Maybe we should trim the filename
+ EXPECT_EQ(" abc", std::string(options_.use_outfile));
+ EXPECT_TRUE(options_.use_socket);
+ EXPECT_TRUE(options_.use_control_socket);
+
+ // Other options retain default values
+ EXPECT_FALSE(options_.show_header_only);
+ EXPECT_TRUE(options_.do_vibrate);
+ EXPECT_FALSE(options_.do_fb);
+ EXPECT_FALSE(options_.do_progress_updates);
+ EXPECT_FALSE(options_.is_remote_mode);
+ EXPECT_FALSE(options_.do_broadcast);
+}
+
+TEST_F(DumpOptionsTest, InitializePartial2) {
+ // clang-format off
+ char* argv[] = {
+ const_cast<char*>("dumpstate"),
+ const_cast<char*>("-v"),
+ const_cast<char*>("-q"),
+ const_cast<char*>("-p"),
+ const_cast<char*>("-P"),
+ const_cast<char*>("-R"),
+ const_cast<char*>("-B"),
+ };
+ // clang-format on
+
+ Dumpstate::RunStatus status = options_.Initialize(ARRAY_SIZE(argv), argv);
+
+ EXPECT_EQ(status, Dumpstate::RunStatus::OK);
+ EXPECT_TRUE(options_.show_header_only);
+ EXPECT_FALSE(options_.do_vibrate);
+ EXPECT_TRUE(options_.do_fb);
+ EXPECT_TRUE(options_.do_progress_updates);
+ EXPECT_TRUE(options_.is_remote_mode);
+ EXPECT_TRUE(options_.do_broadcast);
+
+ // Other options retain default values
+ EXPECT_FALSE(options_.do_add_date);
+ EXPECT_FALSE(options_.do_zip_file);
+ EXPECT_EQ("", options_.use_outfile);
+ EXPECT_FALSE(options_.use_socket);
+ EXPECT_FALSE(options_.use_control_socket);
+}
+
+TEST_F(DumpOptionsTest, InitializeHelp) {
+ // clang-format off
+ char* argv[] = {
+ const_cast<char*>("dumpstate"),
+ const_cast<char*>("-h")
+ };
+ // clang-format on
+
+ Dumpstate::RunStatus status = options_.Initialize(ARRAY_SIZE(argv), argv);
+
+ // -h is for help.
+ EXPECT_EQ(status, Dumpstate::RunStatus::HELP);
+}
+
+TEST_F(DumpOptionsTest, InitializeUnknown) {
+ // clang-format off
+ char* argv[] = {
+ const_cast<char*>("dumpstate"),
+ const_cast<char*>("-u") // unknown flag
+ };
+ // clang-format on
+
+ Dumpstate::RunStatus status = options_.Initialize(ARRAY_SIZE(argv), argv);
+
+ // -u is unknown.
+ EXPECT_EQ(status, Dumpstate::RunStatus::INVALID_INPUT);
+}
+
+TEST_F(DumpOptionsTest, ValidateOptionsNeedOutfile1) {
+ options_.do_zip_file = true;
+ EXPECT_FALSE(options_.ValidateOptions());
+ options_.use_outfile = "a/b/c";
+ EXPECT_TRUE(options_.ValidateOptions());
+}
+
+TEST_F(DumpOptionsTest, ValidateOptionsNeedOutfile2) {
+ options_.do_broadcast = true;
+ EXPECT_FALSE(options_.ValidateOptions());
+ options_.use_outfile = "a/b/c";
+ EXPECT_TRUE(options_.ValidateOptions());
+}
+
+TEST_F(DumpOptionsTest, ValidateOptionsNeedZipfile) {
+ options_.use_control_socket = true;
+ EXPECT_FALSE(options_.ValidateOptions());
+
+ options_.do_zip_file = true;
+ options_.use_outfile = "a/b/c"; // do_zip_file needs outfile
+ EXPECT_TRUE(options_.ValidateOptions());
+}
+
+TEST_F(DumpOptionsTest, ValidateOptionsUpdateProgressNeedsBroadcast) {
+ options_.do_progress_updates = true;
+ options_.use_outfile = "a/b/c"; // do_progress_updates needs outfile
+ EXPECT_FALSE(options_.ValidateOptions());
+
+ options_.do_broadcast = true;
+ EXPECT_TRUE(options_.ValidateOptions());
+}
+
+TEST_F(DumpOptionsTest, ValidateOptionsRemoteMode) {
+ options_.is_remote_mode = true;
+ EXPECT_FALSE(options_.ValidateOptions());
+
+ options_.do_broadcast = true;
+ options_.do_zip_file = true;
+ options_.do_add_date = true;
+ options_.use_outfile = "a/b/c"; // do_broadcast needs outfile
+ EXPECT_TRUE(options_.ValidateOptions());
+}
+
class DumpstateTest : public DumpstateBaseTest {
public:
void SetUp() {
@@ -144,9 +313,8 @@
SetDryRun(false);
SetBuildType(android::base::GetProperty("ro.build.type", "(unknown)"));
ds.progress_.reset(new Progress());
- ds.update_progress_ = false;
ds.update_progress_threshold_ = 0;
- ds.options_ = Dumpstate::DumpOptions();
+ ds.options_.reset(new Dumpstate::DumpOptions());
}
// Runs a command and capture `stdout` and `stderr`.
@@ -171,7 +339,7 @@
}
void SetProgress(long progress, long initial_max, long threshold = 0) {
- ds.update_progress_ = true;
+ ds.options_->do_progress_updates = true;
ds.update_progress_threshold_ = threshold;
ds.last_updated_progress_ = 0;
ds.progress_.reset(new Progress(initial_max, progress, 1.2));
@@ -204,157 +372,6 @@
Dumpstate& ds = Dumpstate::GetInstance();
};
-TEST_F(DumpstateTest, ParseCommandlineOptionsNone) {
- // clang-format off
- char* argv[] = {
- const_cast<char*>("dumpstate")
- };
- // clang-format on
-
- int ret = ds.ParseCommandlineOptions(ARRAY_SIZE(argv), argv);
- EXPECT_EQ(-1, ret);
- EXPECT_FALSE(ds.options_.do_add_date);
- EXPECT_FALSE(ds.options_.do_zip_file);
- EXPECT_EQ("", ds.options_.use_outfile);
- EXPECT_FALSE(ds.options_.use_socket);
- EXPECT_FALSE(ds.options_.use_control_socket);
- EXPECT_FALSE(ds.options_.show_header_only);
- EXPECT_TRUE(ds.options_.do_vibrate);
- EXPECT_FALSE(ds.options_.do_fb);
- EXPECT_FALSE(ds.update_progress_);
- EXPECT_FALSE(ds.options_.is_remote_mode);
- EXPECT_FALSE(ds.options_.do_broadcast);
-}
-
-TEST_F(DumpstateTest, ParseCommandlineOptionsPartial1) {
- // clang-format off
- char* argv[] = {
- const_cast<char*>("dumpstate"),
- const_cast<char*>("-d"),
- const_cast<char*>("-z"),
- const_cast<char*>("-o abc"),
- const_cast<char*>("-s"),
- const_cast<char*>("-S"),
-
- };
- // clang-format on
- int ret = ds.ParseCommandlineOptions(ARRAY_SIZE(argv), argv);
- EXPECT_EQ(-1, ret);
- EXPECT_TRUE(ds.options_.do_add_date);
- EXPECT_TRUE(ds.options_.do_zip_file);
- // TODO: Maybe we should trim the filename
- EXPECT_EQ(" abc", std::string(ds.options_.use_outfile));
- EXPECT_TRUE(ds.options_.use_socket);
- EXPECT_TRUE(ds.options_.use_control_socket);
-
- // Other options retain default values
- EXPECT_FALSE(ds.options_.show_header_only);
- EXPECT_TRUE(ds.options_.do_vibrate);
- EXPECT_FALSE(ds.options_.do_fb);
- EXPECT_FALSE(ds.update_progress_);
- EXPECT_FALSE(ds.options_.is_remote_mode);
- EXPECT_FALSE(ds.options_.do_broadcast);
-}
-
-TEST_F(DumpstateTest, ParseCommandlineOptionsPartial2) {
- // clang-format off
- char* argv[] = {
- const_cast<char*>("dumpstate"),
- const_cast<char*>("-v"),
- const_cast<char*>("-q"),
- const_cast<char*>("-p"),
- const_cast<char*>("-P"),
- const_cast<char*>("-R"),
- const_cast<char*>("-B"),
- };
- // clang-format on
- int ret = ds.ParseCommandlineOptions(ARRAY_SIZE(argv), argv);
- EXPECT_EQ(-1, ret);
- EXPECT_TRUE(ds.options_.show_header_only);
- EXPECT_FALSE(ds.options_.do_vibrate);
- EXPECT_TRUE(ds.options_.do_fb);
- EXPECT_TRUE(ds.update_progress_);
- EXPECT_TRUE(ds.options_.is_remote_mode);
- EXPECT_TRUE(ds.options_.do_broadcast);
-
- // Other options retain default values
- EXPECT_FALSE(ds.options_.do_add_date);
- EXPECT_FALSE(ds.options_.do_zip_file);
- EXPECT_EQ("", ds.options_.use_outfile);
- EXPECT_FALSE(ds.options_.use_socket);
- EXPECT_FALSE(ds.options_.use_control_socket);
-}
-
-TEST_F(DumpstateTest, ParseCommandlineOptionsHelp) {
- // clang-format off
- char* argv[] = {
- const_cast<char*>("dumpstate"),
- const_cast<char*>("-h")
- };
- // clang-format on
- int ret = ds.ParseCommandlineOptions(ARRAY_SIZE(argv), argv);
-
- // -h is for help. Caller exit with code = 0 after printing usage, so expect return = 0.
- EXPECT_EQ(0, ret);
-}
-
-TEST_F(DumpstateTest, ParseCommandlineOptionsUnknown) {
- // clang-format off
- char* argv[] = {
- const_cast<char*>("dumpstate"),
- const_cast<char*>("-u") // unknown flag
- };
- // clang-format on
- int ret = ds.ParseCommandlineOptions(ARRAY_SIZE(argv), argv);
-
- // -u is unknown. Caller exit with code = 1 to show execution failure, after printing usage,
- // so expect return = 1.
- EXPECT_EQ(1, ret);
-}
-
-TEST_F(DumpstateTest, ValidateOptionsNeedOutfile1) {
- ds.options_.do_zip_file = true;
- EXPECT_FALSE(ds.ValidateOptions());
- ds.options_.use_outfile = "a/b/c";
- EXPECT_TRUE(ds.ValidateOptions());
-}
-
-TEST_F(DumpstateTest, ValidateOptionsNeedOutfile2) {
- ds.options_.do_broadcast = true;
- EXPECT_FALSE(ds.ValidateOptions());
- ds.options_.use_outfile = "a/b/c";
- EXPECT_TRUE(ds.ValidateOptions());
-}
-
-TEST_F(DumpstateTest, ValidateOptionsNeedZipfile) {
- ds.options_.use_control_socket = true;
- EXPECT_FALSE(ds.ValidateOptions());
-
- ds.options_.do_zip_file = true;
- ds.options_.use_outfile = "a/b/c"; // do_zip_file needs outfile
- EXPECT_TRUE(ds.ValidateOptions());
-}
-
-TEST_F(DumpstateTest, ValidateOptionsUpdateProgressNeedsBroadcast) {
- ds.update_progress_ = true;
- ds.options_.use_outfile = "a/b/c"; // update_progress_ needs outfile
- EXPECT_FALSE(ds.ValidateOptions());
-
- ds.options_.do_broadcast = true;
- EXPECT_TRUE(ds.ValidateOptions());
-}
-
-TEST_F(DumpstateTest, ValidateOptionsRemoteMode) {
- ds.options_.is_remote_mode = true;
- EXPECT_FALSE(ds.ValidateOptions());
-
- ds.options_.do_broadcast = true;
- ds.options_.do_zip_file = true;
- ds.options_.do_add_date = true;
- ds.options_.use_outfile = "a/b/c"; // do_broadcast needs outfile
- EXPECT_TRUE(ds.ValidateOptions());
-}
-
TEST_F(DumpstateTest, RunCommandNoArgs) {
EXPECT_EQ(-1, RunCommand("", {}));
}
diff --git a/cmds/dumpstate/utils.cpp b/cmds/dumpstate/utils.cpp
index 77f09b7..4ad5c4b 100644
--- a/cmds/dumpstate/utils.cpp
+++ b/cmds/dumpstate/utils.cpp
@@ -916,7 +916,7 @@
bool max_changed = progress_->Inc(delta_sec);
// ...but only notifiy listeners when necessary.
- if (!update_progress_) return;
+ if (!options_->do_progress_updates) return;
int progress = progress_->Get();
int max = progress_->GetMax();