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

Commit 9a76d20e authored by Nandana Dutt's avatar Nandana Dutt
Browse files

Remove outfile option from dumpstate

Remove the ability to write to any directory. Instead support only
writing to its internal directory (currently /bugreports, which points
to Shell app's directory), or a caller-specified file fd.

We cannot expect all callers to supply an fd, because in the poweruser
case where the bugreport is triggered with combo keys, the API is
bypassed and dumpstate binary will be run.

This should be a safe change since sepolicy should not allow dumpstate
to write to arbitrary directories anyway.

This keeps the API lean and keeps the user consent for sharing more
focused.

Note that the current callers all pass in /data/user_de/0/com.android.shell/files/bugreports/bugreport
as the outfile argument already, which is the location /bugreports
symlink points to, so it should work just as before.

BUG:111441001
Test: adb bugreport
Test: adb shell bugreport
Test: interactive bugreport
Test: adb shell /data/nativetest64/dumpstate_test/dumpstate_test
Change-Id: Iae8593dc4745147b7bdae25738fcd69b3c20aaf0
parent 458f1de3
Loading
Loading
Loading
Loading
+4 −2
Original line number Original line Diff line number Diff line
@@ -144,6 +144,9 @@ binder::Status DumpstateService::startBugreport(int32_t /* calling_uid */,
}
}


status_t DumpstateService::dump(int fd, const Vector<String16>&) {
status_t DumpstateService::dump(int fd, const Vector<String16>&) {
    std::string destination = ds_.options_->bugreport_fd.get() != -1
                                  ? StringPrintf("[fd:%d]", ds_.options_->bugreport_fd.get())
                                  : ds_.bugreport_internal_dir_.c_str();
    dprintf(fd, "id: %d\n", ds_.id_);
    dprintf(fd, "id: %d\n", ds_.id_);
    dprintf(fd, "pid: %d\n", ds_.pid_);
    dprintf(fd, "pid: %d\n", ds_.pid_);
    dprintf(fd, "update_progress: %s\n", ds_.options_->do_progress_updates ? "true" : "false");
    dprintf(fd, "update_progress: %s\n", ds_.options_->do_progress_updates ? "true" : "false");
@@ -154,8 +157,7 @@ status_t DumpstateService::dump(int fd, const Vector<String16>&) {
    dprintf(fd, "args: %s\n", ds_.options_->args.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, "extra_options: %s\n", ds_.options_->extra_options.c_str());
    dprintf(fd, "version: %s\n", ds_.version_.c_str());
    dprintf(fd, "version: %s\n", ds_.version_.c_str());
    dprintf(fd, "bugreport_dir: %s\n", ds_.bugreport_dir_.c_str());
    dprintf(fd, "bugreport_dir: %s\n", destination.c_str());
    dprintf(fd, "bugreport_internal_dir_: %s\n", ds_.bugreport_internal_dir_.c_str());
    dprintf(fd, "screenshot_path: %s\n", ds_.screenshot_path_.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, "log_path: %s\n", ds_.log_path_.c_str());
    dprintf(fd, "tmp_path: %s\n", ds_.tmp_path_.c_str());
    dprintf(fd, "tmp_path: %s\n", ds_.tmp_path_.c_str());
+20 −49
Original line number Original line Diff line number Diff line
@@ -135,10 +135,6 @@ static int Open(std::string path, int flags, mode_t mode = 0) {
    return fd;
    return fd;
}
}


static int OpenForWrite(std::string path) {
    return Open(path, O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW,
                S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
}


static int OpenForRead(std::string path) {
static int OpenForRead(std::string path) {
    return Open(path, O_RDONLY | O_CLOEXEC | O_NOFOLLOW);
    return Open(path, O_RDONLY | O_CLOEXEC | O_NOFOLLOW);
@@ -169,17 +165,6 @@ static bool CopyFileToFd(const std::string& input_file, int out_fd) {
    return false;
    return false;
}
}


static bool CopyFileToFile(const std::string& input_file, const std::string& output_file) {
    if (input_file == output_file) {
        MYLOGD("Skipping copying bugreport file since the destination is the same (%s)\n",
               output_file.c_str());
        return false;
    }

    MYLOGD("Going to copy bugreport file (%s) to %s\n", input_file.c_str(), output_file.c_str());
    android::base::unique_fd out_fd(OpenForWrite(output_file));
    return CopyFileToFd(input_file, out_fd.get());
}


}  // namespace
}  // namespace
}  // namespace os
}  // namespace os
@@ -1800,16 +1785,9 @@ static void MaybeResolveSymlink(std::string* path) {
static void PrepareToWriteToFile() {
static void PrepareToWriteToFile() {
    MaybeResolveSymlink(&ds.bugreport_internal_dir_);
    MaybeResolveSymlink(&ds.bugreport_internal_dir_);


    std::string base_name_part1 = "bugreport";
    if (!ds.options_->use_outfile.empty()) {
        ds.bugreport_dir_ = dirname(ds.options_->use_outfile.c_str());
        base_name_part1 = basename(ds.options_->use_outfile.c_str());
    }

    std::string build_id = android::base::GetProperty("ro.build.id", "UNKNOWN_BUILD");
    std::string build_id = android::base::GetProperty("ro.build.id", "UNKNOWN_BUILD");
    std::string device_name = android::base::GetProperty("ro.product.name", "UNKNOWN_DEVICE");
    std::string device_name = android::base::GetProperty("ro.product.name", "UNKNOWN_DEVICE");
    ds.base_name_ =
    ds.base_name_ = StringPrintf("bugreport-%s-%s", device_name.c_str(), build_id.c_str());
        StringPrintf("%s-%s-%s", base_name_part1.c_str(), device_name.c_str(), build_id.c_str());
    if (ds.options_->do_add_date) {
    if (ds.options_->do_add_date) {
        char date[80];
        char date[80];
        strftime(date, sizeof(date), "%Y-%m-%d-%H-%M-%S", localtime(&ds.now_));
        strftime(date, sizeof(date), "%Y-%m-%d-%H-%M-%S", localtime(&ds.now_));
@@ -1832,17 +1810,16 @@ static void PrepareToWriteToFile() {


    std::string destination = ds.options_->bugreport_fd.get() != -1
    std::string destination = ds.options_->bugreport_fd.get() != -1
                                  ? StringPrintf("[fd:%d]", ds.options_->bugreport_fd.get())
                                  ? StringPrintf("[fd:%d]", ds.options_->bugreport_fd.get())
                                  : ds.bugreport_dir_.c_str();
                                  : ds.bugreport_internal_dir_.c_str();
    MYLOGD(
    MYLOGD(
        "Bugreport dir: %s\n"
        "Bugreport dir: %s\n"
        "Internal Bugreport dir: %s\n"
        "Base name: %s\n"
        "Base name: %s\n"
        "Suffix: %s\n"
        "Suffix: %s\n"
        "Log path: %s\n"
        "Log path: %s\n"
        "Temporary path: %s\n"
        "Temporary path: %s\n"
        "Screenshot path: %s\n",
        "Screenshot path: %s\n",
        destination.c_str(), ds.bugreport_internal_dir_.c_str(), ds.base_name_.c_str(),
        destination.c_str(), ds.base_name_.c_str(), ds.name_.c_str(), ds.log_path_.c_str(),
        ds.name_.c_str(), ds.log_path_.c_str(), ds.tmp_path_.c_str(), ds.screenshot_path_.c_str());
        ds.tmp_path_.c_str(), ds.screenshot_path_.c_str());


    if (ds.options_->do_zip_file) {
    if (ds.options_->do_zip_file) {
        ds.path_ = ds.GetPath(".zip");
        ds.path_ = ds.GetPath(".zip");
@@ -1909,18 +1886,13 @@ static void FinalizeFile() {
                }
                }
            }
            }
            // The zip file lives in an internal directory. Copy it over to output.
            // The zip file lives in an internal directory. Copy it over to output.
            bool copy_succeeded = false;
            if (ds.options_->bugreport_fd.get() != -1) {
            if (ds.options_->bugreport_fd.get() != -1) {
                copy_succeeded = android::os::CopyFileToFd(ds.path_, ds.options_->bugreport_fd.get());
                bool copy_succeeded =
            } else {
                    android::os::CopyFileToFd(ds.path_, ds.options_->bugreport_fd.get());
                ds.final_path_ = ds.GetPath(ds.bugreport_dir_, ".zip");
                if (!copy_succeeded && remove(ds.path_.c_str())) {
                copy_succeeded = android::os::CopyFileToFile(ds.path_, ds.final_path_);
            }
            if (copy_succeeded) {
                if (remove(ds.path_.c_str())) {
                    MYLOGE("remove(%s): %s", ds.path_.c_str(), strerror(errno));
                    MYLOGE("remove(%s): %s", ds.path_.c_str(), strerror(errno));
                }
                }
            }
            }  // else - the file just remains in the internal directory.
        }
        }
    }
    }
    if (do_text_file) {
    if (do_text_file) {
@@ -1946,8 +1918,8 @@ static void FinalizeFile() {
/* Broadcasts that we are done with the bugreport */
/* Broadcasts that we are done with the bugreport */
static void SendBugreportFinishedBroadcast() {
static void SendBugreportFinishedBroadcast() {
    // TODO(b/111441001): use callback instead of broadcast.
    // TODO(b/111441001): use callback instead of broadcast.
    if (!ds.final_path_.empty()) {
    if (!ds.path_.empty()) {
        MYLOGI("Final bugreport path: %s\n", ds.final_path_.c_str());
        MYLOGI("Final bugreport path: %s\n", ds.path_.c_str());
        // clang-format off
        // clang-format off


        std::vector<std::string> am_args = {
        std::vector<std::string> am_args = {
@@ -1955,7 +1927,7 @@ static void SendBugreportFinishedBroadcast() {
             "--ei", "android.intent.extra.ID", std::to_string(ds.id_),
             "--ei", "android.intent.extra.ID", std::to_string(ds.id_),
             "--ei", "android.intent.extra.PID", std::to_string(ds.pid_),
             "--ei", "android.intent.extra.PID", std::to_string(ds.pid_),
             "--ei", "android.intent.extra.MAX", std::to_string(ds.progress_->GetMax()),
             "--ei", "android.intent.extra.MAX", std::to_string(ds.progress_->GetMax()),
             "--es", "android.intent.extra.BUGREPORT", ds.final_path_,
             "--es", "android.intent.extra.BUGREPORT", ds.path_,
             "--es", "android.intent.extra.DUMPSTATE_LOG", ds.log_path_
             "--es", "android.intent.extra.DUMPSTATE_LOG", ds.log_path_
        };
        };
        // clang-format on
        // clang-format on
@@ -1977,7 +1949,7 @@ static void SendBugreportFinishedBroadcast() {
        if (ds.options_->is_remote_mode) {
        if (ds.options_->is_remote_mode) {
            am_args.push_back("--es");
            am_args.push_back("--es");
            am_args.push_back("android.intent.extra.REMOTE_BUGREPORT_HASH");
            am_args.push_back("android.intent.extra.REMOTE_BUGREPORT_HASH");
            am_args.push_back(SHA256_file_hash(ds.final_path_));
            am_args.push_back(SHA256_file_hash(ds.path_));
            SendBroadcast("com.android.internal.intent.action.REMOTE_BUGREPORT_FINISHED", am_args);
            SendBroadcast("com.android.internal.intent.action.REMOTE_BUGREPORT_FINISHED", am_args);
        } else {
        } else {
            SendBroadcast("com.android.internal.intent.action.BUGREPORT_FINISHED", am_args);
            SendBroadcast("com.android.internal.intent.action.BUGREPORT_FINISHED", am_args);
@@ -2115,7 +2087,6 @@ static void LogDumpOptions(const Dumpstate::DumpOptions& options) {
    MYLOGI("wifi_only: %d\n", options.wifi_only);
    MYLOGI("wifi_only: %d\n", options.wifi_only);
    MYLOGI("do_progress_updates: %d\n", options.do_progress_updates);
    MYLOGI("do_progress_updates: %d\n", options.do_progress_updates);
    MYLOGI("fd: %d\n", options.bugreport_fd.get());
    MYLOGI("fd: %d\n", options.bugreport_fd.get());
    MYLOGI("use_outfile: %s\n", options.use_outfile.c_str());
    MYLOGI("extra_options: %s\n", options.extra_options.c_str());
    MYLOGI("extra_options: %s\n", options.extra_options.c_str());
    MYLOGI("args: %s\n", options.args.c_str());
    MYLOGI("args: %s\n", options.args.c_str());
    MYLOGI("notification_title: %s\n", options.notification_title.c_str());
    MYLOGI("notification_title: %s\n", options.notification_title.c_str());
@@ -2146,7 +2117,9 @@ Dumpstate::RunStatus Dumpstate::DumpOptions::Initialize(int argc, char* argv[])
            // clang-format off
            // clang-format off
            case 'd': do_add_date = true;            break;
            case 'd': do_add_date = true;            break;
            case 'z': do_zip_file = true;            break;
            case 'z': do_zip_file = true;            break;
            case 'o': use_outfile = optarg;          break;
            // o=use_outfile not supported anymore.
            // TODO(b/111441001): Remove when all callers have migrated.
            case 'o': break;
            case 's': use_socket = true;             break;
            case 's': use_socket = true;             break;
            case 'S': use_control_socket = true;     break;
            case 'S': use_control_socket = true;     break;
            case 'v': show_header_only = true;       break;
            case 'v': show_header_only = true;       break;
@@ -2187,9 +2160,7 @@ bool Dumpstate::DumpOptions::ValidateOptions() const {
        return false;
        return false;
    }
    }


    bool has_out_file_options = !use_outfile.empty() || bugreport_fd.get() != -1;
    if ((do_zip_file || do_add_date || do_progress_updates || do_broadcast) && !OutputToFile()) {
    if ((do_zip_file || do_add_date || do_progress_updates || do_broadcast) &&
        !has_out_file_options) {
        return false;
        return false;
    }
    }


@@ -2251,8 +2222,8 @@ Dumpstate::RunStatus Dumpstate::Run() {
 * If zipping, a bunch of other files and dumps also get added to the zip archive. The log file also
 * If zipping, a bunch of other files and dumps also get added to the zip archive. The log file also
 * gets added to the archive.
 * gets added to the archive.
 *
 *
 * Bugreports are first generated in a local directory and later copied to the caller's fd or
 * Bugreports are first generated in a local directory and later copied to the caller's fd if
 * directory.
 * supplied.
 */
 */
Dumpstate::RunStatus Dumpstate::RunInternal() {
Dumpstate::RunStatus Dumpstate::RunInternal() {
    LogDumpOptions(*options_);
    LogDumpOptions(*options_);
@@ -2293,7 +2264,7 @@ Dumpstate::RunStatus Dumpstate::RunInternal() {
    }
    }


    // Redirect output if needed
    // Redirect output if needed
    bool is_redirecting = !options_->use_socket && !options_->use_outfile.empty();
    bool is_redirecting = options_->OutputToFile();


    // TODO: temporarily set progress until it's part of the Dumpstate constructor
    // TODO: temporarily set progress until it's part of the Dumpstate constructor
    std::string stats_path =
    std::string stats_path =
@@ -2444,7 +2415,7 @@ Dumpstate::RunStatus Dumpstate::RunInternal() {
    }
    }


    /* rename or zip the (now complete) .tmp file to its final location */
    /* rename or zip the (now complete) .tmp file to its final location */
    if (!options_->use_outfile.empty()) {
    if (options_->OutputToFile()) {
        FinalizeFile();
        FinalizeFile();
    }
    }


+9 −10
Original line number Original line Diff line number Diff line
@@ -331,6 +331,7 @@ class Dumpstate {
        bool do_add_date = false;
        bool do_add_date = false;
        bool do_zip_file = false;
        bool do_zip_file = false;
        bool do_vibrate = true;
        bool do_vibrate = true;
        // Writes bugreport content to a socket; only flatfile format is supported.
        bool use_socket = false;
        bool use_socket = false;
        bool use_control_socket = false;
        bool use_control_socket = false;
        bool do_fb = false;
        bool do_fb = false;
@@ -342,13 +343,11 @@ class Dumpstate {
        bool wifi_only = false;
        bool wifi_only = false;
        // Whether progress updates should be published.
        // Whether progress updates should be published.
        bool do_progress_updates = false;
        bool do_progress_updates = false;
        // File descriptor to output zip file. Takes precedence over use_outfile.
        // File descriptor to output zip file.
        android::base::unique_fd bugreport_fd;
        android::base::unique_fd bugreport_fd;
        // File descriptor to screenshot file.
        // File descriptor to screenshot file.
        // TODO(b/111441001): Use this fd.
        // TODO(b/111441001): Use this fd.
        android::base::unique_fd screenshot_fd;
        android::base::unique_fd screenshot_fd;
        // Partial path to output file.
        std::string use_outfile;
        // TODO: rename to MODE.
        // TODO: rename to MODE.
        // Extra options passed as system property.
        // Extra options passed as system property.
        std::string extra_options;
        std::string extra_options;
@@ -367,6 +366,13 @@ class Dumpstate {


        /* Returns true if the options set so far are consistent. */
        /* Returns true if the options set so far are consistent. */
        bool ValidateOptions() const;
        bool ValidateOptions() const;

        /* Returns if options specified require writing bugreport to a file */
        bool OutputToFile() const {
            // If we are not writing to socket, we will write to a file. If bugreport_fd is
            // specified, it is preferred. If not bugreport is written to /bugreports.
            return !use_socket;
        }
    };
    };


    // TODO: initialize fields on constructor
    // TODO: initialize fields on constructor
@@ -424,13 +430,6 @@ class Dumpstate {
    // Full path of the temporary file containing the screenshot (when requested).
    // Full path of the temporary file containing the screenshot (when requested).
    std::string screenshot_path_;
    std::string screenshot_path_;


    // TODO(b/111441001): remove when obsolete.
    // Full path of the final zip file inside the caller-specified directory, if available.
    std::string final_path_;

    // The caller-specified directory, if available.
    std::string bugreport_dir_;

    // Pointer to the zipped file.
    // Pointer to the zipped file.
    std::unique_ptr<FILE, int (*)(FILE*)> zip_file{nullptr, fclose};
    std::unique_ptr<FILE, int (*)(FILE*)> zip_file{nullptr, fclose};


+8 −26
Original line number Original line Diff line number Diff line
@@ -173,7 +173,6 @@ TEST_F(DumpOptionsTest, InitializeNone) {


    EXPECT_FALSE(options_.do_add_date);
    EXPECT_FALSE(options_.do_add_date);
    EXPECT_FALSE(options_.do_zip_file);
    EXPECT_FALSE(options_.do_zip_file);
    EXPECT_EQ("", options_.use_outfile);
    EXPECT_FALSE(options_.use_socket);
    EXPECT_FALSE(options_.use_socket);
    EXPECT_FALSE(options_.use_control_socket);
    EXPECT_FALSE(options_.use_control_socket);
    EXPECT_FALSE(options_.show_header_only);
    EXPECT_FALSE(options_.show_header_only);
@@ -191,7 +190,6 @@ TEST_F(DumpOptionsTest, InitializeAdbBugreport) {
        const_cast<char*>("-S"),
        const_cast<char*>("-S"),
        const_cast<char*>("-d"),
        const_cast<char*>("-d"),
        const_cast<char*>("-z"),
        const_cast<char*>("-z"),
        const_cast<char*>("-o abc"),
    };
    };
    // clang-format on
    // clang-format on


@@ -201,7 +199,6 @@ TEST_F(DumpOptionsTest, InitializeAdbBugreport) {
    EXPECT_TRUE(options_.do_add_date);
    EXPECT_TRUE(options_.do_add_date);
    EXPECT_TRUE(options_.do_zip_file);
    EXPECT_TRUE(options_.do_zip_file);
    EXPECT_TRUE(options_.use_control_socket);
    EXPECT_TRUE(options_.use_control_socket);
    EXPECT_EQ(" abc", std::string(options_.use_outfile));


    // Other options retain default values
    // Other options retain default values
    EXPECT_TRUE(options_.do_vibrate);
    EXPECT_TRUE(options_.do_vibrate);
@@ -228,7 +225,6 @@ TEST_F(DumpOptionsTest, InitializeAdbShellBugreport) {


    // Other options retain default values
    // Other options retain default values
    EXPECT_TRUE(options_.do_vibrate);
    EXPECT_TRUE(options_.do_vibrate);
    EXPECT_EQ("", options_.use_outfile);
    EXPECT_FALSE(options_.do_add_date);
    EXPECT_FALSE(options_.do_add_date);
    EXPECT_FALSE(options_.do_zip_file);
    EXPECT_FALSE(options_.do_zip_file);
    EXPECT_FALSE(options_.use_control_socket);
    EXPECT_FALSE(options_.use_control_socket);
@@ -247,7 +243,6 @@ TEST_F(DumpOptionsTest, InitializeFullBugReport) {
        const_cast<char*>("-p"),
        const_cast<char*>("-p"),
        const_cast<char*>("-B"),
        const_cast<char*>("-B"),
        const_cast<char*>("-z"),
        const_cast<char*>("-z"),
        const_cast<char*>("-o abc"),
    };
    };
    // clang-format on
    // clang-format on
    property_set("dumpstate.options", "bugreportfull");
    property_set("dumpstate.options", "bugreportfull");
@@ -259,7 +254,6 @@ TEST_F(DumpOptionsTest, InitializeFullBugReport) {
    EXPECT_TRUE(options_.do_fb);
    EXPECT_TRUE(options_.do_fb);
    EXPECT_TRUE(options_.do_zip_file);
    EXPECT_TRUE(options_.do_zip_file);
    EXPECT_TRUE(options_.do_broadcast);
    EXPECT_TRUE(options_.do_broadcast);
    EXPECT_EQ(" abc", std::string(options_.use_outfile));


    // Other options retain default values
    // Other options retain default values
    EXPECT_TRUE(options_.do_vibrate);
    EXPECT_TRUE(options_.do_vibrate);
@@ -279,7 +273,6 @@ TEST_F(DumpOptionsTest, InitializeInteractiveBugReport) {
        const_cast<char*>("-p"),
        const_cast<char*>("-p"),
        const_cast<char*>("-B"),
        const_cast<char*>("-B"),
        const_cast<char*>("-z"),
        const_cast<char*>("-z"),
        const_cast<char*>("-o abc"),
    };
    };
    // clang-format on
    // clang-format on


@@ -294,7 +287,6 @@ TEST_F(DumpOptionsTest, InitializeInteractiveBugReport) {
    EXPECT_TRUE(options_.do_progress_updates);
    EXPECT_TRUE(options_.do_progress_updates);
    EXPECT_TRUE(options_.do_start_service);
    EXPECT_TRUE(options_.do_start_service);
    EXPECT_FALSE(options_.do_fb);
    EXPECT_FALSE(options_.do_fb);
    EXPECT_EQ(" abc", std::string(options_.use_outfile));


    // Other options retain default values
    // Other options retain default values
    EXPECT_TRUE(options_.do_vibrate);
    EXPECT_TRUE(options_.do_vibrate);
@@ -312,7 +304,6 @@ TEST_F(DumpOptionsTest, InitializeRemoteBugReport) {
        const_cast<char*>("-p"),
        const_cast<char*>("-p"),
        const_cast<char*>("-B"),
        const_cast<char*>("-B"),
        const_cast<char*>("-z"),
        const_cast<char*>("-z"),
        const_cast<char*>("-o abc"),
    };
    };
    // clang-format on
    // clang-format on


@@ -327,7 +318,6 @@ TEST_F(DumpOptionsTest, InitializeRemoteBugReport) {
    EXPECT_TRUE(options_.is_remote_mode);
    EXPECT_TRUE(options_.is_remote_mode);
    EXPECT_FALSE(options_.do_vibrate);
    EXPECT_FALSE(options_.do_vibrate);
    EXPECT_FALSE(options_.do_fb);
    EXPECT_FALSE(options_.do_fb);
    EXPECT_EQ(" abc", std::string(options_.use_outfile));


    // Other options retain default values
    // Other options retain default values
    EXPECT_FALSE(options_.use_control_socket);
    EXPECT_FALSE(options_.use_control_socket);
@@ -344,7 +334,6 @@ TEST_F(DumpOptionsTest, InitializeWearBugReport) {
        const_cast<char*>("-p"),
        const_cast<char*>("-p"),
        const_cast<char*>("-B"),
        const_cast<char*>("-B"),
        const_cast<char*>("-z"),
        const_cast<char*>("-z"),
        const_cast<char*>("-o abc"),
    };
    };
    // clang-format on
    // clang-format on


@@ -359,7 +348,6 @@ TEST_F(DumpOptionsTest, InitializeWearBugReport) {
    EXPECT_TRUE(options_.do_zip_file);
    EXPECT_TRUE(options_.do_zip_file);
    EXPECT_TRUE(options_.do_progress_updates);
    EXPECT_TRUE(options_.do_progress_updates);
    EXPECT_TRUE(options_.do_start_service);
    EXPECT_TRUE(options_.do_start_service);
    EXPECT_EQ(" abc", std::string(options_.use_outfile));


    // Other options retain default values
    // Other options retain default values
    EXPECT_TRUE(options_.do_vibrate);
    EXPECT_TRUE(options_.do_vibrate);
@@ -377,7 +365,6 @@ TEST_F(DumpOptionsTest, InitializeTelephonyBugReport) {
        const_cast<char*>("-p"),
        const_cast<char*>("-p"),
        const_cast<char*>("-B"),
        const_cast<char*>("-B"),
        const_cast<char*>("-z"),
        const_cast<char*>("-z"),
        const_cast<char*>("-o abc"),
    };
    };
    // clang-format on
    // clang-format on


@@ -391,7 +378,6 @@ TEST_F(DumpOptionsTest, InitializeTelephonyBugReport) {
    EXPECT_TRUE(options_.do_broadcast);
    EXPECT_TRUE(options_.do_broadcast);
    EXPECT_TRUE(options_.do_zip_file);
    EXPECT_TRUE(options_.do_zip_file);
    EXPECT_TRUE(options_.telephony_only);
    EXPECT_TRUE(options_.telephony_only);
    EXPECT_EQ(" abc", std::string(options_.use_outfile));


    // Other options retain default values
    // Other options retain default values
    EXPECT_TRUE(options_.do_vibrate);
    EXPECT_TRUE(options_.do_vibrate);
@@ -410,7 +396,6 @@ TEST_F(DumpOptionsTest, InitializeWifiBugReport) {
        const_cast<char*>("-p"),
        const_cast<char*>("-p"),
        const_cast<char*>("-B"),
        const_cast<char*>("-B"),
        const_cast<char*>("-z"),
        const_cast<char*>("-z"),
        const_cast<char*>("-o abc"),
    };
    };
    // clang-format on
    // clang-format on


@@ -424,7 +409,6 @@ TEST_F(DumpOptionsTest, InitializeWifiBugReport) {
    EXPECT_TRUE(options_.do_broadcast);
    EXPECT_TRUE(options_.do_broadcast);
    EXPECT_TRUE(options_.do_zip_file);
    EXPECT_TRUE(options_.do_zip_file);
    EXPECT_TRUE(options_.wifi_only);
    EXPECT_TRUE(options_.wifi_only);
    EXPECT_EQ(" abc", std::string(options_.use_outfile));


    // Other options retain default values
    // Other options retain default values
    EXPECT_TRUE(options_.do_vibrate);
    EXPECT_TRUE(options_.do_vibrate);
@@ -444,7 +428,6 @@ TEST_F(DumpOptionsTest, InitializeDefaultBugReport) {
        const_cast<char*>("-p"),
        const_cast<char*>("-p"),
        const_cast<char*>("-B"),
        const_cast<char*>("-B"),
        const_cast<char*>("-z"),
        const_cast<char*>("-z"),
        const_cast<char*>("-o abc"),
    };
    };
    // clang-format on
    // clang-format on


@@ -457,7 +440,6 @@ TEST_F(DumpOptionsTest, InitializeDefaultBugReport) {
    EXPECT_TRUE(options_.do_fb);
    EXPECT_TRUE(options_.do_fb);
    EXPECT_TRUE(options_.do_zip_file);
    EXPECT_TRUE(options_.do_zip_file);
    EXPECT_TRUE(options_.do_broadcast);
    EXPECT_TRUE(options_.do_broadcast);
    EXPECT_EQ(" abc", std::string(options_.use_outfile));


    // Other options retain default values
    // Other options retain default values
    EXPECT_TRUE(options_.do_vibrate);
    EXPECT_TRUE(options_.do_vibrate);
@@ -475,7 +457,6 @@ TEST_F(DumpOptionsTest, InitializePartial1) {
        const_cast<char*>("dumpstate"),
        const_cast<char*>("dumpstate"),
        const_cast<char*>("-d"),
        const_cast<char*>("-d"),
        const_cast<char*>("-z"),
        const_cast<char*>("-z"),
        const_cast<char*>("-o abc"),
        const_cast<char*>("-s"),
        const_cast<char*>("-s"),
        const_cast<char*>("-S"),
        const_cast<char*>("-S"),


@@ -488,7 +469,6 @@ TEST_F(DumpOptionsTest, InitializePartial1) {
    EXPECT_TRUE(options_.do_add_date);
    EXPECT_TRUE(options_.do_add_date);
    EXPECT_TRUE(options_.do_zip_file);
    EXPECT_TRUE(options_.do_zip_file);
    // TODO: Maybe we should trim the filename
    // TODO: Maybe we should trim the filename
    EXPECT_EQ(" abc", std::string(options_.use_outfile));
    EXPECT_TRUE(options_.use_socket);
    EXPECT_TRUE(options_.use_socket);
    EXPECT_TRUE(options_.use_control_socket);
    EXPECT_TRUE(options_.use_control_socket);


@@ -527,7 +507,6 @@ TEST_F(DumpOptionsTest, InitializePartial2) {
    // Other options retain default values
    // Other options retain default values
    EXPECT_FALSE(options_.do_add_date);
    EXPECT_FALSE(options_.do_add_date);
    EXPECT_FALSE(options_.do_zip_file);
    EXPECT_FALSE(options_.do_zip_file);
    EXPECT_EQ("", options_.use_outfile);
    EXPECT_FALSE(options_.use_socket);
    EXPECT_FALSE(options_.use_socket);
    EXPECT_FALSE(options_.use_control_socket);
    EXPECT_FALSE(options_.use_control_socket);
}
}
@@ -562,15 +541,21 @@ TEST_F(DumpOptionsTest, InitializeUnknown) {


TEST_F(DumpOptionsTest, ValidateOptionsNeedOutfile1) {
TEST_F(DumpOptionsTest, ValidateOptionsNeedOutfile1) {
    options_.do_zip_file = true;
    options_.do_zip_file = true;
    // Writing to socket = !writing to file.
    options_.use_socket = true;
    EXPECT_FALSE(options_.ValidateOptions());
    EXPECT_FALSE(options_.ValidateOptions());
    options_.use_outfile = "a/b/c";

    options_.use_socket = false;
    EXPECT_TRUE(options_.ValidateOptions());
    EXPECT_TRUE(options_.ValidateOptions());
}
}


TEST_F(DumpOptionsTest, ValidateOptionsNeedOutfile2) {
TEST_F(DumpOptionsTest, ValidateOptionsNeedOutfile2) {
    options_.do_broadcast = true;
    options_.do_broadcast = true;
    // Writing to socket = !writing to file.
    options_.use_socket = true;
    EXPECT_FALSE(options_.ValidateOptions());
    EXPECT_FALSE(options_.ValidateOptions());
    options_.use_outfile = "a/b/c";

    options_.use_socket = false;
    EXPECT_TRUE(options_.ValidateOptions());
    EXPECT_TRUE(options_.ValidateOptions());
}
}


@@ -579,13 +564,11 @@ TEST_F(DumpOptionsTest, ValidateOptionsNeedZipfile) {
    EXPECT_FALSE(options_.ValidateOptions());
    EXPECT_FALSE(options_.ValidateOptions());


    options_.do_zip_file = true;
    options_.do_zip_file = true;
    options_.use_outfile = "a/b/c";  // do_zip_file needs outfile
    EXPECT_TRUE(options_.ValidateOptions());
    EXPECT_TRUE(options_.ValidateOptions());
}
}


TEST_F(DumpOptionsTest, ValidateOptionsUpdateProgressNeedsBroadcast) {
TEST_F(DumpOptionsTest, ValidateOptionsUpdateProgressNeedsBroadcast) {
    options_.do_progress_updates = true;
    options_.do_progress_updates = true;
    options_.use_outfile = "a/b/c";  // do_progress_updates needs outfile
    EXPECT_FALSE(options_.ValidateOptions());
    EXPECT_FALSE(options_.ValidateOptions());


    options_.do_broadcast = true;
    options_.do_broadcast = true;
@@ -599,7 +582,6 @@ TEST_F(DumpOptionsTest, ValidateOptionsRemoteMode) {
    options_.do_broadcast = true;
    options_.do_broadcast = true;
    options_.do_zip_file = true;
    options_.do_zip_file = true;
    options_.do_add_date = true;
    options_.do_add_date = true;
    options_.use_outfile = "a/b/c";  // do_broadcast needs outfile
    EXPECT_TRUE(options_.ValidateOptions());
    EXPECT_TRUE(options_.ValidateOptions());
}
}