Refactored functions that copy sdout and stderr to strings to use a callback.
BUG: 28609499
Change-Id: I04aea346e18678ea00797f7f659480edba4436c2
diff --git a/adb/bugreport.cpp b/adb/bugreport.cpp
index 1af5843..e267f0f 100644
--- a/adb/bugreport.cpp
+++ b/adb/bugreport.cpp
@@ -19,12 +19,59 @@
#include <android-base/strings.h>
#include "bugreport.h"
-#include "commandline.h"
#include "file_sync_service.h"
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, Bugreport* br)
+ : br_(br), dest_file_(dest_file), stdout_str_() {
+ }
+
+ void OnStdout(const char* buffer, int length) {
+ std::string output;
+ OnStream(&output, stdout, buffer, length);
+ stdout_str_.append(output);
+ }
+
+ void OnStderr(const char* buffer, int length) {
+ OnStream(nullptr, stderr, buffer, length);
+ }
+
+ int Done(int unused_) {
+ int status = -1;
+ std::string output = android::base::Trim(stdout_str_);
+ if (android::base::StartsWith(output, BUGZ_OK_PREFIX)) {
+ const char* zip_file = &output[strlen(BUGZ_OK_PREFIX)];
+ std::vector<const char*> srcs{zip_file};
+ status = br_->DoSyncPull(srcs, dest_file_.c_str(), true, dest_file_.c_str()) ? 0 : 1;
+ if (status != 0) {
+ fprintf(stderr, "Could not copy file '%s' to '%s'\n", zip_file, dest_file_.c_str());
+ }
+ } else if (android::base::StartsWith(output, BUGZ_FAIL_PREFIX)) {
+ const char* error_message = &output[strlen(BUGZ_FAIL_PREFIX)];
+ fprintf(stderr, "Device failed to take a zipped bugreport: %s\n", error_message);
+ } else {
+ fprintf(stderr,
+ "Unexpected string (%s) returned by bugreportz, "
+ "device probably does not support it\n",
+ output.c_str());
+ }
+
+ return status;
+ }
+
+ private:
+ Bugreport* br_;
+ const std::string dest_file_;
+ std::string stdout_str_;
+
+ DISALLOW_COPY_AND_ASSIGN(BugreportStandardStreamsCallback);
+};
+
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();
@@ -37,41 +84,18 @@
// TODO: use a case-insensitive comparison (like EndsWithIgnoreCase
dest_file += ".zip";
}
- std::string output;
-
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");
- int status = SendShellCommand(transport_type, serial, "bugreportz", false, &output, nullptr);
- if (status != 0 || output.empty()) return status;
- output = android::base::Trim(output);
-
- if (android::base::StartsWith(output, BUGZ_OK_PREFIX)) {
- const char* zip_file = &output[strlen(BUGZ_OK_PREFIX)];
- std::vector<const char*> srcs{zip_file};
- status = DoSyncPull(srcs, dest_file.c_str(), true, dest_file.c_str()) ? 0 : 1;
- if (status != 0) {
- fprintf(stderr, "Could not copy file '%s' to '%s'\n", zip_file, dest_file.c_str());
- }
- return status;
- }
- if (android::base::StartsWith(output, BUGZ_FAIL_PREFIX)) {
- const char* error_message = &output[strlen(BUGZ_FAIL_PREFIX)];
- fprintf(stderr, "Device failed to take a zipped bugreport: %s\n", error_message);
- return -1;
- }
- fprintf(stderr,
- "Unexpected string (%s) returned by bugreportz, "
- "device probably does not support it\n",
- output.c_str());
- return -1;
+ BugreportStandardStreamsCallback bugz_callback(dest_file, this);
+ return SendShellCommand(transport_type, serial, "bugreportz", false, &bugz_callback);
}
int Bugreport::SendShellCommand(TransportType transport_type, const char* serial,
const std::string& command, bool disable_shell_protocol,
- std::string* output, std::string* err) {
- return send_shell_command(transport_type, serial, command, disable_shell_protocol, output, err);
+ StandardStreamsCallbackInterface* callback) {
+ return send_shell_command(transport_type, serial, command, disable_shell_protocol, callback);
}
bool Bugreport::DoSyncPull(const std::vector<const char*>& srcs, const char* dst, bool copy_attrs,
diff --git a/adb/bugreport.h b/adb/bugreport.h
index ff3a457..1c39a9f 100644
--- a/adb/bugreport.h
+++ b/adb/bugreport.h
@@ -20,8 +20,11 @@
#include <vector>
#include "adb.h"
+#include "commandline.h"
class Bugreport {
+ friend class BugreportStandardStreamsCallback;
+
public:
Bugreport() {
}
@@ -30,9 +33,10 @@
protected:
// Functions below are abstractions of external functions so they can be
// mocked on tests.
- virtual int SendShellCommand(TransportType transport_type, const char* serial,
- const std::string& command, bool disable_shell_protocol,
- std::string* output = nullptr, std::string* err = nullptr);
+ virtual int SendShellCommand(
+ TransportType transport_type, const char* serial, const std::string& command,
+ bool disable_shell_protocol,
+ StandardStreamsCallbackInterface* callback = &DEFAULT_STANDARD_STREAMS_CALLBACK);
virtual bool DoSyncPull(const std::vector<const char*>& srcs, const char* dst, bool copy_attrs,
const char* name);
diff --git a/adb/bugreport_test.cpp b/adb/bugreport_test.cpp
index dd2ff37..15c6f83 100644
--- a/adb/bugreport_test.cpp
+++ b/adb/bugreport_test.cpp
@@ -20,17 +20,19 @@
#include <gtest/gtest.h>
using ::testing::_;
+using ::testing::Action;
+using ::testing::ActionInterface;
using ::testing::DoAll;
using ::testing::ElementsAre;
using ::testing::HasSubstr;
+using ::testing::MakeAction;
using ::testing::Return;
-using ::testing::SetArgPointee;
using ::testing::StrEq;
+using ::testing::WithArg;
using ::testing::internal::CaptureStderr;
using ::testing::internal::GetCapturedStderr;
-// Empty function so tests don't need to be linked against
-// file_sync_service.cpp, which requires
+// Empty function so tests don't need to be linked against file_sync_service.cpp, which requires
// SELinux and its transitive dependencies...
bool do_sync_pull(const std::vector<const char*>& srcs, const char* dst, bool copy_attrs,
const char* name) {
@@ -38,23 +40,55 @@
return false;
}
-// Implemented in commandline.cpp
+// Empty functions so tests don't need to be linked against commandline.cpp
+DefaultStandardStreamsCallback DEFAULT_STANDARD_STREAMS_CALLBACK(nullptr, nullptr);
int usage() {
return -42;
}
-
-// Implemented in commandline.cpp
int send_shell_command(TransportType transport_type, const char* serial, const std::string& command,
- bool disable_shell_protocol, std::string* output, std::string* err) {
+ bool disable_shell_protocol, StandardStreamsCallbackInterface* callback) {
ADD_FAILURE() << "send_shell_command() should have been mocked";
return -42;
}
+// gmock black magic to provide a WithArg<4>(WriteOnStdout(output)) matcher
+typedef void OnStandardStreamsCallbackFunction(StandardStreamsCallbackInterface*);
+
+class OnStandardStreamsCallbackAction : public ActionInterface<OnStandardStreamsCallbackFunction> {
+ public:
+ explicit OnStandardStreamsCallbackAction(const std::string& output) : output_(output) {
+ }
+ virtual Result Perform(const ArgumentTuple& args) {
+ ::std::tr1::get<0>(args)->OnStdout(output_.c_str(), output_.size());
+ }
+
+ private:
+ std::string output_;
+};
+
+Action<OnStandardStreamsCallbackFunction> WriteOnStdout(const std::string& output) {
+ return MakeAction(new OnStandardStreamsCallbackAction(output));
+}
+
+typedef int CallbackDoneFunction(StandardStreamsCallbackInterface*);
+
+class CallbackDoneAction : public ActionInterface<CallbackDoneFunction> {
+ public:
+ virtual Result Perform(const ArgumentTuple& args) {
+ int status = ::std::tr1::get<0>(args)->Done(123); // Value passed does not matter
+ return status;
+ }
+};
+
+Action<CallbackDoneFunction> ReturnCallbackDone() {
+ return MakeAction(new CallbackDoneAction());
+}
+
class BugreportMock : public Bugreport {
public:
- MOCK_METHOD6(SendShellCommand,
+ MOCK_METHOD5(SendShellCommand,
int(TransportType transport_type, const char* serial, const std::string& command,
- bool disable_shell_protocol, std::string* output, std::string* err));
+ bool disable_shell_protocol, StandardStreamsCallbackInterface* callback));
MOCK_METHOD4(DoSyncPull, bool(const std::vector<const char*>& srcs, const char* dst,
bool copy_attrs, const char* name));
};
@@ -72,8 +106,7 @@
// Tests the legacy 'adb bugreport' option
TEST_F(BugreportTest, FlatFileFormat) {
- EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreport", false,
- nullptr, nullptr))
+ EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreport", false, _))
.WillOnce(Return(0));
const char* args[1024] = {"bugreport"};
@@ -82,9 +115,9 @@
// Tests 'adb bugreport file.zip' when it succeeds
TEST_F(BugreportTest, Ok) {
- EXPECT_CALL(
- br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz", false, _, nullptr))
- .WillOnce(DoAll(SetArgPointee<4>("OK:/device/bugreport.zip"), Return(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, StrEq("file.zip")))
.WillOnce(Return(true));
@@ -95,9 +128,9 @@
// Tests 'adb bugreport file' when it succeeds
TEST_F(BugreportTest, OkNoExtension) {
- EXPECT_CALL(
- br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz", false, _, nullptr))
- .WillOnce(DoAll(SetArgPointee<4>("OK:/device/bugreport.zip"), Return(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, StrEq("file.zip")))
.WillOnce(Return(true));
@@ -106,11 +139,39 @@
ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
}
+// Tests 'adb bugreport file.zip' when it succeeds but response was sent in
+// multiple buffer writers.
+TEST_F(BugreportTest, OkSplitBuffer) {
+ 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, StrEq("file.zip")))
+ .WillOnce(Return(true));
+
+ const char* args[1024] = {"bugreport", "file.zip"};
+ ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
+}
+
// Tests 'adb bugreport file.zip' when the bugreport itself failed
TEST_F(BugreportTest, BugreportzReturnedFail) {
- EXPECT_CALL(
- br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz", false, _, nullptr))
- .WillOnce(DoAll(SetArgPointee<4>("FAIL:D'OH!"), Return(0)));
+ EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz", false, _))
+ .WillOnce(DoAll(WithArg<4>(WriteOnStdout("FAIL:D'OH!")), WithArg<4>(ReturnCallbackDone())));
+
+ CaptureStderr();
+ const char* args[1024] = {"bugreport", "file.zip"};
+ ASSERT_EQ(-1, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
+ ASSERT_THAT(GetCapturedStderr(), HasSubstr("D'OH"));
+}
+
+// Tests 'adb bugreport file.zip' when the bugreport itself failed but response
+// was sent in
+// multiple buffer writes
+TEST_F(BugreportTest, BugreportzReturnedFailSplitBuffer) {
+ EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz", false, _))
+ .WillOnce(DoAll(WithArg<4>(WriteOnStdout("FAIL")), WithArg<4>(WriteOnStdout(":D'OH!")),
+ WithArg<4>(ReturnCallbackDone())));
CaptureStderr();
const char* args[1024] = {"bugreport", "file.zip"};
@@ -121,9 +182,9 @@
// Tests 'adb bugreport file.zip' when the bugreportz returned an unsupported
// response.
TEST_F(BugreportTest, BugreportzReturnedUnsupported) {
- EXPECT_CALL(
- br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz", false, _, nullptr))
- .WillOnce(DoAll(SetArgPointee<4>("bugreportz? What am I, a zombie?"), Return(0)));
+ EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz", false, _))
+ .WillOnce(DoAll(WithArg<4>(WriteOnStdout("bugreportz? What am I, a zombie?")),
+ WithArg<4>(ReturnCallbackDone())));
CaptureStderr();
const char* args[1024] = {"bugreport", "file.zip"};
@@ -133,8 +194,7 @@
// Tests 'adb bugreport file.zip' when the bugreportz command fails
TEST_F(BugreportTest, BugreportzFailed) {
- EXPECT_CALL(
- br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz", false, _, nullptr))
+ EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz", false, _))
.WillOnce(Return(666));
const char* args[1024] = {"bugreport", "file.zip"};
@@ -143,9 +203,9 @@
// Tests 'adb bugreport file.zip' when the bugreport could not be pulled
TEST_F(BugreportTest, PullFails) {
- EXPECT_CALL(
- br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz", false, _, nullptr))
- .WillOnce(DoAll(SetArgPointee<4>("OK:/device/bugreport.zip"), Return(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, StrEq("file.zip")))
.WillOnce(Return(false));
diff --git a/adb/commandline.cpp b/adb/commandline.cpp
index 6cb9ae2..0685b70 100644
--- a/adb/commandline.cpp
+++ b/adb/commandline.cpp
@@ -66,6 +66,8 @@
static auto& gProductOutPath = *new std::string();
extern int gListenAll;
+DefaultStandardStreamsCallback DEFAULT_STANDARD_STREAMS_CALLBACK(nullptr, nullptr);
+
static std::string product_file(const char *extra) {
if (gProductOutPath.empty()) {
fprintf(stderr, "adb: Product directory not specified; "
@@ -289,17 +291,14 @@
// this expects that incoming data will use the shell protocol, in which case
// stdout/stderr are routed independently and the remote exit code will be
// returned.
-// if |output| is non-null, stdout will be appended to it instead.
-// if |err| is non-null, stderr will be appended to it instead.
-static int read_and_dump(int fd, bool use_shell_protocol=false, std::string* output=nullptr,
- std::string* err=nullptr) {
+// if |callback| is non-null, stdout/stderr output will be handled by it.
+int read_and_dump(int fd, bool use_shell_protocol = false,
+ StandardStreamsCallbackInterface* callback = &DEFAULT_STANDARD_STREAMS_CALLBACK) {
int exit_code = 0;
if (fd < 0) return exit_code;
std::unique_ptr<ShellProtocol> protocol;
int length = 0;
- FILE* outfile = stdout;
- std::string* outstring = output;
char raw_buffer[BUFSIZ];
char* buffer_ptr = raw_buffer;
@@ -317,14 +316,13 @@
if (!protocol->Read()) {
break;
}
+ length = protocol->data_length();
switch (protocol->id()) {
case ShellProtocol::kIdStdout:
- outfile = stdout;
- outstring = output;
+ callback->OnStdout(buffer_ptr, length);
break;
case ShellProtocol::kIdStderr:
- outfile = stderr;
- outstring = err;
+ callback->OnStderr(buffer_ptr, length);
break;
case ShellProtocol::kIdExit:
exit_code = protocol->data()[0];
@@ -340,17 +338,11 @@
if (length <= 0) {
break;
}
- }
-
- if (outstring == nullptr) {
- fwrite(buffer_ptr, 1, length, outfile);
- fflush(outfile);
- } else {
- outstring->append(buffer_ptr, length);
+ callback->OnStdout(buffer_ptr, length);
}
}
- return exit_code;
+ return callback->Done(exit_code);
}
static void read_status_line(int fd, char* buf, size_t count)
@@ -1130,14 +1122,15 @@
}
int send_shell_command(TransportType transport_type, const char* serial, const std::string& command,
- bool disable_shell_protocol, std::string* output, std::string* err) {
+ bool disable_shell_protocol, StandardStreamsCallbackInterface* callback) {
int fd;
bool use_shell_protocol = false;
while (true) {
bool attempt_connection = true;
- // Use shell protocol if it's supported and the caller doesn't explicitly disable it.
+ // Use shell protocol if it's supported and the caller doesn't explicitly
+ // disable it.
if (!disable_shell_protocol) {
FeatureSet features;
std::string error;
@@ -1159,13 +1152,13 @@
}
}
- fprintf(stderr,"- waiting for device -\n");
+ fprintf(stderr, "- waiting for device -\n");
if (!wait_for_device("wait-for-device", transport_type, serial)) {
return 1;
}
}
- int exit_code = read_and_dump(fd, use_shell_protocol, output, err);
+ int exit_code = read_and_dump(fd, use_shell_protocol, callback);
if (adb_close(fd) < 0) {
PLOG(ERROR) << "failure closing FD " << fd;
diff --git a/adb/commandline.h b/adb/commandline.h
index 39764b4..0cf655c 100644
--- a/adb/commandline.h
+++ b/adb/commandline.h
@@ -19,13 +19,81 @@
#include "adb.h"
+// Callback used to handle the standard streams (stdout and stderr) sent by the
+// device's upon receiving a command.
+//
+class StandardStreamsCallbackInterface {
+ public:
+ StandardStreamsCallbackInterface() {
+ }
+ // Handles the stdout output from devices supporting the Shell protocol.
+ virtual void OnStdout(const char* buffer, int length) = 0;
+
+ // Handles the stderr output from devices supporting the Shell protocol.
+ virtual void OnStderr(const char* buffer, int length) = 0;
+
+ // Indicates the communication is finished and returns the appropriate error
+ // code.
+ //
+ // |status| has the status code returning by the underlying communication
+ // channels
+ virtual int Done(int status) = 0;
+
+ protected:
+ static void OnStream(std::string* string, FILE* stream, const char* buffer, int length) {
+ if (string != nullptr) {
+ string->append(buffer, length);
+ } else {
+ fwrite(buffer, 1, length, stream);
+ fflush(stream);
+ }
+ }
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(StandardStreamsCallbackInterface);
+};
+
+// Default implementation that redirects the streams to the equilavent host
+// stream or to a string
+// passed to the constructor.
+class DefaultStandardStreamsCallback : public StandardStreamsCallbackInterface {
+ public:
+ // If |stdout_str| is non-null, OnStdout will append to it.
+ // If |stderr_str| is non-null, OnStderr will append to it.
+ DefaultStandardStreamsCallback(std::string* stdout_str, std::string* stderr_str)
+ : stdout_str_(stdout_str), stderr_str_(stderr_str) {
+ }
+
+ void OnStdout(const char* buffer, int length) {
+ OnStream(stdout_str_, stdout, buffer, length);
+ }
+
+ void OnStderr(const char* buffer, int length) {
+ OnStream(stderr_str_, stderr, buffer, length);
+ }
+
+ int Done(int status) {
+ return status;
+ }
+
+ private:
+ std::string* stdout_str_;
+ std::string* stderr_str_;
+
+ DISALLOW_COPY_AND_ASSIGN(DefaultStandardStreamsCallback);
+};
+
+// Singleton.
+extern DefaultStandardStreamsCallback DEFAULT_STANDARD_STREAMS_CALLBACK;
+
int adb_commandline(int argc, const char** argv);
int usage();
// Connects to the device "shell" service with |command| and prints the
// resulting output.
+// if |callback| is non-null, stdout/stderr output will be handled by it.
int send_shell_command(TransportType transport_type, const char* serial, const std::string& command,
- bool disable_shell_protocol, std::string* output = nullptr,
- std::string* err = nullptr);
+ bool disable_shell_protocol, StandardStreamsCallbackInterface* callback =
+ &DEFAULT_STANDARD_STREAMS_CALLBACK);
#endif // COMMANDLINE_H
