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

Commit 111b9d06 authored by Felipe Leme's avatar Felipe Leme
Browse files

Improved how the Shell directories are created. 

When dumpstate is run for the first time, the
/data/data/com.android.shell/files/bugreports does not exist, which was
crashing dumpstate because the code that added the version.txt entry was
not checking if the zip_writer was NULL.

The crash itself was fixed by adding a sanity check in the functions
that add entries to the zip file, but that only hid the real problem:
it is necessary to create the parent directories before creating the zip
file, otherwise the first run will always generate a .txt file (since
dumpstate falls back to .txt when it cannot create the .zip).

This change also improves how the parent directories are created by
checking if they exist first, rather than always calling mkdir().

BUG: 26949960

Change-Id: I1434be5c36a3fad0b3a2a26c7eaaab03a1228c30
parent 858a4332

cmds/dumpstate/dumpstate.cpp

+16 −3
Original line number Diff line number Diff line
@@ -368,6 +368,10 @@ static void print_header(std::string version) { 


/* adds a new entry to the existing zip file. */

static bool add_zip_entry_from_fd(const std::string& entry_name, int fd) {

    if (!zip_writer) {

        ALOGD("Not adding zip entry %s from fd because zip_writer is not set", entry_name.c_str());

        return false;

    }

    ALOGD("Adding zip entry %s", entry_name.c_str());

    int32_t err = zip_writer->StartEntryWithTime(entry_name.c_str(),

            ZipWriter::kCompress, get_mtime(fd, now));
@@ -420,14 +424,21 @@ static int _add_file_from_fd(const char *title, const char *path, int fd) { 


/* adds all files from a directory to the zipped bugreport file */

void add_dir(const char *dir, bool recursive) {

    if (!zip_writer) return;

    if (!zip_writer) {

        ALOGD("Not adding dir %s because zip_writer is not set", dir);

        return;

    }

    DurationReporter duration_reporter(dir, NULL);

    dump_files(NULL, dir, recursive ? skip_none : is_dir, _add_file_from_fd);

}



/* adds a text entry entry to the existing zip file. */

static bool add_text_zip_entry(const std::string& entry_name, const std::string& content) {

    ALOGD("Adding zip text entry %s (%s)", entry_name.c_str(), content.c_str());

    if (!zip_writer) {

        ALOGD("Not adding text zip entry %s because zip_writer is not set", entry_name.c_str());

        return false;

    }

    ALOGD("Adding zip text entry %s", entry_name.c_str());

    int32_t err = zip_writer->StartEntryWithTime(entry_name.c_str(), ZipWriter::kCompress, now);

    if (err) {

        ALOGE("zip_writer->StartEntryWithTime(%s): %s\n", entry_name.c_str(),
@@ -772,7 +783,7 @@ static void usage() { 
            "  -s: write output to control socket (for init)\n"

            "  -q: disable vibrate\n"

            "  -B: send broadcast when finished (requires -o)\n"

            "  -P: send broadacast when started and update system properties on progress (requires -o and -B)\n"

            "  -P: send broadcast when started and update system properties on progress (requires -o and -B)\n"

            "  -R: take bugreport in remote mode (requires -o, -z, -d and -B, shouldn't be used with -P)\n"

            "  -V: sets the bugreport format version (%s or %s)\n",

            VERSION_DEFAULT.c_str(), VERSION_DUMPSYS_SPLIT.c_str());
@@ -794,6 +805,7 @@ static bool finish_zip_file(const std::string& bugreport_name, const std::string 
    }

    if (!add_text_zip_entry("main_entry.txt", bugreport_name)) {

        ALOGE("Failed to add main_entry.txt to .zip file\n");

        return false;

    }



    int32_t err = zip_writer->Finish();
@@ -1027,6 +1039,7 @@ int main(int argc, char *argv[]) { 
        if (do_zip_file) {

            ALOGD("Creating initial .zip file");

            path = bugreport_dir + "/" + base_name + "-" + suffix + ".zip";

            create_parent_dirs(path.c_str());

            zip_file.reset(fopen(path.c_str(), "wb"));

            if (!zip_file) {

                ALOGE("fopen(%s, 'wb'): %s\n", path.c_str(), strerror(errno));

cmds/dumpstate/dumpstate.h

+3 −0
Original line number Diff line number Diff line
@@ -112,6 +112,9 @@ void redirect_to_socket(FILE *redirect, const char *service); 
/* redirect output to a file */

void redirect_to_file(FILE *redirect, char *path);



/* create leading directories, if necessary */

void create_parent_dirs(const char *path);



/* dump Dalvik and native stack traces, return the trace file location (NULL if none) */

const char *dump_traces();

cmds/dumpstate/utils.cpp

+16 −4
Original line number Diff line number Diff line
@@ -657,23 +657,35 @@ void redirect_to_socket(FILE *redirect, const char *service) { 
    close(fd);

}



/* redirect output to a file */

void redirect_to_file(FILE *redirect, char *path) {

    char *chp = path;

void create_parent_dirs(const char *path) {

    char *chp = (char*) path;



    /* skip initial slash */

    if (chp[0] == '/')

        chp++;



    /* create leading directories, if necessary */

    struct stat dir_stat;

    while (chp && chp[0]) {

        chp = strchr(chp, '/');

        if (chp) {

            *chp = 0;

            mkdir(path, 0770);  /* drwxrwx--- */

            if (stat(path, &dir_stat) == -1 || !S_ISDIR(dir_stat.st_mode)) {

                ALOGI("Creating directory %s\n", path);

                if (mkdir(path, 0770)) { /* drwxrwx--- */

                    ALOGE("Unable to create directory %s: %s\n", path, strerror(errno));

                } else if (chown(path, AID_SHELL, AID_SHELL)) {

                    ALOGE("Unable to change ownership of dir %s: %s\n", path, strerror(errno));

                }

            }

            *chp++ = '/';

        }

    }

}



/* redirect output to a file */

void redirect_to_file(FILE *redirect, char *path) {

    create_parent_dirs(path);



    int fd = TEMP_FAILURE_RETRY(open(path, O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW,

                                     S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH));