commit 25f50e00ebe47ff9067c37da3b3919dcffb0b7ed
author Rhed Jao <rhedjao@google.com> Thu Aug 20 00:10:32 2020 +0800
committer Rhed Jao <rhedjao@google.com> Tue Sep 15 10:14:03 2020 +0000
tree 983e4a4551790a307ceb97b28cbd232a43b05526
parent b5685b33a856c839c6fb672d83b6a81c6dc1918e

Remove a redundant call of starting DumpstateService

A new DumpstateService instance is created and published,
when the start service is invoked. The member field 'ds_' in
DumpstateService would be null, and a crash could happen when
DumpstateService tried to access it.

Bug: 164340819
Test: atest dumpstate_test
Test: atest dumpstate_smoke_test
Test: Manual cancel report via the notification

Change-Id: Ib77f114414ed0f4f916bd74de151665d65fe4118

cmds/dumpstate/dumpstate.cpp

diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index f987c00..d108e00 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -2515,7 +2515,6 @@
             break;
         case Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE:
             // Currently, the dumpstate binder is only used by Shell to update progress.
-            options->do_start_service = true;
             options->do_progress_updates = true;
             options->do_screenshot = is_screenshot_requested;
             options->dumpstate_hal_mode = DumpstateMode::INTERACTIVE;
@@ -2527,7 +2526,6 @@
             options->dumpstate_hal_mode = DumpstateMode::REMOTE;
             break;
         case Dumpstate::BugreportMode::BUGREPORT_WEAR:
-            options->do_start_service = true;
             options->do_progress_updates = true;
             options->do_zip_file = true;
             options->do_screenshot = is_screenshot_requested;
@@ -2554,12 +2552,12 @@
 static void LogDumpOptions(const Dumpstate::DumpOptions& options) {
     MYLOGI(
         "do_zip_file: %d do_vibrate: %d use_socket: %d use_control_socket: %d do_screenshot: %d "
-        "is_remote_mode: %d show_header_only: %d do_start_service: %d telephony_only: %d "
+        "is_remote_mode: %d show_header_only: %d telephony_only: %d "
         "wifi_only: %d do_progress_updates: %d fd: %d bugreport_mode: %s dumpstate_hal_mode: %s "
         "limited_only: %d args: %s\n",
         options.do_zip_file, options.do_vibrate, options.use_socket, options.use_control_socket,
         options.do_screenshot, options.is_remote_mode, options.show_header_only,
-        options.do_start_service, options.telephony_only, options.wifi_only,
+        options.telephony_only, options.wifi_only,
         options.do_progress_updates, options.bugreport_fd.get(), options.bugreport_mode.c_str(),
         toString(options.dumpstate_hal_mode).c_str(), options.limited_only, options.args.c_str());
 }
@@ -2775,15 +2773,6 @@
 
     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;
-        if ((ret = android::os::DumpstateService::Start()) != android::OK) {
-            MYLOGE("Unable to start DumpstateService: %d\n", ret);
-        }
-    }
-
     if (PropertiesHelper::IsDryRun()) {
         MYLOGI("Running on dry-run mode (to disable it, call 'setprop dumpstate.dry_run false')\n");
     }