Added a -p option to bugreportz to show progress.
BUG: 28609499
Change-Id: I1e60078dfda7e5679fbd19f4981a5dc7a69c4fc7
diff --git a/cmds/bugreportz/bugreportz.cpp b/cmds/bugreportz/bugreportz.cpp
index 4b81c91..7bed284 100644
--- a/cmds/bugreportz/bugreportz.cpp
+++ b/cmds/bugreportz/bugreportz.cpp
@@ -20,9 +20,27 @@
#include <string.h>
#include <unistd.h>
+#include <string>
+
+#include <android-base/file.h>
+#include <android-base/strings.h>
+
#include "bugreportz.h"
-int bugreportz(int s) {
+static constexpr char PROGRESS_PREFIX[] = "PROGRESS:";
+
+static void write_line(const std::string& line, bool show_progress) {
+ if (line.empty()) return;
+
+ // When not invoked with the -p option, it must skip PROGRESS lines otherwise it
+ // will break adb (which is expecting either OK or FAIL).
+ if (!show_progress && android::base::StartsWith(line, PROGRESS_PREFIX)) return;
+
+ android::base::WriteStringToFd(line, STDOUT_FILENO);
+}
+
+int bugreportz(int s, bool show_progress) {
+ std::string line;
while (1) {
char buffer[65536];
ssize_t bytes_read = TEMP_FAILURE_RETRY(read(s, buffer, sizeof(buffer)));
@@ -37,21 +55,18 @@
break;
}
- ssize_t bytes_to_send = bytes_read;
- ssize_t bytes_written;
- do {
- bytes_written = TEMP_FAILURE_RETRY(
- write(STDOUT_FILENO, buffer + bytes_read - bytes_to_send, bytes_to_send));
- if (bytes_written == -1) {
- fprintf(stderr,
- "Failed to write data to stdout: read %zd, trying to send %zd "
- "(%s)\n",
- bytes_read, bytes_to_send, strerror(errno));
- break;
+ // Writes line by line.
+ for (int i = 0; i < bytes_read; i++) {
+ char c = buffer[i];
+ line.append(1, c);
+ if (c == '\n') {
+ write_line(line, show_progress);
+ line.clear();
}
- bytes_to_send -= bytes_written;
- } while (bytes_written != 0 && bytes_to_send > 0);
+ }
}
+ // Process final line, in case it didn't finish with newline
+ write_line(line, show_progress);
if (close(s) == -1) {
fprintf(stderr, "WARNING: error closing socket: %s\n", strerror(errno));
diff --git a/cmds/bugreportz/bugreportz.h b/cmds/bugreportz/bugreportz.h
index d2b79b9..304e4b3 100644
--- a/cmds/bugreportz/bugreportz.h
+++ b/cmds/bugreportz/bugreportz.h
@@ -16,6 +16,6 @@
#define BUGREPORTZ_H
// Calls dumpstate using the given socket and output its result to stdout.
-int bugreportz(int s);
+int bugreportz(int s, bool show_progress);
#endif // BUGREPORTZ_H
diff --git a/cmds/bugreportz/bugreportz_test.cpp b/cmds/bugreportz/bugreportz_test.cpp
index fb6cdc7..dfef17f 100644
--- a/cmds/bugreportz/bugreportz_test.cpp
+++ b/cmds/bugreportz/bugreportz_test.cpp
@@ -70,12 +70,12 @@
// Tests must call WriteToSocket() to set what's written prior to calling it, since the writing
// end of the pipe will be closed before calling bugreportz() (otherwise that function would
// hang).
- void Bugreportz() {
+ void Bugreportz(bool show_progress) {
close(write_fd_);
write_fd_ = -1;
CaptureStdout();
- int status = bugreportz(read_fd_);
+ int status = bugreportz(read_fd_, show_progress);
close(read_fd_);
read_fd_ = -1;
@@ -90,12 +90,36 @@
std::string stdout_;
};
-// Tests 'bugreportz', without any argument - it will just echo dumpstate's output to stdout.
+// Tests 'bugreportz', without any argument - it will ignore progress lines.
TEST_F(BugreportzTest, NoArgument) {
WriteToSocket("What happens on 'dumpstate',");
WriteToSocket("stays on 'bugreportz'.\n");
+ WriteToSocket("PROGRESS:Y U NO OMITTED?\n"); // Should be ommited.
+ WriteToSocket("But ");
+ WriteToSocket("PROGRESS IN THE MIDDLE"); // Ok - not starting a line.
+ WriteToSocket(" is accepted\n");
- Bugreportz();
+ Bugreportz(false);
- AssertStdoutEquals("What happens on 'dumpstate',stays on 'bugreportz'.\n");
+ AssertStdoutEquals(
+ "What happens on 'dumpstate',stays on 'bugreportz'.\n"
+ "But PROGRESS IN THE MIDDLE is accepted\n");
+}
+
+// Tests 'bugreportz -p' - it will just echo dumpstate's output to stdout
+TEST_F(BugreportzTest, WithProgress) {
+ WriteToSocket("What happens on 'dumpstate',");
+ WriteToSocket("stays on 'bugreportz'.\n");
+ WriteToSocket("PROGRESS:IS INEVITABLE\n");
+ WriteToSocket("PROG");
+ WriteToSocket("RESS:IS NOT AUTOMATIC\n");
+ WriteToSocket("Newline is optional");
+
+ Bugreportz(true);
+
+ AssertStdoutEquals(
+ "What happens on 'dumpstate',stays on 'bugreportz'.\n"
+ "PROGRESS:IS INEVITABLE\n"
+ "PROGRESS:IS NOT AUTOMATIC\n"
+ "Newline is optional");
}
diff --git a/cmds/bugreportz/main.cpp b/cmds/bugreportz/main.cpp
index 6fa33cc..a3ae1ff 100644
--- a/cmds/bugreportz/main.cpp
+++ b/cmds/bugreportz/main.cpp
@@ -26,12 +26,13 @@
#include "bugreportz.h"
-static constexpr char VERSION[] = "1.0";
+static constexpr char VERSION[] = "1.1";
static void show_usage() {
fprintf(stderr,
"usage: bugreportz [-h | -v]\n"
" -h: to display this help message\n"
+ " -p: display progress\n"
" -v: to display the version\n"
" or no arguments to generate a zipped bugreport\n");
}
@@ -41,14 +42,18 @@
}
int main(int argc, char* argv[]) {
+ bool show_progress = false;
if (argc > 1) {
/* parse arguments */
int c;
- while ((c = getopt(argc, argv, "vh")) != -1) {
+ while ((c = getopt(argc, argv, "hpv")) != -1) {
switch (c) {
case 'h':
show_usage();
return EXIT_SUCCESS;
+ case 'p':
+ show_progress = true;
+ break;
case 'v':
show_version();
return EXIT_SUCCESS;
@@ -57,11 +62,6 @@
return EXIT_FAILURE;
}
}
- // passed an argument not starting with -
- if (optind > 1 || argv[optind] != nullptr) {
- show_usage();
- return EXIT_FAILURE;
- }
}
// TODO: code below was copy-and-pasted from bugreport.cpp (except by the
@@ -95,5 +95,5 @@
fprintf(stderr, "WARNING: Cannot set socket timeout: %s\n", strerror(errno));
}
- bugreportz(s);
+ bugreportz(s, show_progress);
}
diff --git a/cmds/bugreportz/readme.md b/cmds/bugreportz/readme.md
index 85aafce..2bc277e 100644
--- a/cmds/bugreportz/readme.md
+++ b/cmds/bugreportz/readme.md
@@ -3,6 +3,11 @@
`bugreportz` is used to generate a zippped bugreport whose path is passed back to `adb`, using
the simple protocol defined below.
+# Version 1.1
+On version 1.1, in addition to the `OK` and `FAILURE` lines, `bugreportz -p` generates progress
+lines in the following format:
+
+- `PROGRESS:<progress>/<total>`, where `<progress>` is the current progress units out of a max of `<total>`.
## Version 1.0
On version 1.0, `bugreportz` does not generate any output on `stdout` until the bugreport is
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index 279c010..f74fe39 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -61,7 +61,7 @@
static std::unique_ptr<ZipWriter> zip_writer;
static std::set<std::string> mount_points;
void add_mountinfo();
-static int control_socket_fd;
+int control_socket_fd = -1;
/* suffix of the bugreport files - it's typically the date (when invoked with -d),
* although it could be changed by the user using a system property */
static std::string suffix;
@@ -1196,6 +1196,7 @@
if (use_control_socket) {
MYLOGD("Opening control socket\n");
control_socket_fd = open_socket("dumpstate");
+ do_update_progress = 1;
}
/* full path of the temporary file containing the bugreport */
@@ -1268,7 +1269,7 @@
add_text_zip_entry("version.txt", version);
}
- if (do_update_progress) {
+ if (do_update_progress && do_broadcast) {
std::vector<std::string> am_args = {
"--receiver-permission", "android.permission.DUMP", "--receiver-foreground",
"--es", "android.intent.extra.NAME", suffix,
@@ -1492,9 +1493,9 @@
fclose(stderr);
}
- if (use_control_socket && control_socket_fd >= 0) {
- MYLOGD("Closing control socket\n");
- close(control_socket_fd);
+ if (use_control_socket && control_socket_fd != -1) {
+ MYLOGD("Closing control socket\n");
+ close(control_socket_fd);
}
return 0;
diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h
index 4769974..5e083cc 100644
--- a/cmds/dumpstate/dumpstate.h
+++ b/cmds/dumpstate/dumpstate.h
@@ -79,7 +79,7 @@
* It would be better to take advantage of the C++ migration and encapsulate the state in an object,
* but that will be better handled in a major C++ refactoring, which would also get rid of other C
* idioms (like using std::string instead of char*, removing varargs, etc...) */
-extern int do_update_progress, progress, weight_total;
+extern int do_update_progress, progress, weight_total, control_socket_fd;
/* full path of the directory where the bugreport files will be written */
extern std::string bugreport_dir;
diff --git a/cmds/dumpstate/utils.cpp b/cmds/dumpstate/utils.cpp
index 3c39129..fd6413d 100644
--- a/cmds/dumpstate/utils.cpp
+++ b/cmds/dumpstate/utils.cpp
@@ -1214,6 +1214,11 @@
fprintf(stderr, "Setting progress (%s): %s/%d\n", key, value, weight_total);
}
+ if (control_socket_fd >= 0) {
+ dprintf(control_socket_fd, "PROGRESS:%d/%d\n", progress, weight_total);
+ fsync(control_socket_fd);
+ }
+
int status = property_set(key, value);
if (status) {
MYLOGE("Could not update progress by setting system property %s to %s: %d\n",