Merge "Improve fastboot verbose output."
am: be5332d236

Change-Id: I16cb4351bf9f9e6aa9f8de3b28bfff63a82c3f6a
diff --git a/fastboot/engine.cpp b/fastboot/engine.cpp
index 875e7b9..551ddba 100644
--- a/fastboot/engine.cpp
+++ b/fastboot/engine.cpp
@@ -313,6 +313,7 @@
         a->start = now();
         if (!a->msg.empty()) {
             fprintf(stderr, "%-50s ", a->msg.c_str());
+            verbose("\n");
         }
         if (a->op == OP_DOWNLOAD) {
             status = fb_download_data(transport, a->data, a->size);
diff --git a/fastboot/fastboot.cpp b/fastboot/fastboot.cpp
index 7a30aee..22af194 100644
--- a/fastboot/fastboot.cpp
+++ b/fastboot/fastboot.cpp
@@ -95,7 +95,6 @@
 
 static bool g_disable_verity = false;
 static bool g_disable_verification = false;
-static bool g_verbose = false;
 
 static const std::string convert_fbe_marker_filename("convert_fbe");
 
@@ -239,7 +238,7 @@
 // Opens a new Transport connected to a device. If |serial| is non-null it will be used to identify
 // a specific device, otherwise the first USB device found will be used.
 //
-// If |serial| is non-null but invalid, this prints an error message to stderr and returns nullptr.
+// If |serial| is non-null but invalid, this exits.
 // Otherwise it blocks until the target is available.
 //
 // The returned Transport is a singleton, so multiple calls to this function will return the same
@@ -271,9 +270,7 @@
         if (net_address != nullptr) {
             std::string error;
             if (!android::base::ParseNetAddress(net_address, &host, &port, nullptr, &error)) {
-                fprintf(stderr, "error: Invalid network address '%s': %s\n", net_address,
-                        error.c_str());
-                return nullptr;
+                die("invalid network address '%s': %s\n", net_address, error.c_str());
             }
         }
     }
@@ -541,7 +538,7 @@
     die("make_temporary_directory not supported under Windows, sorry!");
 }
 
-static int make_temporary_fd() {
+static int make_temporary_fd(const char* /*what*/) {
     // TODO: reimplement to avoid leaking a FILE*.
     return fileno(tmpfile());
 }
@@ -557,18 +554,16 @@
 static std::string make_temporary_directory() {
     std::string result(make_temporary_template());
     if (mkdtemp(&result[0]) == nullptr) {
-        fprintf(stderr, "Unable to create temporary directory: %s\n", strerror(errno));
-        return "";
+        die("unable to create temporary directory: %s", strerror(errno));
     }
     return result;
 }
 
-static int make_temporary_fd() {
+static int make_temporary_fd(const char* what) {
     std::string path_template(make_temporary_template());
     int fd = mkstemp(&path_template[0]);
     if (fd == -1) {
-        fprintf(stderr, "Unable to create temporary file: %s\n", strerror(errno));
-        return -1;
+        die("failed to create temporary file for %s: %s\n", what, strerror(errno));
     }
     unlink(path_template.c_str());
     return fd;
@@ -578,16 +573,11 @@
 
 static std::string create_fbemarker_tmpdir() {
     std::string dir = make_temporary_directory();
-    if (dir.empty()) {
-        fprintf(stderr, "Unable to create local temp directory for FBE marker\n");
-        return "";
-    }
     std::string marker_file = dir + "/" + convert_fbe_marker_filename;
     int fd = open(marker_file.c_str(), O_CREAT | O_WRONLY | O_CLOEXEC, 0666);
     if (fd == -1) {
-        fprintf(stderr, "Unable to create FBE marker file %s locally: %d, %s\n",
-            marker_file.c_str(), errno, strerror(errno));
-        return "";
+        die("unable to create FBE marker file %s locally: %s",
+            marker_file.c_str(), strerror(errno));
     }
     close(fd);
     return dir;
@@ -608,10 +598,7 @@
 }
 
 static int unzip_to_file(ZipArchiveHandle zip, const char* entry_name) {
-    unique_fd fd(make_temporary_fd());
-    if (fd == -1) {
-        die("failed to create temporary file for '%s': %s", entry_name, strerror(errno));
-    }
+    unique_fd fd(make_temporary_fd(entry_name));
 
     ZipString zip_entry_name(entry_name);
     ZipEntry zip_entry;
@@ -772,8 +759,8 @@
 static int64_t get_target_sparse_limit(Transport* transport) {
     std::string max_download_size;
     if (!fb_getvar(transport, "max-download-size", &max_download_size) ||
-            max_download_size.empty()) {
-        fprintf(stderr, "target didn't report max-download-size\n");
+        max_download_size.empty()) {
+        verbose("target didn't report max-download-size");
         return 0;
     }
 
@@ -785,9 +772,7 @@
         fprintf(stderr, "couldn't parse max-download-size '%s'\n", max_download_size.c_str());
         return 0;
     }
-    if (g_verbose && limit > 0) {
-        fprintf(stderr, "Target reported max download size of %" PRId64 " bytes\n", limit);
-    }
+    if (limit > 0) verbose("target reported max download size of %" PRId64 " bytes", limit);
     return limit;
 }
 
@@ -878,10 +863,7 @@
         return;
     }
 
-    int fd = make_temporary_fd();
-    if (fd == -1) {
-        die("Failed to create temporary file for vbmeta rewriting");
-    }
+    int fd = make_temporary_fd("vbmeta rewriting");
 
     std::string data;
     if (!android::base::ReadFdToString(buf->fd, &data)) {
@@ -1593,7 +1575,7 @@
             erase_first = false;
             break;
         case 'v':
-            g_verbose = true;
+            set_verbose();
             break;
         case 'w':
             wants_wipe = true;
@@ -1624,9 +1606,7 @@
                 set_fbe_marker = true;
 #endif
             } else {
-                fprintf(stderr, "Internal error in options processing for %s\n",
-                    longopts[longindex].name);
-                return 1;
+                die("unknown option %s", longopts[longindex].name);
             }
             break;
         default:
@@ -1855,14 +1835,10 @@
     }
 
     if (wants_wipe) {
-        fprintf(stderr, "wiping userdata...\n");
         fb_queue_erase("userdata");
         if (set_fbe_marker) {
-            fprintf(stderr, "setting FBE marker...\n");
+            fprintf(stderr, "setting FBE marker on initial userdata...\n");
             std::string initial_userdata_dir = create_fbemarker_tmpdir();
-            if (initial_userdata_dir.empty()) {
-                return 1;
-            }
             fb_perform_format(transport, "userdata", 1, "", "", initial_userdata_dir);
             delete_fbemarker_tmpdir(initial_userdata_dir);
         } else {
@@ -1871,7 +1847,6 @@
 
         std::string cache_type;
         if (fb_getvar(transport, "partition-type:cache", &cache_type) && !cache_type.empty()) {
-            fprintf(stderr, "wiping cache...\n");
             fb_queue_erase("cache");
             fb_perform_format(transport, "cache", 1, "", "", "");
         }
diff --git a/fastboot/fastboot.h b/fastboot/fastboot.h
index a31057a..005ba5a 100644
--- a/fastboot/fastboot.h
+++ b/fastboot/fastboot.h
@@ -75,6 +75,7 @@
 /* util stuff */
 double now();
 char* xstrdup(const char*);
+void set_verbose();
 
 // These printf-like functions are implemented in terms of vsnprintf, so they
 // use the same attribute for compile-time format string checking. On Windows,
@@ -90,6 +91,7 @@
 #endif
 void die(const char* fmt, ...) __attribute__((__noreturn__))
 __attribute__((__format__(FASTBOOT_FORMAT_ARCHETYPE, 1, 2)));
+void verbose(const char* fmt, ...) __attribute__((__format__(FASTBOOT_FORMAT_ARCHETYPE, 1, 2)));
 #undef FASTBOOT_FORMAT_ARCHETYPE
 
 /* Current product */
diff --git a/fastboot/protocol.cpp b/fastboot/protocol.cpp
index c239861..133a2d0 100644
--- a/fastboot/protocol.cpp
+++ b/fastboot/protocol.cpp
@@ -75,18 +75,21 @@
         }
 
         if (!memcmp(status, "INFO", 4)) {
-            fprintf(stderr,"(bootloader) %s\n", status + 4);
+            verbose("received INFO %s", status + 4);
+            fprintf(stderr, "(bootloader) %s\n", status + 4);
             continue;
         }
 
         if (!memcmp(status, "OKAY", 4)) {
+            verbose("received OKAY %s", status + 4);
             if (response) {
-                strcpy(response, (char*) status + 4);
+                strcpy(response, status + 4);
             }
             return 0;
         }
 
         if (!memcmp(status, "FAIL", 4)) {
+            verbose("received FAIL %s", status + 4);
             if (r > 4) {
                 g_error = android::base::StringPrintf("remote: %s", status + 4);
             } else {
@@ -96,6 +99,7 @@
         }
 
         if (!memcmp(status, "DATA", 4) && size > 0){
+            verbose("received DATA %s", status + 4);
             uint32_t dsize = strtol(status + 4, 0, 16);
             if (dsize > size) {
                 g_error = android::base::StringPrintf("data size too large (%d)", dsize);
@@ -105,6 +109,7 @@
             return dsize;
         }
 
+        verbose("received unknown status code \"%4.4s\"", status);
         g_error = "unknown status code";
         transport->Close();
         break;
@@ -124,6 +129,8 @@
         response[0] = 0;
     }
 
+    verbose("sending command \"%s\"", cmd.c_str());
+
     if (transport->Write(cmd.c_str(), cmd.size()) != static_cast<int>(cmd.size())) {
         g_error = android::base::StringPrintf("command write failed (%s)", strerror(errno));
         transport->Close();
@@ -134,6 +141,8 @@
 }
 
 static int64_t _command_write_data(Transport* transport, const void* data, uint32_t size) {
+    verbose("sending data (%" PRIu32 " bytes)", size);
+
     int64_t r = transport->Write(data, size);
     if (r < 0) {
         g_error = android::base::StringPrintf("data write failure (%s)", strerror(errno));
@@ -149,6 +158,8 @@
 }
 
 static int64_t _command_read_data(Transport* transport, void* data, uint32_t size) {
+    verbose("reading data (%" PRIu32 " bytes)", size);
+
     int64_t r = transport->Read(data, size);
     if (r < 0) {
         g_error = android::base::StringPrintf("data read failure (%s)", strerror(errno));
diff --git a/fastboot/util.cpp b/fastboot/util.cpp
index 53fc1be..140270f 100644
--- a/fastboot/util.cpp
+++ b/fastboot/util.cpp
@@ -35,6 +35,8 @@
 
 #include "fastboot.h"
 
+static bool g_verbose = false;
+
 double now() {
     struct timeval tv;
     gettimeofday(&tv, NULL);
@@ -46,11 +48,28 @@
     va_start(ap, fmt);
     fprintf(stderr, "fastboot: error: ");
     vfprintf(stderr, fmt, ap);
-    fprintf(stderr,"\n");
+    fprintf(stderr, "\n");
     va_end(ap);
     exit(EXIT_FAILURE);
 }
 
+void set_verbose() {
+    g_verbose = true;
+}
+
+void verbose(const char* fmt, ...) {
+    if (!g_verbose) return;
+
+    if (*fmt != '\n') {
+        va_list ap;
+        va_start(ap, fmt);
+        fprintf(stderr, "fastboot: verbose: ");
+        vfprintf(stderr, fmt, ap);
+        va_end(ap);
+    }
+    fprintf(stderr, "\n");
+}
+
 char* xstrdup(const char* s) {
     char* result = strdup(s);
     if (!result) die("out of memory");