Refactored functions that copy sdout and stderr to strings to use a callback. (d1885421) · Commits · e / os / android_system_core

adb/bugreport.cpp

+52 −28

Original line number	Diff line number	Diff line
		@@ -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 @@ int Bugreport::DoIt(TransportType transport_type, const char* serial, int argc,
		// 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,

adb/bugreport.h

+7 −3

Original line number	Diff line number	Diff line
		@@ -20,8 +20,11 @@
		#include <vector>

		#include "adb.h"
		#include "commandline.h"

		class Bugreport {
		friend class BugreportStandardStreamsCallback;

		public:
		Bugreport() {
		}
		@@ -30,9 +33,10 @@ class Bugreport {
		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);

adb/bugreport_test.cpp

+88 −28

Original line number	Diff line number	Diff line
		@@ -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 @@ bool do_sync_pull(const std::vector<const char>& srcs, const char dst, bool co
		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 @@ TEST_F(BugreportTest, InvalidNumberArgs) {

		// 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 @@ TEST_F(BugreportTest, FlatFileFormat) {

		// 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 @@ TEST_F(BugreportTest, Ok) {

		// 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 @@ TEST_F(BugreportTest, OkNoExtension) {
		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 @@ TEST_F(BugreportTest, BugreportzReturnedFail) {
		// 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 @@ TEST_F(BugreportTest, BugreportzReturnedUnsupported) {

		// 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 @@ TEST_F(BugreportTest, BugreportzFailed) {

		// 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));

adb/commandline.cpp

+15 −22

Original line number	Diff line number	Diff line
		@@ -67,6 +67,8 @@ static int uninstall_app_legacy(TransportType t, const char* serial, int argc, c
		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; "
		@@ -290,17 +292,14 @@ static void stdin_raw_restore() {
		// 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;
		@@ -318,14 +317,13 @@ static int read_and_dump(int fd, bool use_shell_protocol=false, std::string* out
		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];
		@@ -341,17 +339,11 @@ static int read_and_dump(int fd, bool use_shell_protocol=false, std::string* out
		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)
		@@ -1112,14 +1104,15 @@ static bool adb_root(const char* command) {
		}

		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;
		@@ -1147,7 +1140,7 @@ int send_shell_command(TransportType transport_type, const char* serial, const s
		}
		}

		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;

adb/commandline.h

+70 −2

Original line number	Diff line number	Diff line
		@@ -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