Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 6f5ddce7 authored by Rhed Jao's avatar Rhed Jao
Browse files

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
Merged-In: Ib77f114414ed0f4f916bd74de151665d65fe4118
(cherry picked from commit 25f50e00)
parent a5de5752

cmds/dumpstate/dumpstate.cpp

+2 −13
Original line number Diff line number Diff line
@@ -2516,7 +2516,6 @@ static void SetOptionsFromMode(Dumpstate::BugreportMode mode, Dumpstate::DumpOpt 
            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;
@@ -2528,7 +2527,6 @@ static void SetOptionsFromMode(Dumpstate::BugreportMode mode, Dumpstate::DumpOpt 
            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;
@@ -2555,12 +2553,12 @@ static void SetOptionsFromMode(Dumpstate::BugreportMode mode, Dumpstate::DumpOpt 
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());

}
@@ -2776,15 +2774,6 @@ Dumpstate::RunStatus Dumpstate::RunInternal(int32_t calling_uid, 


    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");

    }

cmds/dumpstate/dumpstate.h

+0 −1
Original line number Diff line number Diff line
@@ -398,7 +398,6 @@ class Dumpstate { 
        bool is_screenshot_copied = false;

        bool is_remote_mode = false;

        bool show_header_only = false;

        bool do_start_service = false;

        bool telephony_only = false;

        bool wifi_only = false;

        // Trimmed-down version of dumpstate to only include whitelisted logs.

cmds/dumpstate/tests/dumpstate_test.cpp

+0 −3
Original line number Diff line number Diff line
@@ -258,7 +258,6 @@ TEST_F(DumpOptionsTest, InitializeFullBugReport) { 
    EXPECT_FALSE(options_.do_progress_updates);

    EXPECT_FALSE(options_.is_remote_mode);

    EXPECT_FALSE(options_.use_socket);

    EXPECT_FALSE(options_.do_start_service);

    EXPECT_FALSE(options_.limited_only);

}
@@ -267,7 +266,6 @@ TEST_F(DumpOptionsTest, InitializeInteractiveBugReport) { 
    EXPECT_TRUE(options_.do_add_date);

    EXPECT_TRUE(options_.do_zip_file);

    EXPECT_TRUE(options_.do_progress_updates);

    EXPECT_TRUE(options_.do_start_service);

    EXPECT_TRUE(options_.do_screenshot);

    EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::INTERACTIVE);
@@ -303,7 +301,6 @@ TEST_F(DumpOptionsTest, InitializeWearBugReport) { 
    EXPECT_TRUE(options_.do_screenshot);

    EXPECT_TRUE(options_.do_zip_file);

    EXPECT_TRUE(options_.do_progress_updates);

    EXPECT_TRUE(options_.do_start_service);

    EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::WEAR);



    // Other options retain default values