Commit: aed7048ce0c8ff20a2ed7ce46a848d8af1d3142d - system-coremmmm (git)

system/corennnnn

Commit MetaInfo

• Révision: aed7048ce0c8ff20a2ed7ce46a848d8af1d3142d (tree)
• l'heure: 2016-08-04 23:44:35
• Auteur: TreeHugger Robot <treehugger-gerrit@goog...>
• Commiter: Android (Google) Code Review

Message de Log

Merge changes from topic 'bug_30451114' into nyc-mr1-dev

* changes:

Deprecated 'adb bugreport' with flat files.
Minor improvements on bugreport generation.

Change Summary

• modified: adb/bugreport.cpp
• modified: adb/bugreport_test.cpp
• modified: adb/commandline.cpp

Modification

--- a/adb/bugreport.cpp
+++ b/adb/bugreport.cpp
@@ -14,28 +14,38 @@
  * limitations under the License.
  */
 
+#include "bugreport.h"
+
 #include <string>
+#include <vector>
 
 #include <android-base/strings.h>
 
-#include "bugreport.h"
+#include "sysdeps.h"
+#include "adb_utils.h"
 #include "file_sync_service.h"
 
-static constexpr char BUGZ_OK_PREFIX[] = "OK:";
-static constexpr char BUGZ_FAIL_PREFIX[] = "FAIL:";
+static constexpr char BUGZ_BEGIN_PREFIX[] = "BEGIN:";
 static constexpr char BUGZ_PROGRESS_PREFIX[] = "PROGRESS:";
 static constexpr char BUGZ_PROGRESS_SEPARATOR[] = "/";
+static constexpr char BUGZ_OK_PREFIX[] = "OK:";
+static constexpr char BUGZ_FAIL_PREFIX[] = "FAIL:";
 
 // Custom callback used to handle the output of zipped bugreports.
 class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface {
   public:
-    BugreportStandardStreamsCallback(const std::string& dest_file, bool show_progress, Bugreport* br)
+    BugreportStandardStreamsCallback(const std::string& dest_dir, const std::string& dest_file,
+                                     bool show_progress, Bugreport* br)
         : br_(br),
+          src_file_(),
+          dest_dir_(dest_dir),
           dest_file_(dest_file),
-          line_message_(android::base::StringPrintf("generating %s", dest_file_.c_str())),
+          line_message_(),
+          invalid_lines_(),
           show_progress_(show_progress),
-          status_(-1),
+          status_(0),
           line_() {
+        SetLineMessage();
     }
 
     void OnStdout(const char* buffer, int length) {
@@ -54,28 +64,72 @@ class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface
         OnStream(nullptr, stderr, buffer, length);
     }
     int Done(int unused_) {
-        // Process remaining line, if any...
+        // Process remaining line, if any.
         ProcessLine(line_);
-        // ..then return.
+
+        // Warn about invalid lines, if any.
+        if (!invalid_lines_.empty()) {
+            fprintf(stderr,
+                    "WARNING: bugreportz generated %zu line(s) with unknown commands, "
+                    "device might not support zipped bugreports:\n",
+                    invalid_lines_.size());
+            for (const auto& line : invalid_lines_) {
+                fprintf(stderr, "\t%s\n", line.c_str());
+            }
+            fprintf(stderr,
+                    "If the zipped bugreport was not generated, try 'adb bugreport' instead.\n");
+        }
+
+        // Pull the generated bug report.
+        if (status_ == 0) {
+            if (src_file_.empty()) {
+                fprintf(stderr, "bugreportz did not return a '%s' or '%s' line\n", BUGZ_OK_PREFIX,
+                        BUGZ_FAIL_PREFIX);
+                return -1;
+            }
+            std::string destination;
+            if (dest_dir_.empty()) {
+                destination = dest_file_;
+            } else {
+                destination = android::base::StringPrintf("%s%c%s", dest_dir_.c_str(),
+                                                          OS_PATH_SEPARATOR, dest_file_.c_str());
+            }
+            std::vector<const char*> srcs{src_file_.c_str()};
+            status_ =
+                br_->DoSyncPull(srcs, destination.c_str(), true, line_message_.c_str()) ? 0 : 1;
+            if (status_ != 0) {
+                fprintf(stderr,
+                        "Bug report finished but could not be copied to '%s'.\n"
+                        "Try to run 'adb pull %s <directory>'\n"
+                        "to copy it to a directory that can be written.\n",
+                        destination.c_str(), src_file_.c_str());
+            }
+        }
         return status_;
     }
 
   private:
+    void SetLineMessage() {
+        line_message_ =
+            android::base::StringPrintf("generating %s", adb_basename(dest_file_).c_str());
+    }
+
+    void SetSrcFile(const std::string path) {
+        src_file_ = path;
+        if (!dest_dir_.empty()) {
+            // Only uses device-provided name when user passed a directory.
+            dest_file_ = adb_basename(path);
+            SetLineMessage();
+        }
+    }
+
     void ProcessLine(const std::string& line) {
         if (line.empty()) return;
 
-        if (android::base::StartsWith(line, BUGZ_OK_PREFIX)) {
-            if (show_progress_) {
-                // Make sure pull message doesn't conflict with generation message.
-                br_->UpdateProgress(line_message_, 100, 100);
-            }
-
-            const char* zip_file = &line[strlen(BUGZ_OK_PREFIX)];
-            std::vector<const char*> srcs{zip_file};
-            status_ = br_->DoSyncPull(srcs, dest_file_.c_str(), true, line_message_.c_str()) ? 0 : 1;
-            if (status_ != 0) {
-                fprintf(stderr, "Could not copy file '%s' to '%s'\n", zip_file, dest_file_.c_str());
-            }
+        if (android::base::StartsWith(line, BUGZ_BEGIN_PREFIX)) {
+            SetSrcFile(&line[strlen(BUGZ_BEGIN_PREFIX)]);
+        } else if (android::base::StartsWith(line, BUGZ_OK_PREFIX)) {
+            SetSrcFile(&line[strlen(BUGZ_OK_PREFIX)]);
         } else if (android::base::StartsWith(line, BUGZ_FAIL_PREFIX)) {
             const char* error_message = &line[strlen(BUGZ_FAIL_PREFIX)];
             fprintf(stderr, "Device failed to take a zipped bugreport: %s\n", error_message);
@@ -89,24 +143,38 @@ class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface
             size_t idx2 = line.rfind(BUGZ_PROGRESS_SEPARATOR);
             int progress = std::stoi(line.substr(idx1, (idx2 - idx1)));
             int total = std::stoi(line.substr(idx2 + 1));
-            br_->UpdateProgress(dest_file_, progress, total);
+            br_->UpdateProgress(line_message_, progress, total);
         } else {
-            fprintf(stderr,
-                    "WARNING: unexpected line (%s) returned by bugreportz, "
-                    "device probably does not support zipped bugreports.\n"
-                    "Try 'adb bugreport' instead.",
-                    line.c_str());
+            invalid_lines_.push_back(line);
         }
     }
 
     Bugreport* br_;
-    const std::string dest_file_;
-    const std::string line_message_;
+
+    // Path of bugreport on device.
+    std::string src_file_;
+
+    // Bugreport destination on host, depending on argument passed on constructor:
+    // - if argument is a directory, dest_dir_ is set with it and dest_file_ will be the name
+    //   of the bugreport reported by the device.
+    // - if argument is empty, dest_dir is set as the current directory and dest_file_ will be the
+    //   name of the bugreport reported by the device.
+    // - otherwise, dest_dir_ is not set and dest_file_ is set with the value passed on constructor.
+    std::string dest_dir_, dest_file_;
+
+    // Message displayed on LinePrinter, it's updated every time the destination above change.
+    std::string line_message_;
+
+    // Lines sent by bugreportz that contain invalid commands; will be displayed at the end.
+    std::vector<std::string> invalid_lines_;
+
+    // Whether PROGRESS_LINES should be interpreted as progress.
     bool show_progress_;
+
+    // Overall process of the operation, as returned by Done().
     int status_;
 
-    // Temporary buffer containing the characters read since the last newline
-    // (\n).
+    // Temporary buffer containing the characters read since the last newline (\n).
     std::string line_;
 
     DISALLOW_COPY_AND_ASSIGN(BugreportStandardStreamsCallback);
@@ -116,34 +184,54 @@ class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface
 int usage();
 
 int Bugreport::DoIt(TransportType transport_type, const char* serial, int argc, const char** argv) {
-    if (argc == 1) return SendShellCommand(transport_type, serial, "bugreport", false);
-    if (argc != 2) return usage();
-
-    // Zipped bugreport option - will call 'bugreportz', which prints the location
-    // of the generated
-    // file, then pull it to the destination file provided by the user.
-    std::string dest_file = argv[1];
-    if (!android::base::EndsWith(argv[1], ".zip")) {
-        // TODO: use a case-insensitive comparison (like EndsWithIgnoreCase
-        dest_file += ".zip";
-    }
+    if (argc > 2) return usage();
 
     // Gets bugreportz version.
     std::string bugz_stderr;
     DefaultStandardStreamsCallback version_callback(nullptr, &bugz_stderr);
     int status = SendShellCommand(transport_type, serial, "bugreportz -v", false, &version_callback);
+    std::string bugz_version = android::base::Trim(bugz_stderr);
+
+    if (status != 0 || bugz_version.empty()) {
+        // Device does not support bugreportz: if called as 'adb bugreport', just falls out to the
+        // flat-file version
+        if (argc == 1) return SendShellCommand(transport_type, serial, "bugreport", false);
 
-    if (status != 0) {
+        // But if user explicitly asked for a zipped bug report, fails instead (otherwise calling
+        // 'bugreport' would generate a lot of output the user might not be prepared to handle)
         fprintf(stderr,
-                "Failed to get bugreportz version: 'bugreport -v' returned '%s' "
-                "(code %d)."
-                "\nIf the device does not support it, try running 'adb bugreport' "
-                "to get a "
-                "flat-file bugreport.",
+                "Failed to get bugreportz version: 'bugreportz -v' returned '%s' (code %d).\n"
+                "If the device runs Android M or below, try 'adb bugreport' instead.\n",
                 bugz_stderr.c_str(), status);
-        return status;
+        return status != 0 ? status : -1;
+    }
+
+    std::string dest_file, dest_dir;
+
+    if (argc == 1) {
+        // No args - use current directory
+        if (!getcwd(&dest_dir)) {
+            perror("adb: getcwd failed");
+            return 1;
+        }
+    } else {
+        // Check whether argument is a directory or file
+        if (directory_exists(argv[1])) {
+            dest_dir = argv[1];
+        } else {
+            dest_file = argv[1];
+        }
+    }
+
+    if (dest_file.empty()) {
+        // Uses a default value until device provides the proper name
+        dest_file = "bugreport.zip";
+    } else {
+        if (!android::base::EndsWith(dest_file, ".zip")) {
+            // TODO: use a case-insensitive comparison (like EndsWithIgnoreCase
+            dest_file += ".zip";
+        }
     }
-    std::string bugz_version = android::base::Trim(bugz_stderr);
 
     bool show_progress = true;
     std::string bugz_command = "bugreportz -p";
@@ -153,12 +241,11 @@ int Bugreport::DoIt(TransportType transport_type, const char* serial, int argc,
         fprintf(stderr,
                 "Bugreport is in progress and it could take minutes to complete.\n"
                 "Please be patient and do not cancel or disconnect your device "
-                "until it completes."
-                "\n");
+                "until it completes.\n");
         show_progress = false;
         bugz_command = "bugreportz";
     }
-    BugreportStandardStreamsCallback bugz_callback(dest_file, show_progress, this);
+    BugreportStandardStreamsCallback bugz_callback(dest_dir, dest_file, show_progress, this);
     return SendShellCommand(transport_type, serial, bugz_command, false, &bugz_callback);
 }
 

--- a/adb/bugreport_test.cpp
+++ b/adb/bugreport_test.cpp
@@ -19,6 +19,12 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+#include <android-base/strings.h>
+#include <android-base/test_utils.h>
+
+#include "sysdeps.h"
+#include "adb_utils.h"
+
 using ::testing::_;
 using ::testing::Action;
 using ::testing::ActionInterface;
@@ -122,28 +128,38 @@ class BugreportMock : public Bugreport {
 
 class BugreportTest : public ::testing::Test {
   public:
-    void SetBugreportzVersion(const std::string& version) {
+    void SetUp() {
+        if (!getcwd(&cwd_)) {
+            ADD_FAILURE() << "getcwd failed: " << strerror(errno);
+            return;
+        }
+    }
+
+    void ExpectBugreportzVersion(const std::string& version) {
         EXPECT_CALL(br_,
                     SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -v", false, _))
             .WillOnce(DoAll(WithArg<4>(WriteOnStderr(version.c_str())),
                             WithArg<4>(ReturnCallbackDone(0))));
     }
 
-    void ExpectProgress(int progress, int total) {
-        EXPECT_CALL(br_, UpdateProgress(HasSubstr("file.zip"), progress, total));
+    void ExpectProgress(int progress, int total, const std::string& file = "file.zip") {
+        EXPECT_CALL(br_, UpdateProgress(StrEq("generating " + file), progress, total));
     }
 
     BugreportMock br_;
+    std::string cwd_;  // TODO: make it static
 };
 
-// Tests when called with invalid number of argumnts
+// Tests when called with invalid number of arguments
 TEST_F(BugreportTest, InvalidNumberArgs) {
     const char* args[1024] = {"bugreport", "to", "principal"};
     ASSERT_EQ(-42, br_.DoIt(kTransportLocal, "HannibalLecter", 3, args));
 }
 
-// Tests the legacy 'adb bugreport' option
-TEST_F(BugreportTest, FlatFileFormat) {
+// Tests the 'adb bugreport' option when the device does not support 'bugreportz' - it falls back
+// to the flat-file format ('bugreport' binary on device)
+TEST_F(BugreportTest, NoArgumentsPreNDevice) {
+    ExpectBugreportzVersion("");
     EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreport", false, _))
         .WillOnce(Return(0));
 
@@ -151,15 +167,52 @@ TEST_F(BugreportTest, FlatFileFormat) {
     ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 1, args));
 }
 
-// Tests 'adb bugreport file.zip' when it succeeds and device does not support
-// progress.
-TEST_F(BugreportTest, OkLegacy) {
-    SetBugreportzVersion("1.0");
+// Tests the 'adb bugreport' option when the device supports 'bugreportz' version 1.0 - it will
+// save the bugreport in the current directory with the name provided by the device.
+TEST_F(BugreportTest, NoArgumentsNDevice) {
+    ExpectBugreportzVersion("1.0");
+
+    std::string dest_file =
+        android::base::StringPrintf("%s%cda_bugreport.zip", cwd_.c_str(), OS_PATH_SEPARATOR);
+    EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz", false, _))
+        .WillOnce(DoAll(WithArg<4>(WriteOnStdout("OK:/device/da_bugreport.zip")),
+                        WithArg<4>(ReturnCallbackDone())));
+    EXPECT_CALL(br_, DoSyncPull(ElementsAre(StrEq("/device/da_bugreport.zip")), StrEq(dest_file),
+                                true, StrEq("generating da_bugreport.zip")))
+        .WillOnce(Return(true));
+
+    const char* args[1024] = {"bugreport"};
+    ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 1, args));
+}
+
+// Tests the 'adb bugreport' option when the device supports 'bugreportz' version 1.1 - it will
+// save the bugreport in the current directory with the name provided by the device.
+TEST_F(BugreportTest, NoArgumentsPostNDevice) {
+    ExpectBugreportzVersion("1.1");
+    std::string dest_file =
+        android::base::StringPrintf("%s%cda_bugreport.zip", cwd_.c_str(), OS_PATH_SEPARATOR);
+    ExpectProgress(50, 100, "da_bugreport.zip");
+    EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _))
+        .WillOnce(DoAll(WithArg<4>(WriteOnStdout("BEGIN:/device/da_bugreport.zip\n")),
+                        WithArg<4>(WriteOnStdout("PROGRESS:50/100\n")),
+                        WithArg<4>(WriteOnStdout("OK:/device/da_bugreport.zip\n")),
+                        WithArg<4>(ReturnCallbackDone())));
+    EXPECT_CALL(br_, DoSyncPull(ElementsAre(StrEq("/device/da_bugreport.zip")), StrEq(dest_file),
+                                true, StrEq("generating da_bugreport.zip")))
+        .WillOnce(Return(true));
+
+    const char* args[1024] = {"bugreport"};
+    ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 1, args));
+}
+
+// Tests 'adb bugreport file.zip' when it succeeds and device does not support progress.
+TEST_F(BugreportTest, OkNDevice) {
+    ExpectBugreportzVersion("1.0");
     EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz", false, _))
         .WillOnce(DoAll(WithArg<4>(WriteOnStdout("OK:/device/bugreport.zip")),
                         WithArg<4>(ReturnCallbackDone())));
     EXPECT_CALL(br_, DoSyncPull(ElementsAre(StrEq("/device/bugreport.zip")), StrEq("file.zip"),
-                                true, HasSubstr("file.zip")))
+                                true, StrEq("generating file.zip")))
         .WillOnce(Return(true));
 
     const char* args[1024] = {"bugreport", "file.zip"};
@@ -168,14 +221,14 @@ TEST_F(BugreportTest, OkLegacy) {
 
 // Tests 'adb bugreport file.zip' when it succeeds but response was sent in
 // multiple buffer writers and without progress updates.
-TEST_F(BugreportTest, OkLegacySplitBuffer) {
-    SetBugreportzVersion("1.0");
+TEST_F(BugreportTest, OkNDeviceSplitBuffer) {
+    ExpectBugreportzVersion("1.0");
     EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz", false, _))
         .WillOnce(DoAll(WithArg<4>(WriteOnStdout("OK:/device")),
                         WithArg<4>(WriteOnStdout("/bugreport.zip")),
                         WithArg<4>(ReturnCallbackDone())));
     EXPECT_CALL(br_, DoSyncPull(ElementsAre(StrEq("/device/bugreport.zip")), StrEq("file.zip"),
-                                true, HasSubstr("file.zip")))
+                                true, StrEq("generating file.zip")))
         .WillOnce(Return(true));
 
     const char* args[1024] = {"bugreport", "file.zip"};
@@ -183,20 +236,22 @@ TEST_F(BugreportTest, OkLegacySplitBuffer) {
 }
 
 // Tests 'adb bugreport file.zip' when it succeeds and displays progress.
-TEST_F(BugreportTest, Ok) {
-    SetBugreportzVersion("1.1");
+TEST_F(BugreportTest, OkProgress) {
+    ExpectBugreportzVersion("1.1");
     ExpectProgress(1, 100);
     ExpectProgress(10, 100);
     ExpectProgress(50, 100);
     ExpectProgress(99, 100);
-    ExpectProgress(100, 100);
     // clang-format off
     EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _))
+        // NOTE: DoAll accepts at most 10 arguments, and we're almost reached that limit...
         .WillOnce(DoAll(
+            // Name might change on OK, so make sure the right one is picked.
+            WithArg<4>(WriteOnStdout("BEGIN:/device/bugreport___NOT.zip\n")),
             // Progress line in one write
             WithArg<4>(WriteOnStdout("PROGRESS:1/100\n")),
             // Add some bogus lines
-            WithArg<4>(WriteOnStdout("\nDUDE:SWEET\n\n")),
+            WithArg<4>(WriteOnStdout("\nDUDE:SWEET\n\nBLA\n\nBLA\nBLA\n\n")),
             // Multiple progress lines in one write
             WithArg<4>(WriteOnStdout("PROGRESS:10/100\nPROGRESS:50/100\n")),
             // Progress line in multiple writes
@@ -207,32 +262,70 @@ TEST_F(BugreportTest, Ok) {
             WithArg<4>(WriteOnStdout("OK:/device/bugreport")),
             WithArg<4>(WriteOnStdout(".zip")),
             WithArg<4>(ReturnCallbackDone())));
+    // clang-format on
     EXPECT_CALL(br_, DoSyncPull(ElementsAre(StrEq("/device/bugreport.zip")), StrEq("file.zip"),
-                                true, HasSubstr("file.zip")))
+                                true, StrEq("generating file.zip")))
         .WillOnce(Return(true));
 
     const char* args[1024] = {"bugreport", "file.zip"};
     ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
 }
 
+// Tests 'adb bugreport dir' when it succeeds and destination is a directory.
+TEST_F(BugreportTest, OkDirectory) {
+    ExpectBugreportzVersion("1.1");
+    TemporaryDir td;
+    std::string dest_file =
+        android::base::StringPrintf("%s%cda_bugreport.zip", td.path, OS_PATH_SEPARATOR);
+
+    EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _))
+        .WillOnce(DoAll(WithArg<4>(WriteOnStdout("BEGIN:/device/da_bugreport.zip\n")),
+                        WithArg<4>(WriteOnStdout("OK:/device/da_bugreport.zip")),
+                        WithArg<4>(ReturnCallbackDone())));
+    EXPECT_CALL(br_, DoSyncPull(ElementsAre(StrEq("/device/da_bugreport.zip")), StrEq(dest_file),
+                                true, StrEq("generating da_bugreport.zip")))
+        .WillOnce(Return(true));
+
+    const char* args[1024] = {"bugreport", td.path};
+    ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
+}
+
 // Tests 'adb bugreport file' when it succeeds
 TEST_F(BugreportTest, OkNoExtension) {
-    SetBugreportzVersion("1.1");
-    ExpectProgress(100, 100);
+    ExpectBugreportzVersion("1.1");
     EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _))
         .WillOnce(DoAll(WithArg<4>(WriteOnStdout("OK:/device/bugreport.zip\n")),
                         WithArg<4>(ReturnCallbackDone())));
     EXPECT_CALL(br_, DoSyncPull(ElementsAre(StrEq("/device/bugreport.zip")), StrEq("file.zip"),
-                                true, HasSubstr("file.zip")))
+                                true, StrEq("generating file.zip")))
         .WillOnce(Return(true));
 
     const char* args[1024] = {"bugreport", "file"};
     ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
 }
 
+// Tests 'adb bugreport dir' when it succeeds and destination is a directory and device runs N.
+TEST_F(BugreportTest, OkNDeviceDirectory) {
+    ExpectBugreportzVersion("1.0");
+    TemporaryDir td;
+    std::string dest_file =
+        android::base::StringPrintf("%s%cda_bugreport.zip", td.path, OS_PATH_SEPARATOR);
+
+    EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz", false, _))
+        .WillOnce(DoAll(WithArg<4>(WriteOnStdout("BEGIN:/device/da_bugreport.zip\n")),
+                        WithArg<4>(WriteOnStdout("OK:/device/da_bugreport.zip")),
+                        WithArg<4>(ReturnCallbackDone())));
+    EXPECT_CALL(br_, DoSyncPull(ElementsAre(StrEq("/device/da_bugreport.zip")), StrEq(dest_file),
+                                true, StrEq("generating da_bugreport.zip")))
+        .WillOnce(Return(true));
+
+    const char* args[1024] = {"bugreport", td.path};
+    ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
+}
+
 // Tests 'adb bugreport file.zip' when the bugreport itself failed
 TEST_F(BugreportTest, BugreportzReturnedFail) {
-    SetBugreportzVersion("1.1");
+    ExpectBugreportzVersion("1.1");
     EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _))
         .WillOnce(
             DoAll(WithArg<4>(WriteOnStdout("FAIL:D'OH!\n")), WithArg<4>(ReturnCallbackDone())));
@@ -247,7 +340,7 @@ TEST_F(BugreportTest, BugreportzReturnedFail) {
 // was sent in
 // multiple buffer writes
 TEST_F(BugreportTest, BugreportzReturnedFailSplitBuffer) {
-    SetBugreportzVersion("1.1");
+    ExpectBugreportzVersion("1.1");
     EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _))
         .WillOnce(DoAll(WithArg<4>(WriteOnStdout("FAIL")), WithArg<4>(WriteOnStdout(":D'OH!\n")),
                         WithArg<4>(ReturnCallbackDone())));
@@ -261,7 +354,7 @@ TEST_F(BugreportTest, BugreportzReturnedFailSplitBuffer) {
 // Tests 'adb bugreport file.zip' when the bugreportz returned an unsupported
 // response.
 TEST_F(BugreportTest, BugreportzReturnedUnsupported) {
-    SetBugreportzVersion("1.1");
+    ExpectBugreportzVersion("1.1");
     EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _))
         .WillOnce(DoAll(WithArg<4>(WriteOnStdout("bugreportz? What am I, a zombie?")),
                         WithArg<4>(ReturnCallbackDone())));
@@ -281,9 +374,17 @@ TEST_F(BugreportTest, BugreportzVersionFailed) {
     ASSERT_EQ(666, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
 }
 
+// Tests 'adb bugreport file.zip' when the bugreportz -v returns status 0 but with no output.
+TEST_F(BugreportTest, BugreportzVersionEmpty) {
+    ExpectBugreportzVersion("");
+
+    const char* args[1024] = {"bugreport", "file.zip"};
+    ASSERT_EQ(-1, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
+}
+
 // Tests 'adb bugreport file.zip' when the main bugreportz command failed
 TEST_F(BugreportTest, BugreportzFailed) {
-    SetBugreportzVersion("1.1");
+    ExpectBugreportzVersion("1.1");
     EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _))
         .WillOnce(Return(666));
 
@@ -293,8 +394,7 @@ TEST_F(BugreportTest, BugreportzFailed) {
 
 // Tests 'adb bugreport file.zip' when the bugreport could not be pulled
 TEST_F(BugreportTest, PullFails) {
-    SetBugreportzVersion("1.1");
-    ExpectProgress(100, 100);
+    ExpectBugreportzVersion("1.1");
     EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _))
         .WillOnce(DoAll(WithArg<4>(WriteOnStdout("OK:/device/bugreport.zip")),
                         WithArg<4>(ReturnCallbackDone())));

--- a/adb/commandline.cpp
+++ b/adb/commandline.cpp
@@ -81,6 +81,7 @@ static std::string product_file(const char *extra) {
 
 static void help() {
     fprintf(stderr, "%s\n", adb_version().c_str());
+    // clang-format off
     fprintf(stderr,
         " -a                            - directs adb to listen on all interfaces for a connection\n"
         " -d                            - directs command to the only connected USB device\n"
@@ -173,9 +174,11 @@ static void help() {
         "                                 (-g: grant all runtime permissions)\n"
         "  adb uninstall [-k] <package> - remove this app package from the device\n"
         "                                 ('-k' means keep the data and cache directories)\n"
-        "  adb bugreport [<zip_file>]   - return all information from the device\n"
-        "                                 that should be included in a bug report.\n"
-        "\n"
+        "  adb bugreport [<path>]       - return all information from the device that should be included in a zipped bug report.\n"
+        "                                 If <path> is a file, the bug report will be saved as that file.\n"
+        "                                 If <path> is a directory, the bug report will be saved in that directory with the name provided by the device.\n"
+        "                                 If <path> is omitted, the bug report will be saved in the current directory with the name provided by the device.\n"
+        "                                 NOTE: if the device does not support zipped bug reports, the bug report will be output on stdout.\n"
         "  adb backup [-f <file>] [-apk|-noapk] [-obb|-noobb] [-shared|-noshared] [-all] [-system|-nosystem] [<packages...>]\n"
         "                               - write an archive of the device's data to <file>.\n"
         "                                 If no -f option is supplied then the data is written\n"
@@ -249,8 +252,8 @@ static void help() {
         "  ADB_TRACE                    - Print debug information. A comma separated list of the following values\n"
         "                                 1 or all, adb, sockets, packets, rwx, usb, sync, sysdeps, transport, jdwp\n"
         "  ANDROID_SERIAL               - The serial number to connect to. -s takes priority over this if given.\n"
-        "  ANDROID_LOG_TAGS             - When used with the logcat option, only these debug tags are printed.\n"
-        );
+        "  ANDROID_LOG_TAGS             - When used with the logcat option, only these debug tags are printed.\n");
+    // clang-format on
 }
 
 int usage() {
@@ -1327,7 +1330,7 @@ static std::string find_product_out_path(const std::string& hint) {
     if (hint.find_first_of(OS_PATH_SEPARATORS) != std::string::npos) {
         std::string cwd;
         if (!getcwd(&cwd)) {
-            fprintf(stderr, "adb: getcwd failed: %s\n", strerror(errno));
+            perror("adb: getcwd failed");
             return "";
         }
         return android::base::StringPrintf("%s%c%s", cwd.c_str(), OS_PATH_SEPARATOR, hint.c_str());