Fix error handling/reporting for "adb forward" and "adb reverse".
We really need better infrastructure for parsing adb subcommands, but
in the meantime...
At least this cleans up a little more of the implementation too.
Bug: http://b/20736014
Change-Id: I76209847da3724906c71924017bcb69fa31e0b49
diff --git a/adb.cpp b/adb.cpp
index 2c959a4..9c1ead5 100644
--- a/adb.cpp
+++ b/adb.cpp
@@ -721,56 +721,50 @@
return 1;
}
- if (!strncmp(service, "forward:",8) ||
- !strncmp(service, "killforward:",12)) {
- char *local, *remote;
- atransport *transport;
-
- int createForward = strncmp(service, "kill", 4);
- int no_rebind = 0;
-
- local = strchr(service, ':') + 1;
-
- // Handle forward:norebind:<local>... here
- if (createForward && !strncmp(local, "norebind:", 9)) {
- no_rebind = 1;
- local = strchr(local, ':') + 1;
+ if (!strncmp(service, "forward:", 8) || !strncmp(service, "killforward:", 12)) {
+ // killforward:local
+ // forward:(norebind:)?local;remote
+ bool kill_forward = false;
+ bool no_rebind = false;
+ if (android::base::StartsWith(service, "killforward:")) {
+ kill_forward = true;
+ service += 12;
+ if (android::base::StartsWith(service, "norebind:")) {
+ no_rebind = true;
+ service += 9;
+ }
+ } else {
+ service += 8;
}
- remote = strchr(local,';');
+ std::vector<std::string> pieces = android::base::Split(service, ";");
- if (createForward) {
- // Check forward: parameter format: '<local>;<remote>'
- if(remote == 0) {
- SendFail(reply_fd, "malformed forward spec");
- return 1;
- }
-
- *remote++ = 0;
- if((local[0] == 0) || (remote[0] == 0) || (remote[0] == '*')) {
- SendFail(reply_fd, "malformed forward spec");
+ if (kill_forward) {
+ // Check killforward: parameter format: '<local>'
+ if (pieces.size() != 1 || pieces[0].empty()) {
+ SendFail(reply_fd, android::base::StringPrintf("bad killforward: %s", service));
return 1;
}
} else {
- // Check killforward: parameter format: '<local>'
- if (local[0] == 0) {
- SendFail(reply_fd, "malformed forward spec");
+ // Check forward: parameter format: '<local>;<remote>'
+ if (pieces.size() != 2 || pieces[0].empty() || pieces[1].empty() || pieces[1][0] == '*') {
+ SendFail(reply_fd, android::base::StringPrintf("bad forward: %s", service));
return 1;
}
}
std::string error_msg;
- transport = acquire_one_transport(kCsAny, type, serial, &error_msg);
+ atransport* transport = acquire_one_transport(kCsAny, type, serial, &error_msg);
if (!transport) {
SendFail(reply_fd, error_msg);
return 1;
}
InstallStatus r;
- if (createForward) {
- r = install_listener(local, remote, transport, no_rebind);
+ if (kill_forward) {
+ r = remove_listener(pieces[0].c_str(), transport);
} else {
- r = remove_listener(local, transport);
+ r = install_listener(pieces[0], pieces[1].c_str(), transport, no_rebind);
}
if (r == INSTALL_STATUS_OK) {
#if ADB_HOST
@@ -783,7 +777,7 @@
std::string message;
switch (r) {
- case INSTALL_STATUS_OK: message = " "; break;
+ case INSTALL_STATUS_OK: message = "success (!)"; break;
case INSTALL_STATUS_INTERNAL_ERROR: message = "internal error"; break;
case INSTALL_STATUS_CANNOT_BIND:
message = android::base::StringPrintf("cannot bind to socket: %s", strerror(errno));
@@ -791,7 +785,9 @@
case INSTALL_STATUS_CANNOT_REBIND:
message = android::base::StringPrintf("cannot rebind existing socket: %s", strerror(errno));
break;
- case INSTALL_STATUS_LISTENER_NOT_FOUND: message = "listener not found"; break;
+ case INSTALL_STATUS_LISTENER_NOT_FOUND:
+ message = android::base::StringPrintf("listener '%s' not found", service);
+ break;
}
SendFail(reply_fd, message);
return 1;
diff --git a/adb_client.cpp b/adb_client.cpp
index c73d737..ef9a586 100644
--- a/adb_client.cpp
+++ b/adb_client.cpp
@@ -256,19 +256,21 @@
}
-int adb_command(const std::string& service, std::string* error) {
- int fd = adb_connect(service, error);
+bool adb_command(const std::string& service) {
+ std::string error;
+ int fd = adb_connect(service, &error);
if (fd < 0) {
- fprintf(stderr, "error: %s\n", error->c_str());
- return -1;
+ fprintf(stderr, "error: %s\n", error.c_str());
+ return false;
}
- if (!adb_status(fd, error)) {
+ if (!adb_status(fd, &error)) {
+ fprintf(stderr, "error: %s\n", error.c_str());
adb_close(fd);
- return -1;
+ return false;
}
- return 0;
+ return true;
}
bool adb_query(const std::string& service, std::string* result, std::string* error) {
diff --git a/adb_client.h b/adb_client.h
index 9895c49..5de0638 100644
--- a/adb_client.h
+++ b/adb_client.h
@@ -26,9 +26,9 @@
int adb_connect(const std::string& service, std::string* error);
int _adb_connect(const std::string& service, std::string* error);
-// Connect to adb, connect to the named service, return 0 if the connection
-// succeeded AND the service returned OKAY.
-int adb_command(const std::string& service, std::string* error);
+// Connect to adb, connect to the named service, returns true if the connection
+// succeeded AND the service returned OKAY. Outputs any returned error otherwise.
+bool adb_command(const std::string& service);
// Connects to the named adb service and fills 'result' with the response.
// Returns true on success; returns false and fills 'error' on failure.
diff --git a/adb_listeners.cpp b/adb_listeners.cpp
index cf193ab..a335eec 100644
--- a/adb_listeners.cpp
+++ b/adb_listeners.cpp
@@ -26,13 +26,12 @@
int gListenAll = 0; /* Not static because it is used in commandline.c. */
-alistener listener_list = {
+static alistener listener_list = {
.next = &listener_list,
.prev = &listener_list,
};
-void ss_listener_event_func(int _fd, unsigned ev, void *_l)
-{
+static void ss_listener_event_func(int _fd, unsigned ev, void *_l) {
asocket *s;
if(ev & FDE_READ) {
@@ -56,7 +55,7 @@
}
}
-void listener_event_func(int _fd, unsigned ev, void* _l)
+static void listener_event_func(int _fd, unsigned ev, void* _l)
{
alistener* listener = reinterpret_cast<alistener*>(_l);
asocket *s;
@@ -106,38 +105,27 @@
free(l);
}
-void listener_disconnect(void* listener, atransport* t)
-{
+static void listener_disconnect(void* listener, atransport* t) {
free_listener(reinterpret_cast<alistener*>(listener));
}
-int local_name_to_fd(const char *name)
-{
- int port;
-
- if(!strncmp("tcp:", name, 4)){
- int ret;
- port = atoi(name + 4);
-
+static int local_name_to_fd(const char* name) {
+ if (!strncmp("tcp:", name, 4)) {
+ int port = atoi(name + 4);
if (gListenAll > 0) {
- ret = socket_inaddr_any_server(port, SOCK_STREAM);
+ return socket_inaddr_any_server(port, SOCK_STREAM);
} else {
- ret = socket_loopback_server(port, SOCK_STREAM);
+ return socket_loopback_server(port, SOCK_STREAM);
}
-
- return ret;
}
#ifndef HAVE_WIN32_IPC /* no Unix-domain sockets on Win32 */
- // It's non-sensical to support the "reserved" space on the adb host side
- if(!strncmp(name, "local:", 6)) {
- return socket_local_server(name + 6,
- ANDROID_SOCKET_NAMESPACE_ABSTRACT, SOCK_STREAM);
- } else if(!strncmp(name, "localabstract:", 14)) {
- return socket_local_server(name + 14,
- ANDROID_SOCKET_NAMESPACE_ABSTRACT, SOCK_STREAM);
- } else if(!strncmp(name, "localfilesystem:", 16)) {
- return socket_local_server(name + 16,
- ANDROID_SOCKET_NAMESPACE_FILESYSTEM, SOCK_STREAM);
+ // It's nonsensical to support the "reserved" space on the adb host side
+ if (!strncmp(name, "local:", 6)) {
+ return socket_local_server(name + 6, ANDROID_SOCKET_NAMESPACE_ABSTRACT, SOCK_STREAM);
+ } else if (!strncmp(name, "localabstract:", 14)) {
+ return socket_local_server(name + 14, ANDROID_SOCKET_NAMESPACE_ABSTRACT, SOCK_STREAM);
+ } else if (!strncmp(name, "localfilesystem:", 16)) {
+ return socket_local_server(name + 16, ANDROID_SOCKET_NAMESPACE_FILESYSTEM, SOCK_STREAM);
}
#endif
diff --git a/adb_listeners.h b/adb_listeners.h
index 9a7ded1..67deb21 100644
--- a/adb_listeners.h
+++ b/adb_listeners.h
@@ -30,12 +30,6 @@
INSTALL_STATUS_LISTENER_NOT_FOUND = -4,
};
-extern alistener listener_list;
-
-void listener_disconnect(void* _l, atransport* t);
-void listener_event_func(int _fd, unsigned ev, void *_l);
-void ss_listener_event_func(int _fd, unsigned ev, void *_l);
-
InstallStatus install_listener(const std::string& local_name,
const char* connect_to,
atransport* transport,
diff --git a/commandline.cpp b/commandline.cpp
index d7bee91..7fbca31 100644
--- a/commandline.cpp
+++ b/commandline.cpp
@@ -680,14 +680,7 @@
}
std::string cmd = format_host_command(service, t, serial);
- std::string error;
- if (adb_command(cmd, &error)) {
- D("failure: %s *\n", error.c_str());
- fprintf(stderr,"error: %s\n", error.c_str());
- return false;
- }
-
- return true;
+ return adb_command(cmd);
}
static int send_shell_command(TransportType transport_type, const char* serial,
@@ -1249,90 +1242,50 @@
if (argc != 1) return usage();
return send_shell_command(transport_type, serial, "shell:bugreport");
}
- /* adb_command() wrapper commands */
else if (!strcmp(argv[0], "forward") || !strcmp(argv[0], "reverse")) {
- std::string cmd;
- char host_prefix[64];
- char reverse = (char) !strcmp(argv[0], "reverse");
- char remove = 0;
- char remove_all = 0;
- char list = 0;
- char no_rebind = 0;
-
- // Parse options here.
- while (argc > 1 && argv[1][0] == '-') {
- if (!strcmp(argv[1], "--list"))
- list = 1;
- else if (!strcmp(argv[1], "--remove"))
- remove = 1;
- else if (!strcmp(argv[1], "--remove-all"))
- remove_all = 1;
- else if (!strcmp(argv[1], "--no-rebind"))
- no_rebind = 1;
- else {
- return usage();
- }
- argc--;
- argv++;
- }
-
- // Ensure we can only use one option at a time.
- if (list + remove + remove_all + no_rebind > 1) {
- return usage();
- }
+ bool reverse = !strcmp(argv[0], "reverse");
+ ++argv;
+ --argc;
+ if (argc < 1) return usage();
// Determine the <host-prefix> for this command.
+ std::string host_prefix;
if (reverse) {
- snprintf(host_prefix, sizeof host_prefix, "reverse");
+ host_prefix = "reverse";
} else {
if (serial) {
- snprintf(host_prefix, sizeof host_prefix, "host-serial:%s",
- serial);
+ host_prefix = android::base::StringPrintf("host-serial:%s", serial);
} else if (transport_type == kTransportUsb) {
- snprintf(host_prefix, sizeof host_prefix, "host-usb");
+ host_prefix = "host-usb";
} else if (transport_type == kTransportLocal) {
- snprintf(host_prefix, sizeof host_prefix, "host-local");
+ host_prefix = "host-local";
} else {
- snprintf(host_prefix, sizeof host_prefix, "host");
+ host_prefix = "host";
}
}
- // Implement forward --list
- if (list) {
- if (argc != 1) {
- return usage();
- }
-
- std::string query = android::base::StringPrintf("%s:list-forward", host_prefix);
- return adb_query_command(query);
- }
-
- // Implement forward --remove-all
- else if (remove_all) {
+ std::string cmd;
+ if (strcmp(argv[0], "--list") == 0) {
if (argc != 1) return usage();
- cmd = android::base::StringPrintf("%s:killforward-all", host_prefix);
- }
-
- // Implement forward --remove <local>
- else if (remove) {
+ return adb_query_command(host_prefix + ":list-forward");
+ } else if (strcmp(argv[0], "--remove-all") == 0) {
+ if (argc != 1) return usage();
+ cmd = host_prefix + ":killforward-all";
+ } else if (strcmp(argv[0], "--remove") == 0) {
+ // forward --remove <local>
if (argc != 2) return usage();
- cmd = android::base::StringPrintf("%s:killforward:%s", host_prefix, argv[1]);
- }
- // Or implement one of:
- // forward <local> <remote>
- // forward --no-rebind <local> <remote>
- else {
+ cmd = host_prefix + ":killforward:" + argv[1];
+ } else if (strcmp(argv[0], "--no-rebind") == 0) {
+ // forward --no-rebind <local> <remote>
if (argc != 3) return usage();
- const char* command = no_rebind ? "forward:norebind" : "forward";
- cmd = android::base::StringPrintf("%s:%s:%s;%s", host_prefix, command, argv[1], argv[2]);
+ cmd = host_prefix + ":forward:norebind:" + argv[1] + ";" + argv[2];
+ } else {
+ // forward <local> <remote>
+ if (argc != 2) return usage();
+ cmd = host_prefix + ":forward:" + argv[0] + ";" + argv[1];
}
- std::string error;
- if (adb_command(cmd, &error)) {
- fprintf(stderr, "error: %s\n", error.c_str());
- return 1;
- }
- return 0;
+ return adb_command(cmd) ? 0 : 1;
}
/* do_sync_*() commands */
else if (!strcmp(argv[0], "ls")) {