adb: rationalize fatal/error logging.
Let's use LOG(FATAL)/PLOG(FATAL) for actual fatal stuff.
Add a Windows error(3) and move folks who didn't really mean "abort"
fatal over to it. Also get rid of syntax_error which wasn't adding a
lot of value, and most of the places it was adding "usage: " didn't seem
entirely appropriate anyway.
In particular, we seemed to have confused fastdeploy.cpp into aborting
in most user error cases, and none of the reviewers noticed. Clearly
we'd all lost track of far too many options.
(I've also cleaned up a few random instances of fprintf(3) + exit(2).)
Bug: N/A
Test: manual
Change-Id: I3e8440848a24e30d928de9eded505916bc324786
diff --git a/adb/client/commandline.cpp b/adb/client/commandline.cpp
index 07b5747..0e03201 100644
--- a/adb/client/commandline.cpp
+++ b/adb/client/commandline.cpp
@@ -71,8 +71,7 @@
static std::string product_file(const std::string& file) {
const char* ANDROID_PRODUCT_OUT = getenv("ANDROID_PRODUCT_OUT");
if (ANDROID_PRODUCT_OUT == nullptr) {
- fprintf(stderr, "adb: product directory not specified; set $ANDROID_PRODUCT_OUT\n");
- exit(1);
+ error(1, 0, "product directory not specified; set $ANDROID_PRODUCT_OUT");
}
return std::string{ANDROID_PRODUCT_OUT} + OS_PATH_SEPARATOR_STR + file;
}
@@ -318,7 +317,7 @@
#ifdef _WIN32
old_stdin_mode = _setmode(STDIN_FILENO, _O_BINARY);
if (old_stdin_mode == -1) {
- fatal_errno("could not set stdin to binary");
+ PLOG(FATAL) << "could not set stdin to binary";
}
#endif
}
@@ -327,7 +326,7 @@
if (outFd == STDOUT_FILENO) {
old_stdout_mode = _setmode(STDOUT_FILENO, _O_BINARY);
if (old_stdout_mode == -1) {
- fatal_errno("could not set stdout to binary");
+ PLOG(FATAL) << "could not set stdout to binary";
}
}
#endif
@@ -338,7 +337,7 @@
stdin_raw_restore();
#ifdef _WIN32
if (_setmode(STDIN_FILENO, old_stdin_mode) == -1) {
- fatal_errno("could not restore stdin mode");
+ PLOG(FATAL) << "could not restore stdin mode";
}
#endif
}
@@ -346,7 +345,7 @@
#ifdef _WIN32
if (outFd == STDOUT_FILENO) {
if (_setmode(STDOUT_FILENO, old_stdout_mode) == -1) {
- fatal_errno("could not restore stdout mode");
+ PLOG(FATAL) << "could not restore stdout mode";
}
}
#endif
@@ -660,9 +659,9 @@
static int adb_shell(int argc, const char** argv) {
FeatureSet features;
- std::string error;
- if (!adb_get_feature_set(&features, &error)) {
- fprintf(stderr, "error: %s\n", error.c_str());
+ std::string error_message;
+ if (!adb_get_feature_set(&features, &error_message)) {
+ fprintf(stderr, "error: %s\n", error_message.c_str());
return 1;
}
@@ -685,7 +684,7 @@
switch (opt) {
case 'e':
if (!(strlen(optarg) == 1 || strcmp(optarg, "none") == 0)) {
- return syntax_error("-e requires a single-character argument or 'none'");
+ error(1, 0, "-e requires a single-character argument or 'none'");
}
escape_char = (strcmp(optarg, "none") == 0) ? 0 : optarg[0];
break;
@@ -933,31 +932,29 @@
*/
static int ppp(int argc, const char** argv) {
#if defined(_WIN32)
- fprintf(stderr, "error: adb %s not implemented on Win32\n", argv[0]);
- return -1;
+ error(1, 0, "adb %s not implemented on Win32", argv[0]);
+ __builtin_unreachable();
#else
- if (argc < 2) return syntax_error("adb %s <adb service name> [ppp opts]", argv[0]);
+ if (argc < 2) error(1, 0, "usage: adb %s <adb service name> [ppp opts]", argv[0]);
const char* adb_service_name = argv[1];
- std::string error;
- int fd = adb_connect(adb_service_name, &error);
+ std::string error_message;
+ int fd = adb_connect(adb_service_name, &error_message);
if (fd < 0) {
- fprintf(stderr, "adb: could not open adb service %s: %s\n", adb_service_name, error.c_str());
- return 1;
+ error(1, 0, "could not open adb service %s: %s", adb_service_name, error_message.c_str());
}
pid_t pid = fork();
+ if (pid == -1) {
+ error(1, errno, "fork failed");
+ }
- if (pid < 0) {
- perror("from fork()");
- return 1;
- } else if (pid == 0) {
- int err;
+ if (pid == 0) {
+ // child side
int i;
- const char **ppp_args;
// copy args
- ppp_args = (const char **) alloca(sizeof(char *) * argc + 1);
+ const char** ppp_args = (const char**)alloca(sizeof(char*) * argc + 1);
ppp_args[0] = "pppd";
for (i = 2 ; i < argc ; i++) {
//argv[2] and beyond become ppp_args[1] and beyond
@@ -965,25 +962,18 @@
}
ppp_args[i-1] = nullptr;
- // child side
-
dup2(fd, STDIN_FILENO);
dup2(fd, STDOUT_FILENO);
adb_close(STDERR_FILENO);
adb_close(fd);
- err = execvp("pppd", (char * const *)ppp_args);
-
- if (err < 0) {
- perror("execing pppd");
- }
- exit(-1);
- } else {
- // parent side
-
- adb_close(fd);
- return 0;
+ execvp("pppd", (char* const*)ppp_args);
+ error(1, errno, "exec pppd failed");
}
+
+ // parent side
+ adb_close(fd);
+ return 0;
#endif /* !defined(_WIN32) */
}
@@ -1161,7 +1151,7 @@
/* find, extract, and use any -f argument */
for (int i = 1; i < argc; i++) {
if (!strcmp("-f", argv[i])) {
- if (i == argc - 1) return syntax_error("backup -f passed with no filename");
+ if (i == argc - 1) error(1, 0, "backup -f passed with no filename");
filename = argv[i+1];
for (int j = i+2; j <= argc; ) {
argv[i++] = argv[j++];
@@ -1173,7 +1163,7 @@
// Bare "adb backup" or "adb backup -f filename" are not valid invocations ---
// a list of packages is required.
- if (argc < 2) return syntax_error("backup either needs a list of packages or -all/-shared");
+ if (argc < 2) error(1, 0, "backup either needs a list of packages or -all/-shared");
adb_unlink(filename);
int outFd = adb_creat(filename, 0640);
@@ -1209,7 +1199,7 @@
}
static int restore(int argc, const char** argv) {
- if (argc != 2) return syntax_error("restore requires an argument");
+ if (argc != 2) error(1, 0, "restore requires an argument");
const char* filename = argv[1];
int tarFd = adb_open(filename, O_RDONLY);
@@ -1263,8 +1253,7 @@
} else if (!strcmp(*arg, "--")) {
ignore_flags = true;
} else {
- syntax_error("unrecognized option '%s'", *arg);
- exit(1);
+ error(1, 0, "unrecognized option '%s'", *arg);
}
}
++arg;
@@ -1342,7 +1331,7 @@
/* this is a special flag used only when the ADB client launches the ADB Server */
is_daemon = true;
} else if (!strcmp(argv[0], "--reply-fd")) {
- if (argc < 2) return syntax_error("--reply-fd requires an argument");
+ if (argc < 2) error(1, 0, "--reply-fd requires an argument");
const char* reply_fd_str = argv[1];
argc--;
argv++;
@@ -1355,7 +1344,7 @@
if (isdigit(argv[0][2])) {
serial = argv[0] + 2;
} else {
- if (argc < 2 || argv[0][2] != '\0') return syntax_error("-s requires an argument");
+ if (argc < 2 || argv[0][2] != '\0') error(1, 0, "-s requires an argument");
serial = argv[1];
argc--;
argv++;
@@ -1371,7 +1360,7 @@
}
transport_id = strtoll(id, const_cast<char**>(&id), 10);
if (*id != '\0') {
- return syntax_error("invalid transport id");
+ error(1, 0, "invalid transport id");
}
} else if (!strcmp(argv[0],"-d")) {
transport_type = kTransportUsb;
@@ -1381,7 +1370,7 @@
gListenAll = 1;
} else if (!strncmp(argv[0], "-H", 2)) {
if (argv[0][2] == '\0') {
- if (argc < 2) return syntax_error("-H requires an argument");
+ if (argc < 2) error(1, 0, "-H requires an argument");
server_host_str = argv[1];
argc--;
argv++;
@@ -1390,7 +1379,7 @@
}
} else if (!strncmp(argv[0], "-P", 2)) {
if (argv[0][2] == '\0') {
- if (argc < 2) return syntax_error("-P requires an argument");
+ if (argc < 2) error(1, 0, "-P requires an argument");
server_port_str = argv[1];
argc--;
argv++;
@@ -1398,7 +1387,7 @@
server_port_str = argv[0] + 2;
}
} else if (!strcmp(argv[0], "-L")) {
- if (argc < 2) return syntax_error("-L requires an argument");
+ if (argc < 2) error(1, 0, "-L requires an argument");
server_socket_str = argv[1];
argc--;
argv++;
@@ -1411,7 +1400,7 @@
}
if ((server_host_str || server_port_str) && server_socket_str) {
- return syntax_error("-L is incompatible with -H or -P");
+ error(1, 0, "-L is incompatible with -H or -P");
}
// If -L, -H, or -P are specified, ignore environment variables.
@@ -1428,11 +1417,10 @@
server_port_str = server_port_str ? server_port_str : getenv("ANDROID_ADB_SERVER_PORT");
if (server_port_str && strlen(server_port_str) > 0) {
if (!android::base::ParseInt(server_port_str, &server_port, 1, 65535)) {
- fprintf(stderr,
- "adb: Env var ANDROID_ADB_SERVER_PORT must be a positive"
- " number less than 65535. Got \"%s\"\n",
- server_port_str);
- exit(1);
+ error(1, 0,
+ "$ANDROID_ADB_SERVER_PORT must be a positive number less than 65535: "
+ "got \"%s\"",
+ server_port_str);
}
}
@@ -1444,7 +1432,7 @@
rc = asprintf(&temp, "tcp:%d", server_port);
}
if (rc < 0) {
- fatal("failed to allocate server socket specification");
+ LOG(FATAL) << "failed to allocate server socket specification";
}
server_socket_str = temp;
}
@@ -1506,7 +1494,7 @@
} else if (argc == 2 && !strcmp(argv[1], "-l")) {
listopt = argv[1];
} else {
- return syntax_error("adb devices [-l]");
+ error(1, 0, "adb devices [-l]");
}
std::string query = android::base::StringPrintf("host:%s%s", argv[0], listopt);
@@ -1514,13 +1502,13 @@
return adb_query_command(query);
}
else if (!strcmp(argv[0], "connect")) {
- if (argc != 2) return syntax_error("adb connect <host>[:<port>]");
+ if (argc != 2) error(1, 0, "usage: adb connect <host>[:<port>]");
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) return syntax_error("adb disconnect [<host>[:<port>]]");
+ if (argc > 2) error(1, 0, "usage: adb disconnect [<host>[:<port>]]");
std::string query = android::base::StringPrintf("host:disconnect:%s",
(argc == 2) ? argv[1] : "");
@@ -1535,7 +1523,7 @@
else if (!strcmp(argv[0], "exec-in") || !strcmp(argv[0], "exec-out")) {
int exec_in = !strcmp(argv[0], "exec-in");
- if (argc < 2) return syntax_error("adb %s command", argv[0]);
+ if (argc < 2) error(1, 0, "usage: adb %s command", argv[0]);
std::string cmd = "exec:";
cmd += argv[1];
@@ -1565,17 +1553,17 @@
return adb_kill_server() ? 0 : 1;
}
else if (!strcmp(argv[0], "sideload")) {
- if (argc != 2) return syntax_error("sideload requires an argument");
+ if (argc != 2) error(1, 0, "sideload requires an argument");
if (adb_sideload_host(argv[1])) {
return 1;
} else {
return 0;
}
} else if (!strcmp(argv[0], "tcpip")) {
- if (argc != 2) return syntax_error("tcpip requires an argument");
+ if (argc != 2) error(1, 0, "tcpip requires an argument");
int port;
if (!android::base::ParseInt(argv[1], &port, 1, 65535)) {
- return syntax_error("tcpip: invalid port: %s", argv[1]);
+ error(1, 0, "tcpip: invalid port: %s", argv[1]);
}
return adb_connect_command(android::base::StringPrintf("tcpip:%d", port));
}
@@ -1607,7 +1595,7 @@
} else if (!strcmp(argv[0], "forward") || !strcmp(argv[0], "reverse")) {
bool reverse = !strcmp(argv[0], "reverse");
--argc;
- if (argc < 1) return syntax_error("%s requires an argument", argv[0]);
+ if (argc < 1) error(1, 0, "%s requires an argument", argv[0]);
++argv;
// Determine the <host-prefix> for this command.
@@ -1626,46 +1614,44 @@
}
}
- std::string cmd, error;
+ std::string cmd, error_message;
if (strcmp(argv[0], "--list") == 0) {
- if (argc != 1) return syntax_error("--list doesn't take any arguments");
+ if (argc != 1) error(1, 0, "--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 syntax_error("--remove-all doesn't take any arguments");
+ if (argc != 1) error(1, 0, "--remove-all doesn't take any arguments");
cmd = host_prefix + ":killforward-all";
} else if (strcmp(argv[0], "--remove") == 0) {
// forward --remove <local>
- if (argc != 2) return syntax_error("--remove requires an argument");
+ if (argc != 2) error(1, 0, "--remove requires an argument");
cmd = host_prefix + ":killforward:" + argv[1];
} else if (strcmp(argv[0], "--no-rebind") == 0) {
// forward --no-rebind <local> <remote>
- if (argc != 3) return syntax_error("--no-rebind takes two arguments");
- if (forward_targets_are_valid(argv[1], argv[2], &error)) {
+ if (argc != 3) error(1, 0, "--no-rebind takes two arguments");
+ if (forward_targets_are_valid(argv[1], argv[2], &error_message)) {
cmd = host_prefix + ":forward:norebind:" + argv[1] + ";" + argv[2];
}
} else {
// forward <local> <remote>
- if (argc != 2) return syntax_error("forward takes two arguments");
- if (forward_targets_are_valid(argv[0], argv[1], &error)) {
+ if (argc != 2) error(1, 0, "forward takes two arguments");
+ if (forward_targets_are_valid(argv[0], argv[1], &error_message)) {
cmd = host_prefix + ":forward:" + argv[0] + ";" + argv[1];
}
}
- if (!error.empty()) {
- fprintf(stderr, "error: %s\n", error.c_str());
- return 1;
+ if (!error_message.empty()) {
+ error(1, 0, "error: %s", error_message.c_str());
}
- int fd = adb_connect(cmd, &error);
- if (fd < 0 || !adb_status(fd, &error)) {
+ int fd = adb_connect(cmd, &error_message);
+ if (fd < 0 || !adb_status(fd, &error_message)) {
adb_close(fd);
- fprintf(stderr, "error: %s\n", error.c_str());
- return 1;
+ error(1, 0, "error: %s", error_message.c_str());
}
// Server or device may optionally return a resolved TCP port number.
std::string resolved_port;
- if (ReadProtocolString(fd, &resolved_port, &error) && !resolved_port.empty()) {
+ if (ReadProtocolString(fd, &resolved_port, &error_message) && !resolved_port.empty()) {
printf("%s\n", resolved_port.c_str());
}
@@ -1674,7 +1660,7 @@
}
/* do_sync_*() commands */
else if (!strcmp(argv[0], "ls")) {
- if (argc != 2) return syntax_error("ls requires an argument");
+ if (argc != 2) error(1, 0, "ls requires an argument");
return do_sync_ls(argv[1]) ? 0 : 1;
}
else if (!strcmp(argv[0], "push")) {
@@ -1684,7 +1670,7 @@
const char* dst = nullptr;
parse_push_pull_args(&argv[1], argc - 1, &srcs, &dst, ©_attrs, &sync);
- if (srcs.empty() || !dst) return syntax_error("push requires an argument");
+ if (srcs.empty() || !dst) error(1, 0, "push requires an argument");
return do_sync_push(srcs, dst, sync) ? 0 : 1;
}
else if (!strcmp(argv[0], "pull")) {
@@ -1693,19 +1679,19 @@
const char* dst = ".";
parse_push_pull_args(&argv[1], argc - 1, &srcs, &dst, ©_attrs, nullptr);
- if (srcs.empty()) return syntax_error("pull requires an argument");
+ if (srcs.empty()) error(1, 0, "pull requires an argument");
return do_sync_pull(srcs, dst, copy_attrs) ? 0 : 1;
}
else if (!strcmp(argv[0], "install")) {
- if (argc < 2) return syntax_error("install requires an argument");
+ if (argc < 2) error(1, 0, "install requires an argument");
return install_app(argc, argv);
}
else if (!strcmp(argv[0], "install-multiple")) {
- if (argc < 2) return syntax_error("install-multiple requires an argument");
+ if (argc < 2) error(1, 0, "install-multiple requires an argument");
return install_multiple_app(argc, argv);
}
else if (!strcmp(argv[0], "uninstall")) {
- if (argc < 2) return syntax_error("uninstall requires an argument");
+ if (argc < 2) error(1, 0, "uninstall requires an argument");
return uninstall_app(argc, argv);
}
else if (!strcmp(argv[0], "sync")) {
@@ -1719,7 +1705,7 @@
} else if (argc == 2) {
src = argv[1];
} else {
- return syntax_error("adb sync [-l] [PARTITION]");
+ error(1, 0, "usage: adb sync [-l] [PARTITION]");
}
if (src.empty()) src = "all";
@@ -1734,7 +1720,8 @@
if (!do_sync_sync(src_dir, "/" + partition, list_only)) return 1;
}
}
- return found ? 0 : syntax_error("don't know how to sync %s partition", src.c_str());
+ if (!found) error(1, 0, "don't know how to sync %s partition", src.c_str());
+ return 0;
}
/* passthrough commands */
else if (!strcmp(argv[0],"get-state") ||
@@ -1765,7 +1752,7 @@
return restore(argc, argv);
}
else if (!strcmp(argv[0], "keygen")) {
- if (argc != 2) return syntax_error("keygen requires an argument");
+ if (argc != 2) error(1, 0, "keygen requires an argument");
// Always print key generation information for keygen command.
adb_trace_enable(AUTH);
return adb_auth_keygen(argv[1]);
@@ -1780,7 +1767,7 @@
return adb_connect_command("host:track-devices");
} else if (!strcmp(argv[0], "raw")) {
if (argc != 2) {
- return syntax_error("adb raw SERVICE");
+ error(1, 0, "usage: adb raw SERVICE");
}
return adb_connect_command(argv[1]);
}
@@ -1823,11 +1810,11 @@
std::string err;
return adb_query_command("host:reconnect-offline");
} else {
- return syntax_error("adb reconnect [device|offline]");
+ error(1, 0, "usage: adb reconnect [device|offline]");
}
}
}
- syntax_error("unknown command %s", argv[0]);
- return 1;
+ error(1, 0, "unknown command %s", argv[0]);
+ __builtin_unreachable();
}