From 3828ebcdd8e70d141ef5ead031e796cf33ea9a94 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Tue, 3 Dec 2019 16:05:54 -0800 Subject: [PATCH] adb: report error in copy_to_file. Previously, if we failed to read a file after successfully opening it, (e.g. because it's a directory), we would return prematurely without notifying our caller, which would then wait forever for a response that isn't coming. Switch all of the adb install callsites over to checking the result, but leave the other callsites ignoring the result, because they're either deprecated, or don't care. Bug: http://b/145621968 Test: mkdir foo.apk; adb install foo.apk Change-Id: Ia82f8280b144f7881e067a10cd17f9a89019cf3f --- adb/client/adb_install.cpp | 29 +++++++++++++++++++++-------- adb/client/commandline.cpp | 7 +++++-- adb/client/commandline.h | 2 +- 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/adb/client/adb_install.cpp b/adb/client/adb_install.cpp index 73dcde1fe..2bcd0a682 100644 --- a/adb/client/adb_install.cpp +++ b/adb/client/adb_install.cpp @@ -227,16 +227,20 @@ static int install_app_streamed(int argc, const char** argv, bool use_fastdeploy return 1; } - copy_to_file(local_fd.get(), remote_fd.get()); + if (!copy_to_file(local_fd.get(), remote_fd.get())) { + fprintf(stderr, "adb: failed to install: copy_to_file: %s: %s", file, strerror(errno)); + return 1; + } char buf[BUFSIZ]; read_status_line(remote_fd.get(), buf, sizeof(buf)); - if (!strncmp("Success", buf, 7)) { - fputs(buf, stdout); - return 0; + if (strncmp("Success", buf, 7) != 0) { + fprintf(stderr, "adb: failed to install %s: %s", file, buf); + return 1; } - fprintf(stderr, "adb: failed to install %s: %s", file, buf); - return 1; + + fputs(buf, stdout); + return 0; } static int install_app_legacy(int argc, const char** argv, bool use_fastdeploy) { @@ -455,7 +459,12 @@ int install_multiple_app(int argc, const char** argv) { goto finalize_session; } - copy_to_file(local_fd.get(), remote_fd.get()); + if (!copy_to_file(local_fd.get(), remote_fd.get())) { + fprintf(stderr, "adb: failed to write \"%s\": %s\n", file, strerror(errno)); + success = false; + goto finalize_session; + } + read_status_line(remote_fd.get(), buf, sizeof(buf)); if (strncmp("Success", buf, 7)) { @@ -634,7 +643,11 @@ int install_multi_package(int argc, const char** argv) { goto finalize_multi_package_session; } - copy_to_file(local_fd.get(), remote_fd.get()); + if (!copy_to_file(local_fd.get(), remote_fd.get())) { + fprintf(stderr, "adb: failed to write %s: %s\n", split.c_str(), strerror(errno)); + goto finalize_multi_package_session; + } + read_status_line(remote_fd.get(), buf, sizeof(buf)); if (strncmp("Success", buf, 7)) { diff --git a/adb/client/commandline.cpp b/adb/client/commandline.cpp index a6d7e3193..c3029652c 100644 --- a/adb/client/commandline.cpp +++ b/adb/client/commandline.cpp @@ -352,7 +352,8 @@ static void stdinout_raw_epilogue(int inFd, int outFd, int old_stdin_mode, int o #endif } -void copy_to_file(int inFd, int outFd) { +bool copy_to_file(int inFd, int outFd) { + bool result = true; std::vector buf(64 * 1024); int len; long total = 0; @@ -375,6 +376,7 @@ void copy_to_file(int inFd, int outFd) { } if (len < 0) { D("copy_to_file(): read failed: %s", strerror(errno)); + result = false; break; } if (outFd == STDOUT_FILENO) { @@ -388,7 +390,8 @@ void copy_to_file(int inFd, int outFd) { stdinout_raw_epilogue(inFd, outFd, old_stdin_mode, old_stdout_mode); - D("copy_to_file() finished after %lu bytes", total); + D("copy_to_file() finished with %s after %lu bytes", result ? "success" : "failure", total); + return result; } static void send_window_size_change(int fd, std::unique_ptr& shell) { diff --git a/adb/client/commandline.h b/adb/client/commandline.h index ab77b29b3..b9dee5625 100644 --- a/adb/client/commandline.h +++ b/adb/client/commandline.h @@ -100,7 +100,7 @@ extern DefaultStandardStreamsCallback DEFAULT_STANDARD_STREAMS_CALLBACK; int adb_commandline(int argc, const char** argv); -void copy_to_file(int inFd, int outFd); +bool copy_to_file(int inFd, int outFd); // Connects to the device "shell" service with |command| and prints the // resulting output.