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

Commit a6366c9f authored by Nandana Dutt's avatar Nandana Dutt Committed by android-build-merger
Browse files

Merge "Remove outfile option from dumpstate" am: 4bcfa50c 

am: 12cefbc5

Change-Id: I11cd28898c46b5d228f0655e17ad1285bfdbccec
parents a93bc27b 12cefbc5

cmds/dumpstate/DumpstateService.cpp

+4 −2
Original line number 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>&) {

    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, "pid: %d\n", ds_.pid_);

    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, "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, "bugreport_internal_dir_: %s\n", ds_.bugreport_internal_dir_.c_str());

    dprintf(fd, "bugreport_dir: %s\n", destination.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());

cmds/dumpstate/dumpstate.cpp

+20 −49
Original line number Diff line number Diff line
@@ -135,10 +135,6 @@ static int Open(std::string path, int flags, mode_t mode = 0) { 
    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) {

    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;

}



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 os
@@ -1800,16 +1785,9 @@ static void MaybeResolveSymlink(std::string* path) { 
static void PrepareToWriteToFile() {

    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 device_name = android::base::GetProperty("ro.product.name", "UNKNOWN_DEVICE");

    ds.base_name_ =

        StringPrintf("%s-%s-%s", base_name_part1.c_str(), device_name.c_str(), build_id.c_str());

    ds.base_name_ = StringPrintf("bugreport-%s-%s", 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_));
@@ -1832,17 +1810,16 @@ static void PrepareToWriteToFile() { 


    std::string destination = ds.options_->bugreport_fd.get() != -1

                                  ? StringPrintf("[fd:%d]", ds.options_->bugreport_fd.get())

                                  : ds.bugreport_dir_.c_str();

                                  : ds.bugreport_internal_dir_.c_str();

    MYLOGD(

        "Bugreport dir: %s\n"

        "Internal Bugreport dir: %s\n"

        "Base name: %s\n"

        "Suffix: %s\n"

        "Log path: %s\n"

        "Temporary path: %s\n"

        "Screenshot path: %s\n",

        destination.c_str(), ds.bugreport_internal_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());

        destination.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 (ds.options_->do_zip_file) {

        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.

            bool copy_succeeded = false;

            if (ds.options_->bugreport_fd.get() != -1) {

                copy_succeeded = android::os::CopyFileToFd(ds.path_, ds.options_->bugreport_fd.get());

            } else {

                ds.final_path_ = ds.GetPath(ds.bugreport_dir_, ".zip");

                copy_succeeded = android::os::CopyFileToFile(ds.path_, ds.final_path_);

            }

            if (copy_succeeded) {

                if (remove(ds.path_.c_str())) {

                bool copy_succeeded =

                    android::os::CopyFileToFd(ds.path_, ds.options_->bugreport_fd.get());

                if (!copy_succeeded && remove(ds.path_.c_str())) {

                    MYLOGE("remove(%s): %s", ds.path_.c_str(), strerror(errno));

                }

            }

            }  // else - the file just remains in the internal directory.

        }

    }

    if (do_text_file) {
@@ -1946,8 +1918,8 @@ static void FinalizeFile() { 
/* Broadcasts that we are done with the bugreport */

static void SendBugreportFinishedBroadcast() {

    // TODO(b/111441001): use callback instead of broadcast.

    if (!ds.final_path_.empty()) {

        MYLOGI("Final bugreport path: %s\n", ds.final_path_.c_str());

    if (!ds.path_.empty()) {

        MYLOGI("Final bugreport path: %s\n", ds.path_.c_str());

        // clang-format off



        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.PID", std::to_string(ds.pid_),

             "--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_

        };

        // clang-format on
@@ -1977,7 +1949,7 @@ static void SendBugreportFinishedBroadcast() { 
        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.final_path_));

            am_args.push_back(SHA256_file_hash(ds.path_));

            SendBroadcast("com.android.internal.intent.action.REMOTE_BUGREPORT_FINISHED", am_args);

        } else {

            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("do_progress_updates: %d\n", options.do_progress_updates);

    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("args: %s\n", options.args.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

            case 'd': do_add_date = 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_control_socket = true;     break;

            case 'v': show_header_only = true;       break;
@@ -2187,9 +2160,7 @@ bool Dumpstate::DumpOptions::ValidateOptions() const { 
        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) &&

        !has_out_file_options) {

    if ((do_zip_file || do_add_date || do_progress_updates || do_broadcast) && !OutputToFile()) {

        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

 * gets added to the archive.

 *

 * Bugreports are first generated in a local directory and later copied to the caller's fd or

 * directory.

 * Bugreports are first generated in a local directory and later copied to the caller's fd if

 * supplied.

 */

Dumpstate::RunStatus Dumpstate::RunInternal() {

    LogDumpOptions(*options_);
@@ -2293,7 +2264,7 @@ Dumpstate::RunStatus Dumpstate::RunInternal() { 
    }



    // 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

    std::string stats_path =
@@ -2444,7 +2415,7 @@ Dumpstate::RunStatus Dumpstate::RunInternal() { 
    }



    /* rename or zip the (now complete) .tmp file to its final location */

    if (!options_->use_outfile.empty()) {

    if (options_->OutputToFile()) {

        FinalizeFile();

    }

cmds/dumpstate/dumpstate.h

+9 −10
Original line number Diff line number Diff line
@@ -331,6 +331,7 @@ class Dumpstate { 
        bool do_add_date = false;

        bool do_zip_file = false;

        bool do_vibrate = true;

        // Writes bugreport content to a socket; only flatfile format is supported.

        bool use_socket = false;

        bool use_control_socket = false;

        bool do_fb = false;
@@ -342,13 +343,11 @@ class Dumpstate { 
        bool wifi_only = false;

        // Whether progress updates should be published.

        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;

        // File descriptor to screenshot file.

        // TODO(b/111441001): Use this fd.

        android::base::unique_fd screenshot_fd;

        // Partial path to output file.

        std::string use_outfile;

        // TODO: rename to MODE.

        // Extra options passed as system property.

        std::string extra_options;
@@ -367,6 +366,13 @@ class Dumpstate { 


        /* Returns true if the options set so far are consistent. */

        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
@@ -424,13 +430,6 @@ class Dumpstate { 
    // Full path of the temporary file containing the screenshot (when requested).

    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.

    std::unique_ptr<FILE, int (*)(FILE*)> zip_file{nullptr, fclose};

cmds/dumpstate/tests/dumpstate_test.cpp

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


    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);
@@ -191,7 +190,6 @@ TEST_F(DumpOptionsTest, InitializeAdbBugreport) { 
        const_cast<char*>("-S"),

        const_cast<char*>("-d"),

        const_cast<char*>("-z"),

        const_cast<char*>("-o abc"),

    };

    // clang-format on
@@ -201,7 +199,6 @@ TEST_F(DumpOptionsTest, InitializeAdbBugreport) { 
    EXPECT_TRUE(options_.do_add_date);

    EXPECT_TRUE(options_.do_zip_file);

    EXPECT_TRUE(options_.use_control_socket);

    EXPECT_EQ(" abc", std::string(options_.use_outfile));



    // Other options retain default values

    EXPECT_TRUE(options_.do_vibrate);
@@ -228,7 +225,6 @@ TEST_F(DumpOptionsTest, InitializeAdbShellBugreport) { 


    // Other options retain default values

    EXPECT_TRUE(options_.do_vibrate);

    EXPECT_EQ("", options_.use_outfile);

    EXPECT_FALSE(options_.do_add_date);

    EXPECT_FALSE(options_.do_zip_file);

    EXPECT_FALSE(options_.use_control_socket);
@@ -247,7 +243,6 @@ TEST_F(DumpOptionsTest, InitializeFullBugReport) { 
        const_cast<char*>("-p"),

        const_cast<char*>("-B"),

        const_cast<char*>("-z"),

        const_cast<char*>("-o abc"),

    };

    // clang-format on

    property_set("dumpstate.options", "bugreportfull");
@@ -259,7 +254,6 @@ TEST_F(DumpOptionsTest, InitializeFullBugReport) { 
    EXPECT_TRUE(options_.do_fb);

    EXPECT_TRUE(options_.do_zip_file);

    EXPECT_TRUE(options_.do_broadcast);

    EXPECT_EQ(" abc", std::string(options_.use_outfile));



    // Other options retain default values

    EXPECT_TRUE(options_.do_vibrate);
@@ -279,7 +273,6 @@ TEST_F(DumpOptionsTest, InitializeInteractiveBugReport) { 
        const_cast<char*>("-p"),

        const_cast<char*>("-B"),

        const_cast<char*>("-z"),

        const_cast<char*>("-o abc"),

    };

    // clang-format on
@@ -294,7 +287,6 @@ TEST_F(DumpOptionsTest, InitializeInteractiveBugReport) { 
    EXPECT_TRUE(options_.do_progress_updates);

    EXPECT_TRUE(options_.do_start_service);

    EXPECT_FALSE(options_.do_fb);

    EXPECT_EQ(" abc", std::string(options_.use_outfile));



    // Other options retain default values

    EXPECT_TRUE(options_.do_vibrate);
@@ -312,7 +304,6 @@ TEST_F(DumpOptionsTest, InitializeRemoteBugReport) { 
        const_cast<char*>("-p"),

        const_cast<char*>("-B"),

        const_cast<char*>("-z"),

        const_cast<char*>("-o abc"),

    };

    // clang-format on
@@ -327,7 +318,6 @@ TEST_F(DumpOptionsTest, InitializeRemoteBugReport) { 
    EXPECT_TRUE(options_.is_remote_mode);

    EXPECT_FALSE(options_.do_vibrate);

    EXPECT_FALSE(options_.do_fb);

    EXPECT_EQ(" abc", std::string(options_.use_outfile));



    // Other options retain default values

    EXPECT_FALSE(options_.use_control_socket);
@@ -344,7 +334,6 @@ TEST_F(DumpOptionsTest, InitializeWearBugReport) { 
        const_cast<char*>("-p"),

        const_cast<char*>("-B"),

        const_cast<char*>("-z"),

        const_cast<char*>("-o abc"),

    };

    // clang-format on
@@ -359,7 +348,6 @@ TEST_F(DumpOptionsTest, InitializeWearBugReport) { 
    EXPECT_TRUE(options_.do_zip_file);

    EXPECT_TRUE(options_.do_progress_updates);

    EXPECT_TRUE(options_.do_start_service);

    EXPECT_EQ(" abc", std::string(options_.use_outfile));



    // Other options retain default values

    EXPECT_TRUE(options_.do_vibrate);
@@ -377,7 +365,6 @@ TEST_F(DumpOptionsTest, InitializeTelephonyBugReport) { 
        const_cast<char*>("-p"),

        const_cast<char*>("-B"),

        const_cast<char*>("-z"),

        const_cast<char*>("-o abc"),

    };

    // clang-format on
@@ -391,7 +378,6 @@ TEST_F(DumpOptionsTest, InitializeTelephonyBugReport) { 
    EXPECT_TRUE(options_.do_broadcast);

    EXPECT_TRUE(options_.do_zip_file);

    EXPECT_TRUE(options_.telephony_only);

    EXPECT_EQ(" abc", std::string(options_.use_outfile));



    // Other options retain default values

    EXPECT_TRUE(options_.do_vibrate);
@@ -410,7 +396,6 @@ TEST_F(DumpOptionsTest, InitializeWifiBugReport) { 
        const_cast<char*>("-p"),

        const_cast<char*>("-B"),

        const_cast<char*>("-z"),

        const_cast<char*>("-o abc"),

    };

    // clang-format on
@@ -424,7 +409,6 @@ TEST_F(DumpOptionsTest, InitializeWifiBugReport) { 
    EXPECT_TRUE(options_.do_broadcast);

    EXPECT_TRUE(options_.do_zip_file);

    EXPECT_TRUE(options_.wifi_only);

    EXPECT_EQ(" abc", std::string(options_.use_outfile));



    // Other options retain default values

    EXPECT_TRUE(options_.do_vibrate);
@@ -444,7 +428,6 @@ TEST_F(DumpOptionsTest, InitializeDefaultBugReport) { 
        const_cast<char*>("-p"),

        const_cast<char*>("-B"),

        const_cast<char*>("-z"),

        const_cast<char*>("-o abc"),

    };

    // clang-format on
@@ -457,7 +440,6 @@ TEST_F(DumpOptionsTest, InitializeDefaultBugReport) { 
    EXPECT_TRUE(options_.do_fb);

    EXPECT_TRUE(options_.do_zip_file);

    EXPECT_TRUE(options_.do_broadcast);

    EXPECT_EQ(" abc", std::string(options_.use_outfile));



    // Other options retain default values

    EXPECT_TRUE(options_.do_vibrate);
@@ -475,7 +457,6 @@ TEST_F(DumpOptionsTest, InitializePartial1) { 
        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"),
@@ -488,7 +469,6 @@ TEST_F(DumpOptionsTest, InitializePartial1) { 
    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);
@@ -527,7 +507,6 @@ TEST_F(DumpOptionsTest, InitializePartial2) { 
    // 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);

}
@@ -562,15 +541,21 @@ TEST_F(DumpOptionsTest, InitializeUnknown) { 


TEST_F(DumpOptionsTest, ValidateOptionsNeedOutfile1) {

    options_.do_zip_file = true;

    // Writing to socket = !writing to file.

    options_.use_socket = true;

    EXPECT_FALSE(options_.ValidateOptions());

    options_.use_outfile = "a/b/c";



    options_.use_socket = false;

    EXPECT_TRUE(options_.ValidateOptions());

}



TEST_F(DumpOptionsTest, ValidateOptionsNeedOutfile2) {

    options_.do_broadcast = true;

    // Writing to socket = !writing to file.

    options_.use_socket = true;

    EXPECT_FALSE(options_.ValidateOptions());

    options_.use_outfile = "a/b/c";



    options_.use_socket = false;

    EXPECT_TRUE(options_.ValidateOptions());

}
@@ -579,13 +564,11 @@ TEST_F(DumpOptionsTest, ValidateOptionsNeedZipfile) { 
    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;
@@ -599,7 +582,6 @@ TEST_F(DumpOptionsTest, ValidateOptionsRemoteMode) { 
    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());

}