From 1fc8f6e0cfdfd5c9dbbd82116fba2ebec82b4659 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Tue, 18 Apr 2017 14:34:16 -0700 Subject: [PATCH] Fix various adb error reporting bugs. `adb install` was writing success to stderr rather than stdout. Server mismatch messages were going to stdout rather than stderr. Error messages should consistently start with a lower case letter. Also improve consistency of syntax error reporting. Bug: https://issuetracker.google.com/37128706 (adb install success to stdout) Bug: https://issuetracker.google.com/37140458 (server mismatch on stderr) Bug: http://b/32413861 (consistency) Test: ran tests Change-Id: I0d6cb0c7482bec03483dacefd996644b7a28d273 --- adb/adb.cpp | 48 +++++++------ adb/adb_client.cpp | 14 ++-- adb/adb_utils.cpp | 4 +- adb/adb_utils.h | 2 +- adb/bugreport.cpp | 4 +- adb/commandline.cpp | 162 +++++++++++++++++++------------------------- adb/commandline.h | 1 - 7 files changed, 103 insertions(+), 132 deletions(-) diff --git a/adb/adb.cpp b/adb/adb.cpp index cf6b3593d..51d5876f5 100644 --- a/adb/adb.cpp +++ b/adb/adb.cpp @@ -507,8 +507,8 @@ static bool _make_handle_noninheritable(HANDLE h) { if (!_try_make_handle_noninheritable(h)) { // Show the handle value to give us a clue in case we have problems // with pseudo-handle values. - fprintf(stderr, "Cannot make handle 0x%p non-inheritable: %s\n", - h, android::base::SystemErrorCodeToString(GetLastError()).c_str()); + fprintf(stderr, "adb: cannot make handle 0x%p non-inheritable: %s\n", h, + android::base::SystemErrorCodeToString(GetLastError()).c_str()); return false; } @@ -523,7 +523,7 @@ static bool _create_anonymous_pipe(unique_handle* pipe_read_out, HANDLE pipe_read_raw = NULL; HANDLE pipe_write_raw = NULL; if (!CreatePipe(&pipe_read_raw, &pipe_write_raw, sa, 0)) { - fprintf(stderr, "Cannot create pipe: %s\n", + fprintf(stderr, "adb: CreatePipe failed: %s\n", android::base::SystemErrorCodeToString(GetLastError()).c_str()); return false; } @@ -554,7 +554,8 @@ static unsigned _redirect_pipe_thread(HANDLE h, DWORD nStdHandle) { std::unique_ptr stream(nullptr, fclose); if (original_fd == -1) { - fprintf(stderr, "Failed to get file descriptor for %s: %s\n", output_name, strerror(errno)); + fprintf(stderr, "adb: failed to get file descriptor for %s: %s\n", output_name, + strerror(errno)); return EXIT_FAILURE; } @@ -566,7 +567,7 @@ static unsigned _redirect_pipe_thread(HANDLE h, DWORD nStdHandle) { // call this function if subprocesses may be started concurrently. const int fd = dup(original_fd); if (fd == -1) { - fprintf(stderr, "Failed to duplicate file descriptor for %s: %s\n", output_name, + fprintf(stderr, "adb: failed to duplicate file descriptor for %s: %s\n", output_name, strerror(errno)); return EXIT_FAILURE; } @@ -574,7 +575,7 @@ static unsigned _redirect_pipe_thread(HANDLE h, DWORD nStdHandle) { // Note that although we call fdopen() below with a binary flag, it may not adhere to that // flag, so we have to set the mode manually. if (_setmode(fd, _O_BINARY) == -1) { - fprintf(stderr, "Failed to set binary mode for duplicate of %s: %s\n", output_name, + fprintf(stderr, "adb: failed to set binary mode for duplicate of %s: %s\n", output_name, strerror(errno)); unix_close(fd); return EXIT_FAILURE; @@ -582,7 +583,7 @@ static unsigned _redirect_pipe_thread(HANDLE h, DWORD nStdHandle) { stream.reset(fdopen(fd, "wb")); if (stream.get() == nullptr) { - fprintf(stderr, "Failed to open duplicate stream for %s: %s\n", output_name, + fprintf(stderr, "adb: failed to open duplicate stream for %s: %s\n", output_name, strerror(errno)); unix_close(fd); return EXIT_FAILURE; @@ -591,7 +592,7 @@ static unsigned _redirect_pipe_thread(HANDLE h, DWORD nStdHandle) { // Unbuffer the stream because it will be buffered by default and we want subprocess output // to be shown immediately. if (setvbuf(stream.get(), NULL, _IONBF, 0) == -1) { - fprintf(stderr, "Failed to unbuffer %s: %s\n", output_name, strerror(errno)); + fprintf(stderr, "adb: failed to unbuffer %s: %s\n", output_name, strerror(errno)); return EXIT_FAILURE; } @@ -608,7 +609,7 @@ static unsigned _redirect_pipe_thread(HANDLE h, DWORD nStdHandle) { if (err == ERROR_BROKEN_PIPE) { return EXIT_SUCCESS; } else { - fprintf(stderr, "Failed to read from %s: %s\n", output_name, + fprintf(stderr, "adb: failed to read from %s: %s\n", output_name, android::base::SystemErrorCodeToString(err).c_str()); return EXIT_FAILURE; } @@ -619,8 +620,8 @@ static unsigned _redirect_pipe_thread(HANDLE h, DWORD nStdHandle) { // fwrite() actually calls adb_fwrite() which can write UTF-8 to the console. const size_t bytes_written = fwrite(buf, 1, bytes_read, stream.get()); if (bytes_written != bytes_read) { - fprintf(stderr, "Only wrote %zu of %lu bytes to %s\n", bytes_written, bytes_read, - output_name); + fprintf(stderr, "adb: error: only wrote %zu of %lu bytes to %s\n", bytes_written, + bytes_read, output_name); return EXIT_FAILURE; } } @@ -663,7 +664,7 @@ int launch_server(const std::string& socket_spec) { FILE_SHARE_READ | FILE_SHARE_WRITE, &sa, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL)); if (nul_read.get() == INVALID_HANDLE_VALUE) { - fprintf(stderr, "Cannot open 'nul': %s\n", + fprintf(stderr, "adb: CreateFileW 'nul' failed: %s\n", android::base::SystemErrorCodeToString(GetLastError()).c_str()); return -1; } @@ -725,8 +726,7 @@ int launch_server(const std::string& socket_spec) { // If this fires, either handle values are larger than 32-bits or else // there is a bug in our casting. // https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203%28v=vs.85%29.aspx - fprintf(stderr, "Cannot fit pipe handle value into 32-bits: 0x%p\n", - ack_write.get()); + fprintf(stderr, "adb: cannot fit pipe handle value into 32-bits: 0x%p\n", ack_write.get()); return -1; } @@ -736,7 +736,7 @@ int launch_server(const std::string& socket_spec) { arraysize(program_path)); if ((module_result >= arraysize(program_path)) || (module_result == 0)) { // String truncation or some other error. - fprintf(stderr, "Cannot get executable path: %s\n", + fprintf(stderr, "adb: cannot get executable path: %s\n", android::base::SystemErrorCodeToString(GetLastError()).c_str()); return -1; } @@ -761,7 +761,7 @@ int launch_server(const std::string& socket_spec) { NULL, /* use parent's starting directory */ &startup, /* startup info, i.e. std handles */ &pinfo )) { - fprintf(stderr, "Cannot create process: %s\n", + fprintf(stderr, "adb: CreateProcessW failed: %s\n", android::base::SystemErrorCodeToString(GetLastError()).c_str()); return -1; } @@ -791,7 +791,7 @@ int launch_server(const std::string& socket_spec) { _beginthreadex(NULL, 0, _redirect_stdout_thread, stdout_read.get(), 0, NULL))); if (stdout_thread.get() == nullptr) { - fprintf(stderr, "Cannot create thread: %s\n", strerror(errno)); + fprintf(stderr, "adb: cannot create thread: %s\n", strerror(errno)); return -1; } stdout_read.release(); // Transfer ownership to new thread @@ -800,7 +800,7 @@ int launch_server(const std::string& socket_spec) { _beginthreadex(NULL, 0, _redirect_stderr_thread, stderr_read.get(), 0, NULL))); if (stderr_thread.get() == nullptr) { - fprintf(stderr, "Cannot create thread: %s\n", strerror(errno)); + fprintf(stderr, "adb: cannot create thread: %s\n", strerror(errno)); return -1; } stderr_read.release(); // Transfer ownership to new thread @@ -845,22 +845,20 @@ int launch_server(const std::string& socket_spec) { if (wait_result == WAIT_TIMEOUT) { // Threads did not finish after waiting a little while. Perhaps the // server didn't close pipes, or it is hung. - fprintf(stderr, "Timed-out waiting for threads to finish reading from " - "ADB Server\n"); + fprintf(stderr, "adb: timed out waiting for threads to finish reading from ADB server\n"); // Process handles are signaled when the process exits, so if we wait // on the handle for 0 seconds and it returns 'timeout', that means that // the process is still running. if (WaitForSingleObject(process_handle.get(), 0) == WAIT_TIMEOUT) { // We could TerminateProcess(), but that seems somewhat presumptive. - fprintf(stderr, "ADB Server is running: process id %lu\n", - pinfo.dwProcessId); + fprintf(stderr, "adb: server is running with process id %lu\n", pinfo.dwProcessId); } return -1; } if (wait_result != WAIT_OBJECT_0) { - fprintf(stderr, "Unexpected result waiting for threads: %lu: %s\n", - wait_result, android::base::SystemErrorCodeToString(GetLastError()).c_str()); + fprintf(stderr, "adb: unexpected result waiting for threads: %lu: %s\n", wait_result, + android::base::SystemErrorCodeToString(GetLastError()).c_str()); return -1; } @@ -894,7 +892,7 @@ int launch_server(const std::string& socket_spec) { int result = execl(path.c_str(), "adb", "-L", socket_spec.c_str(), "fork-server", "server", "--reply-fd", reply_fd, NULL); // this should not return - fprintf(stderr, "OOPS! execl returned %d, errno: %d\n", result, errno); + fprintf(stderr, "adb: execl returned %d: %s\n", result, strerror(errno)); } else { // parent side of the fork diff --git a/adb/adb_client.cpp b/adb/adb_client.cpp index ef52189f2..b2b5c0ee0 100644 --- a/adb/adb_client.cpp +++ b/adb/adb_client.cpp @@ -164,21 +164,21 @@ int adb_connect(const std::string& service, std::string* error) { D("adb_connect: service %s", service.c_str()); if (fd == -2 && !is_local_socket_spec(__adb_server_socket_spec)) { - fprintf(stderr,"** Cannot start server on remote host\n"); + fprintf(stderr, "* cannot start server on remote host\n"); // error is the original network connection error return fd; } else if (fd == -2) { - fprintf(stdout, "* daemon not running. starting it now at %s *\n", __adb_server_socket_spec); + fprintf(stderr, "* daemon not running; starting now at %s\n", __adb_server_socket_spec); start_server: if (launch_server(__adb_server_socket_spec)) { - fprintf(stderr,"* failed to start daemon *\n"); + fprintf(stderr, "* failed to start daemon\n"); // launch_server() has already printed detailed error info, so just // return a generic error string about the overall adb_connect() // that the caller requested. *error = "cannot connect to daemon"; return -1; } else { - fprintf(stdout,"* daemon started successfully *\n"); + fprintf(stderr, "* daemon started successfully\n"); } // Give the server some time to start properly and detect devices. std::this_thread::sleep_for(3s); @@ -213,8 +213,8 @@ int adb_connect(const std::string& service, std::string* error) { } if (version != ADB_SERVER_VERSION) { - printf("adb server version (%d) doesn't match this client (%d); killing...\n", - version, ADB_SERVER_VERSION); + fprintf(stderr, "adb server version (%d) doesn't match this client (%d); killing...\n", + version, ADB_SERVER_VERSION); fd = _adb_connect("host:kill", error); if (fd >= 0) { ReadOrderlyShutdown(fd); @@ -240,7 +240,7 @@ int adb_connect(const std::string& service, std::string* error) { if (fd == -1) { D("_adb_connect error: %s", error->c_str()); } else if(fd == -2) { - fprintf(stderr,"** daemon still not running\n"); + fprintf(stderr, "* daemon still not running\n"); } D("adb_connect: return fd %d", fd); diff --git a/adb/adb_utils.cpp b/adb/adb_utils.cpp index 31d3dc63a..6f2403d03 100644 --- a/adb/adb_utils.cpp +++ b/adb/adb_utils.cpp @@ -267,8 +267,8 @@ void AdbCloser::Close(int fd) { adb_close(fd); } -int usage(const char* fmt, ...) { - fprintf(stderr, "adb: "); +int syntax_error(const char* fmt, ...) { + fprintf(stderr, "adb: usage: "); va_list ap; va_start(ap, fmt); diff --git a/adb/adb_utils.h b/adb/adb_utils.h index e0ad10380..c1d5549b0 100644 --- a/adb/adb_utils.h +++ b/adb/adb_utils.h @@ -21,7 +21,7 @@ #include -int usage(const char*, ...); +int syntax_error(const char*, ...); void close_stdin(); diff --git a/adb/bugreport.cpp b/adb/bugreport.cpp index 9dc981102..da2cfa208 100644 --- a/adb/bugreport.cpp +++ b/adb/bugreport.cpp @@ -137,7 +137,7 @@ class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface 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); + fprintf(stderr, "adb: device failed to take a zipped bugreport: %s\n", error_message); status_ = -1; } else if (show_progress_ && android::base::StartsWith(line, BUGZ_PROGRESS_PREFIX)) { // progress_line should have the following format: @@ -195,7 +195,7 @@ class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface }; int Bugreport::DoIt(TransportType transport_type, const char* serial, int argc, const char** argv) { - if (argc > 2) return usage("usage: adb bugreport [PATH]"); + if (argc > 2) return syntax_error("adb bugreport [PATH]"); // Gets bugreportz version. std::string bugz_stdout, bugz_stderr; diff --git a/adb/commandline.cpp b/adb/commandline.cpp index 7702b0edf..d626259f2 100644 --- a/adb/commandline.cpp +++ b/adb/commandline.cpp @@ -690,8 +690,7 @@ static int adb_shell(int argc, const char** argv) { switch (opt) { case 'e': if (!(strlen(optarg) == 1 || strcmp(optarg, "none") == 0)) { - fprintf(stderr, "error: -e requires a single-character argument or 'none'\n"); - return 1; + return syntax_error("-e requires a single-character argument or 'none'"); } escape_char = (strcmp(optarg, "none") == 0) ? 0 : optarg[0]; break; @@ -843,7 +842,10 @@ static int adb_download_buffer(const char* service, const char* filename) { * we hang up. */ static int adb_sideload_host(const char* filename) { - fprintf(stderr, "opening '%s'...\n", filename); + // TODO: use a LinePrinter instead... + fprintf(stdout, "opening '%s'...\n", filename); + fflush(stdout); + struct stat sb; if (stat(filename, &sb) == -1) { fprintf(stderr, "failed to stat file %s: %s\n", filename, strerror(errno)); @@ -855,7 +857,8 @@ static int adb_sideload_host(const char* filename) { return -1; } - fprintf(stderr, "connecting...\n"); + fprintf(stdout, "connecting...\n"); + fflush(stdout); std::string service = android::base::StringPrintf( "sideload-host:%d:%d", static_cast(sb.st_size), SIDELOAD_HOST_BLOCK_SIZE); std::string error; @@ -944,19 +947,13 @@ static int ppp(int argc, const char** argv) { fprintf(stderr, "error: adb %s not implemented on Win32\n", argv[0]); return -1; #else - if (argc < 2) { - fprintf(stderr, "usage: adb %s [ppp opts]\n", - argv[0]); - - return 1; - } + if (argc < 2) return syntax_error("adb %s [ppp opts]", argv[0]); const char* adb_service_name = argv[1]; std::string error; int fd = adb_connect(adb_service_name, &error); if (fd < 0) { - fprintf(stderr,"Error: Could not open adb service: %s. Error: %s\n", - adb_service_name, error.c_str()); + fprintf(stderr, "adb: could not open adb service %s: %s\n", adb_service_name, error.c_str()); return 1; } @@ -1175,10 +1172,7 @@ static int backup(int argc, const char** argv) { /* find, extract, and use any -f argument */ for (int i = 1; i < argc; i++) { if (!strcmp("-f", argv[i])) { - if (i == argc-1) { - fprintf(stderr, "adb: backup -f passed with no filename.\n"); - return EXIT_FAILURE; - } + if (i == argc - 1) return syntax_error("backup -f passed with no filename"); filename = argv[i+1]; for (int j = i+2; j <= argc; ) { argv[i++] = argv[j++]; @@ -1190,10 +1184,7 @@ static int backup(int argc, const char** argv) { // Bare "adb backup" or "adb backup -f filename" are not valid invocations --- // a list of packages is required. - if (argc < 2) { - fprintf(stderr, "adb: backup either needs a list of packages or -all/-shared.\n"); - return EXIT_FAILURE; - } + if (argc < 2) return syntax_error("backup either needs a list of packages or -all/-shared"); adb_unlink(filename); int outFd = adb_creat(filename, 0640); @@ -1218,7 +1209,7 @@ static int backup(int argc, const char** argv) { return EXIT_FAILURE; } - printf("Now unlock your device and confirm the backup operation...\n"); + fprintf(stdout, "Now unlock your device and confirm the backup operation...\n"); fflush(stdout); copy_to_file(fd, outFd); @@ -1229,7 +1220,7 @@ static int backup(int argc, const char** argv) { } static int restore(int argc, const char** argv) { - if (argc != 2) return usage("restore requires an argument"); + if (argc != 2) return syntax_error("restore requires an argument"); const char* filename = argv[1]; int tarFd = adb_open(filename, O_RDONLY); @@ -1246,7 +1237,9 @@ static int restore(int argc, const char** argv) { return -1; } - printf("Now unlock your device and confirm the restore operation.\n"); + fprintf(stdout, "Now unlock your device and confirm the restore operation.\n"); + fflush(stdout); + copy_to_file(tarFd, fd); // Provide an in-band EOD marker in case the archive file is malformed @@ -1338,7 +1331,7 @@ static void parse_push_pull_args(const char** arg, int narg, } else if (!strcmp(*arg, "--")) { ignore_flags = true; } else { - fprintf(stderr, "adb: unrecognized option '%s'\n", *arg); + syntax_error("unrecognized option '%s'", *arg); exit(1); } } @@ -1437,7 +1430,7 @@ int adb_commandline(int argc, const char** argv) { /* this is a special flag used only when the ADB client launches the ADB Server */ is_daemon = 1; } else if (!strcmp(argv[0], "--reply-fd")) { - if (argc < 2) return usage("--reply-fd requires an argument"); + if (argc < 2) return syntax_error("--reply-fd requires an argument"); const char* reply_fd_str = argv[1]; argc--; argv++; @@ -1449,7 +1442,7 @@ int adb_commandline(int argc, const char** argv) { } else if (!strncmp(argv[0], "-p", 2)) { const char* product = nullptr; if (argv[0][2] == '\0') { - if (argc < 2) return usage("-p requires an argument"); + if (argc < 2) return syntax_error("-p requires an argument"); product = argv[1]; argc--; argv++; @@ -1465,7 +1458,7 @@ int adb_commandline(int argc, const char** argv) { if (isdigit(argv[0][2])) { serial = argv[0] + 2; } else { - if (argc < 2 || argv[0][2] != '\0') return usage("-s requires an argument"); + if (argc < 2 || argv[0][2] != '\0') return syntax_error("-s requires an argument"); serial = argv[1]; argc--; argv++; @@ -1478,7 +1471,7 @@ int adb_commandline(int argc, const char** argv) { gListenAll = 1; } else if (!strncmp(argv[0], "-H", 2)) { if (argv[0][2] == '\0') { - if (argc < 2) return usage("-H requires an argument"); + if (argc < 2) return syntax_error("-H requires an argument"); server_host_str = argv[1]; argc--; argv++; @@ -1487,7 +1480,7 @@ int adb_commandline(int argc, const char** argv) { } } else if (!strncmp(argv[0], "-P", 2)) { if (argv[0][2] == '\0') { - if (argc < 2) return usage("-P requires an argument"); + if (argc < 2) return syntax_error("-P requires an argument"); server_port_str = argv[1]; argc--; argv++; @@ -1495,7 +1488,7 @@ int adb_commandline(int argc, const char** argv) { server_port_str = argv[0] + 2; } } else if (!strcmp(argv[0], "-L")) { - if (argc < 2) return usage("-L requires an argument"); + if (argc < 2) return syntax_error("-L requires an argument"); server_socket_str = argv[1]; argc--; argv++; @@ -1508,8 +1501,7 @@ int adb_commandline(int argc, const char** argv) { } if ((server_host_str || server_port_str) && server_socket_str) { - fprintf(stderr, "adb: -L is incompatible with -H or -P\n"); - exit(1); + return syntax_error("-L is incompatible with -H or -P"); } // If -L, -H, or -P are specified, ignore environment variables. @@ -1604,8 +1596,7 @@ int adb_commandline(int argc, const char** argv) { } else if (argc == 2 && !strcmp(argv[1], "-l")) { listopt = argv[1]; } else { - fprintf(stderr, "Usage: adb devices [-l]\n"); - return 1; + return syntax_error("adb devices [-l]"); } std::string query = android::base::StringPrintf("host:%s%s", argv[0], listopt); @@ -1613,19 +1604,13 @@ int adb_commandline(int argc, const char** argv) { return adb_query_command(query); } else if (!strcmp(argv[0], "connect")) { - if (argc != 2) { - fprintf(stderr, "Usage: adb connect [:]\n"); - return 1; - } + if (argc != 2) return syntax_error("adb connect [:]"); std::string query = android::base::StringPrintf("host:connect:%s", argv[1]); return adb_query_command(query); } else if (!strcmp(argv[0], "disconnect")) { - if (argc > 2) { - fprintf(stderr, "Usage: adb disconnect [[:]]\n"); - return 1; - } + if (argc > 2) return syntax_error("adb disconnect [[:]]"); std::string query = android::base::StringPrintf("host:disconnect:%s", (argc == 2) ? argv[1] : ""); @@ -1640,10 +1625,7 @@ int adb_commandline(int argc, const char** argv) { else if (!strcmp(argv[0], "exec-in") || !strcmp(argv[0], "exec-out")) { int exec_in = !strcmp(argv[0], "exec-in"); - if (argc < 2) { - fprintf(stderr, "Usage: adb %s command\n", argv[0]); - return 1; - } + if (argc < 2) return syntax_error("adb %s command", argv[0]); std::string cmd = "exec:"; cmd += argv[1]; @@ -1691,7 +1673,7 @@ int adb_commandline(int argc, const char** argv) { } } else if (!strcmp(argv[0], "sideload")) { - if (argc != 2) return usage("sideload requires an argument"); + if (argc != 2) return syntax_error("sideload requires an argument"); if (adb_sideload_host(argv[1])) { return 1; } else { @@ -1725,7 +1707,7 @@ int adb_commandline(int argc, const char** argv) { bool reverse = !strcmp(argv[0], "reverse"); ++argv; --argc; - if (argc < 1) return usage("%s requires an argument", argv[0]); + if (argc < 1) return syntax_error("%s requires an argument", argv[0]); // Determine the for this command. std::string host_prefix; @@ -1745,24 +1727,24 @@ int adb_commandline(int argc, const char** argv) { std::string cmd, error; if (strcmp(argv[0], "--list") == 0) { - if (argc != 1) return usage("--list doesn't take any arguments"); + if (argc != 1) return syntax_error("--list doesn't take any arguments"); return adb_query_command(host_prefix + ":list-forward"); } else if (strcmp(argv[0], "--remove-all") == 0) { - if (argc != 1) return usage("--remove-all doesn't take any arguments"); + if (argc != 1) return syntax_error("--remove-all doesn't take any arguments"); cmd = host_prefix + ":killforward-all"; } else if (strcmp(argv[0], "--remove") == 0) { // forward --remove - if (argc != 2) return usage("--remove requires an argument"); + if (argc != 2) return syntax_error("--remove requires an argument"); cmd = host_prefix + ":killforward:" + argv[1]; } else if (strcmp(argv[0], "--no-rebind") == 0) { // forward --no-rebind - if (argc != 3) return usage("--no-rebind takes two arguments"); + if (argc != 3) return syntax_error("--no-rebind takes two arguments"); if (forward_targets_are_valid(argv[1], argv[2], &error)) { cmd = host_prefix + ":forward:norebind:" + argv[1] + ";" + argv[2]; } } else { // forward - if (argc != 2) return usage("forward takes two arguments"); + if (argc != 2) return syntax_error("forward takes two arguments"); if (forward_targets_are_valid(argv[0], argv[1], &error)) { cmd = host_prefix + ":forward:" + argv[0] + ";" + argv[1]; } @@ -1791,7 +1773,7 @@ int adb_commandline(int argc, const char** argv) { } /* do_sync_*() commands */ else if (!strcmp(argv[0], "ls")) { - if (argc != 2) return usage("ls requires an argument"); + if (argc != 2) return syntax_error("ls requires an argument"); return do_sync_ls(argv[1]) ? 0 : 1; } else if (!strcmp(argv[0], "push")) { @@ -1800,7 +1782,7 @@ int adb_commandline(int argc, const char** argv) { const char* dst = nullptr; parse_push_pull_args(&argv[1], argc - 1, &srcs, &dst, ©_attrs); - if (srcs.empty() || !dst) return usage("push requires an argument"); + if (srcs.empty() || !dst) return syntax_error("push requires an argument"); return do_sync_push(srcs, dst) ? 0 : 1; } else if (!strcmp(argv[0], "pull")) { @@ -1809,22 +1791,22 @@ int adb_commandline(int argc, const char** argv) { const char* dst = "."; parse_push_pull_args(&argv[1], argc - 1, &srcs, &dst, ©_attrs); - if (srcs.empty()) return usage("pull requires an argument"); + if (srcs.empty()) return syntax_error("pull requires an argument"); return do_sync_pull(srcs, dst, copy_attrs) ? 0 : 1; } else if (!strcmp(argv[0], "install")) { - if (argc < 2) return usage("install requires an argument"); + if (argc < 2) return syntax_error("install requires an argument"); if (_use_legacy_install()) { return install_app_legacy(transport_type, serial, argc, argv); } return install_app(transport_type, serial, argc, argv); } else if (!strcmp(argv[0], "install-multiple")) { - if (argc < 2) return usage("install-multiple requires an argument"); + if (argc < 2) return syntax_error("install-multiple requires an argument"); return install_multiple_app(transport_type, serial, argc, argv); } else if (!strcmp(argv[0], "uninstall")) { - if (argc < 2) return usage("uninstall requires an argument"); + if (argc < 2) return syntax_error("uninstall requires an argument"); if (_use_legacy_install()) { return uninstall_app_legacy(transport_type, serial, argc, argv); } @@ -1847,12 +1829,12 @@ int adb_commandline(int argc, const char** argv) { // A local path or "android"/"data" arg was specified. src = argv[1]; } else { - return usage("usage: adb sync [-l] [PARTITION]"); + return syntax_error("adb sync [-l] [PARTITION]"); } if (src != "" && src != "system" && src != "data" && src != "vendor" && src != "oem") { - return usage("don't know how to sync %s partition", src.c_str()); + return syntax_error("don't know how to sync %s partition", src.c_str()); } std::string system_src_path = product_file("system"); @@ -1904,7 +1886,7 @@ int adb_commandline(int argc, const char** argv) { return restore(argc, argv); } else if (!strcmp(argv[0], "keygen")) { - if (argc != 2) return usage("keygen requires an argument"); + if (argc != 2) return syntax_error("keygen requires an argument"); // Always print key generation information for keygen command. adb_trace_enable(AUTH); return adb_auth_keygen(argv[1]); @@ -1957,12 +1939,12 @@ int adb_commandline(int argc, const char** argv) { std::string err; return adb_query_command("host:reconnect-offline"); } else { - return usage("usage: adb reconnect [device|offline]"); + return syntax_error("adb reconnect [device|offline]"); } } } - usage("unknown command %s", argv[0]); + syntax_error("unknown command %s", argv[0]); return 1; } @@ -1989,19 +1971,18 @@ static int install_app(TransportType transport, const char* serial, int argc, co // The last argument must be the APK file const char* file = argv[argc - 1]; if (!android::base::EndsWithIgnoreCase(file, ".apk")) { - fprintf(stderr, "Filename doesn't end .apk: %s\n", file); - return EXIT_FAILURE; + return syntax_error("filename doesn't end .apk: %s", file); } struct stat sb; if (stat(file, &sb) == -1) { - fprintf(stderr, "Failed to stat %s: %s\n", file, strerror(errno)); + fprintf(stderr, "adb: failed to stat %s: %s\n", file, strerror(errno)); return 1; } int localFd = adb_open(file, O_RDONLY); if (localFd < 0) { - fprintf(stderr, "Failed to open %s: %s\n", file, strerror(errno)); + fprintf(stderr, "adb: failed to open %s: %s\n", file, strerror(errno)); return 1; } @@ -2019,7 +2000,7 @@ static int install_app(TransportType transport, const char* serial, int argc, co int remoteFd = adb_connect(cmd, &error); if (remoteFd < 0) { - fprintf(stderr, "Connect error for write: %s\n", error.c_str()); + fprintf(stderr, "adb: connect error for write: %s\n", error.c_str()); adb_close(localFd); return 1; } @@ -2031,12 +2012,12 @@ static int install_app(TransportType transport, const char* serial, int argc, co adb_close(localFd); adb_close(remoteFd); - if (strncmp("Success", buf, 7)) { - fprintf(stderr, "Failed to install %s: %s", file, buf); - return 1; + if (!strncmp("Success", buf, 7)) { + fputs(buf, stdout); + return 0; } - fputs(buf, stderr); - return 0; + fprintf(stderr, "adb: failed to install %s: %s", file, buf); + return 1; } static int install_multiple_app(TransportType transport, const char* serial, int argc, @@ -2058,10 +2039,7 @@ static int install_multiple_app(TransportType transport, const char* serial, int } } - if (first_apk == -1) { - fprintf(stderr, "No APK file on command line\n"); - return 1; - } + if (first_apk == -1) return syntax_error("need APK file on command line"); std::string install_cmd; if (_use_legacy_install()) { @@ -2079,7 +2057,7 @@ static int install_multiple_app(TransportType transport, const char* serial, int std::string error; int fd = adb_connect(cmd, &error); if (fd < 0) { - fprintf(stderr, "Connect error for create: %s\n", error.c_str()); + fprintf(stderr, "adb: connect error for create: %s\n", error.c_str()); return EXIT_FAILURE; } char buf[BUFSIZ]; @@ -2096,7 +2074,7 @@ static int install_multiple_app(TransportType transport, const char* serial, int } } if (session_id < 0) { - fprintf(stderr, "Failed to create session\n"); + fprintf(stderr, "adb: failed to create session\n"); fputs(buf, stderr); return EXIT_FAILURE; } @@ -2107,7 +2085,7 @@ static int install_multiple_app(TransportType transport, const char* serial, int const char* file = argv[i]; struct stat sb; if (stat(file, &sb) == -1) { - fprintf(stderr, "Failed to stat %s: %s\n", file, strerror(errno)); + fprintf(stderr, "adb: failed to stat %s: %s\n", file, strerror(errno)); success = 0; goto finalize_session; } @@ -2119,7 +2097,7 @@ static int install_multiple_app(TransportType transport, const char* serial, int int localFd = adb_open(file, O_RDONLY); if (localFd < 0) { - fprintf(stderr, "Failed to open %s: %s\n", file, strerror(errno)); + fprintf(stderr, "adb: failed to open %s: %s\n", file, strerror(errno)); success = 0; goto finalize_session; } @@ -2127,7 +2105,7 @@ static int install_multiple_app(TransportType transport, const char* serial, int std::string error; int remoteFd = adb_connect(cmd, &error); if (remoteFd < 0) { - fprintf(stderr, "Connect error for write: %s\n", error.c_str()); + fprintf(stderr, "adb: connect error for write: %s\n", error.c_str()); adb_close(localFd); success = 0; goto finalize_session; @@ -2140,7 +2118,7 @@ static int install_multiple_app(TransportType transport, const char* serial, int adb_close(remoteFd); if (strncmp("Success", buf, 7)) { - fprintf(stderr, "Failed to write %s\n", file); + fprintf(stderr, "adb: failed to write %s\n", file); fputs(buf, stderr); success = 0; goto finalize_session; @@ -2154,20 +2132,19 @@ finalize_session: install_cmd.c_str(), success ? "commit" : "abandon", session_id); fd = adb_connect(service, &error); if (fd < 0) { - fprintf(stderr, "Connect error for finalize: %s\n", error.c_str()); + fprintf(stderr, "adb: connect error for finalize: %s\n", error.c_str()); return EXIT_FAILURE; } read_status_line(fd, buf, sizeof(buf)); adb_close(fd); if (!strncmp("Success", buf, 7)) { - fputs(buf, stderr); + fputs(buf, stdout); return 0; - } else { - fprintf(stderr, "Failed to finalize session\n"); - fputs(buf, stderr); - return EXIT_FAILURE; } + fprintf(stderr, "adb: failed to finalize session\n"); + fputs(buf, stderr); + return EXIT_FAILURE; } static int pm_command(TransportType transport, const char* serial, int argc, const char** argv) { @@ -2225,10 +2202,7 @@ static int install_app_legacy(TransportType transport, const char* serial, int a } } - if (last_apk == -1) { - fprintf(stderr, "No APK file on command line\n"); - return EXIT_FAILURE; - } + if (last_apk == -1) return syntax_error("need APK file on command line"); int result = -1; std::vector apk_file = {argv[last_apk]}; diff --git a/adb/commandline.h b/adb/commandline.h index 0cf655c94..9ba69a396 100644 --- a/adb/commandline.h +++ b/adb/commandline.h @@ -87,7 +87,6 @@ class DefaultStandardStreamsCallback : public StandardStreamsCallbackInterface { 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.