From f801d8b3128cf5aae3725c981f32abfd4b6c307e Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Fri, 19 Nov 2021 22:42:49 +0100 Subject: [PATCH 01/13] Expose flags for process execution Let the caller decide if stdout and stderr must be inherited on process creation, i.e. if stdout and stderr of the child process should be printed in the scrcpy console. This allows to get output and errors for specific adb commands depending on the context. PR #2827 --- app/src/adb.c | 2 +- app/src/sys/unix/process.c | 15 ++++++++++++++- app/src/sys/win/process.c | 25 +++++++++++++++++-------- app/src/util/process.c | 4 ++-- app/src/util/process.h | 16 ++++++++++++++-- 5 files changed, 48 insertions(+), 14 deletions(-) diff --git a/app/src/adb.c b/app/src/adb.c index 92744164c8..729b57247a 100644 --- a/app/src/adb.c +++ b/app/src/adb.c @@ -181,7 +181,7 @@ adb_execute_p(const char *serial, const char *const adb_cmd[], size_t len, sc_pid pid; enum sc_process_result r = - sc_process_execute_p(argv, &pid, NULL, pout, NULL); + sc_process_execute_p(argv, &pid, 0, NULL, pout, NULL); if (r != SC_PROCESS_SUCCESS) { show_adb_err_msg(r, argv); pid = SC_PROCESS_NONE; diff --git a/app/src/sys/unix/process.c b/app/src/sys/unix/process.c index 5f4a9890f8..54a1bb80b3 100644 --- a/app/src/sys/unix/process.c +++ b/app/src/sys/unix/process.c @@ -11,8 +11,11 @@ #include "util/log.h" enum sc_process_result -sc_process_execute_p(const char *const argv[], sc_pid *pid, +sc_process_execute_p(const char *const argv[], sc_pid *pid, unsigned flags, int *pin, int *pout, int *perr) { + bool inherit_stdout = !pout && !(flags & SC_PROCESS_NO_STDOUT); + bool inherit_stderr = !perr && !(flags & SC_PROCESS_NO_STDERR); + int in[2]; int out[2]; int err[2]; @@ -90,20 +93,30 @@ sc_process_execute_p(const char *const argv[], sc_pid *pid, } close(in[1]); } + // Do not close stdin in the child process, this makes adb fail on Linux + if (pout) { if (out[1] != STDOUT_FILENO) { dup2(out[1], STDOUT_FILENO); close(out[1]); } close(out[0]); + } else if (!inherit_stdout) { + // Close stdout in the child process + close(STDOUT_FILENO); } + if (perr) { if (err[1] != STDERR_FILENO) { dup2(err[1], STDERR_FILENO); close(err[1]); } close(err[0]); + } else if (!inherit_stderr) { + // Close stderr in the child process + close(STDERR_FILENO); } + close(internal[0]); enum sc_process_result err; if (fcntl(internal[1], F_SETFD, FD_CLOEXEC) == 0) { diff --git a/app/src/sys/win/process.c b/app/src/sys/win/process.c index 1d35629382..bed9847901 100644 --- a/app/src/sys/win/process.c +++ b/app/src/sys/win/process.c @@ -24,12 +24,17 @@ build_cmd(char *cmd, size_t len, const char *const argv[]) { } enum sc_process_result -sc_process_execute_p(const char *const argv[], HANDLE *handle, +sc_process_execute_p(const char *const argv[], HANDLE *handle, unsigned flags, HANDLE *pin, HANDLE *pout, HANDLE *perr) { - enum sc_process_result ret = SC_PROCESS_ERROR_GENERIC; + bool inherit_stdout = !pout && !(flags & SC_PROCESS_NO_STDOUT); + bool inherit_stderr = !perr && !(flags & SC_PROCESS_NO_STDERR); // Add 1 per non-NULL pointer - unsigned handle_count = !!pin + !!pout + !!perr; + unsigned handle_count = !!pin + + (pout || inherit_stdout) + + (perr || inherit_stderr); + + enum sc_process_result ret = SC_PROCESS_ERROR_GENERIC; SECURITY_ATTRIBUTES sa; sa.nLength = sizeof(SECURITY_ATTRIBUTES); @@ -85,12 +90,14 @@ sc_process_execute_p(const char *const argv[], HANDLE *handle, si.StartupInfo.hStdInput = stdin_read_handle; handles[i++] = si.StartupInfo.hStdInput; } - if (pout) { - si.StartupInfo.hStdOutput = stdout_write_handle; + if (pout || inherit_stdout) { + si.StartupInfo.hStdOutput = pout ? stdout_write_handle + : GetStdHandle(STD_OUTPUT_HANDLE); handles[i++] = si.StartupInfo.hStdOutput; } - if (perr) { - si.StartupInfo.hStdError = stderr_write_handle; + if (perr || inherit_stderr) { + si.StartupInfo.hStdError = perr ? stderr_write_handle + : GetStdHandle(STD_ERROR_HANDLE); handles[i++] = si.StartupInfo.hStdError; } @@ -140,7 +147,9 @@ sc_process_execute_p(const char *const argv[], HANDLE *handle, } BOOL bInheritHandles = handle_count > 0; - DWORD dwCreationFlags = handle_count > 0 ? EXTENDED_STARTUPINFO_PRESENT : 0; + // DETACHED_PROCESS to disable stdin, stdout and stderr + DWORD dwCreationFlags = handle_count > 0 ? EXTENDED_STARTUPINFO_PRESENT + : DETACHED_PROCESS; BOOL ok = CreateProcessW(NULL, wide, NULL, NULL, bInheritHandles, dwCreationFlags, NULL, NULL, &si.StartupInfo, &pi); free(wide); diff --git a/app/src/util/process.c b/app/src/util/process.c index 38931d9c82..ad1af0a970 100644 --- a/app/src/util/process.c +++ b/app/src/util/process.c @@ -5,8 +5,8 @@ #include "log.h" enum sc_process_result -sc_process_execute(const char *const argv[], sc_pid *pid) { - return sc_process_execute_p(argv, pid, NULL, NULL, NULL); +sc_process_execute(const char *const argv[], sc_pid *pid, unsigned flags) { + return sc_process_execute_p(argv, pid, flags, NULL, NULL, NULL); } ssize_t diff --git a/app/src/util/process.h b/app/src/util/process.h index 14bc060e25..17c09bc58b 100644 --- a/app/src/util/process.h +++ b/app/src/util/process.h @@ -67,20 +67,32 @@ enum sc_process_result { SC_PROCESS_ERROR_MISSING_BINARY, }; +#define SC_PROCESS_NO_STDOUT (1 << 0) +#define SC_PROCESS_NO_STDERR (1 << 1) + /** * Execute the command and write the process id to `pid` + * + * The `flags` argument is a bitwise OR of the following values: + * - SC_PROCESS_NO_STDOUT + * - SC_PROCESS_NO_STDERR + * + * It indicates if stdout and stderr must be inherited from the scrcpy process + * (i.e. if the process must output to the scrcpy console). */ enum sc_process_result -sc_process_execute(const char *const argv[], sc_pid *pid); +sc_process_execute(const char *const argv[], sc_pid *pid, unsigned flags); /** * Execute the command and write the process id to `pid` * * If not NULL, provide a pipe for stdin (`pin`), stdout (`pout`) and stderr * (`perr`). + * + * The `flags` argument has the same semantics as in `sc_process_execute()`. */ enum sc_process_result -sc_process_execute_p(const char *const argv[], sc_pid *pid, +sc_process_execute_p(const char *const argv[], sc_pid *pid, unsigned flags, sc_pipe *pin, sc_pipe *pout, sc_pipe *perr); /** From e3d4aa8c5d42279e0095404d76f2b5bc2b7c52d7 Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Wed, 24 Nov 2021 22:27:28 +0100 Subject: [PATCH 02/13] Use flags for adb commands Explicitly indicate, for each adb call, if stdout and stderr must be inherited. PR #2827 --- app/src/adb.c | 49 ++++++++++++++++++++++++++---------------- app/src/adb.h | 22 ++++++++++++------- app/src/adb_tunnel.c | 14 +++++++----- app/src/file_handler.c | 4 ++-- app/src/server.c | 7 +++--- 5 files changed, 60 insertions(+), 36 deletions(-) diff --git a/app/src/adb.c b/app/src/adb.c index 729b57247a..13018f4dd0 100644 --- a/app/src/adb.c +++ b/app/src/adb.c @@ -173,16 +173,26 @@ adb_create_argv(const char *serial, const char *const adb_cmd[], size_t len) { static sc_pid adb_execute_p(const char *serial, const char *const adb_cmd[], size_t len, - sc_pipe *pout) { + unsigned flags, sc_pipe *pout) { const char **argv = adb_create_argv(serial, adb_cmd, len); if (!argv) { return SC_PROCESS_NONE; } + unsigned process_flags = 0; + if (flags & SC_ADB_NO_STDOUT) { + process_flags |= SC_PROCESS_NO_STDOUT; + } + if (flags & SC_ADB_NO_STDERR) { + process_flags |= SC_PROCESS_NO_STDERR; + } + sc_pid pid; enum sc_process_result r = - sc_process_execute_p(argv, &pid, 0, NULL, pout, NULL); + sc_process_execute_p(argv, &pid, process_flags, NULL, pout, NULL); if (r != SC_PROCESS_SUCCESS) { + // If the execution itself failed (not the command exit code), log the + // error in all cases show_adb_err_msg(r, argv); pid = SC_PROCESS_NONE; } @@ -192,61 +202,63 @@ adb_execute_p(const char *serial, const char *const adb_cmd[], size_t len, } sc_pid -adb_execute(const char *serial, const char *const adb_cmd[], size_t len) { - return adb_execute_p(serial, adb_cmd, len, NULL); +adb_execute(const char *serial, const char *const adb_cmd[], size_t len, + unsigned flags) { + return adb_execute_p(serial, adb_cmd, len, flags, NULL); } bool adb_forward(struct sc_intr *intr, const char *serial, uint16_t local_port, - const char *device_socket_name) { + const char *device_socket_name, unsigned flags) { char local[4 + 5 + 1]; // tcp:PORT char remote[108 + 14 + 1]; // localabstract:NAME sprintf(local, "tcp:%" PRIu16, local_port); snprintf(remote, sizeof(remote), "localabstract:%s", device_socket_name); const char *const adb_cmd[] = {"forward", local, remote}; - sc_pid pid = adb_execute(serial, adb_cmd, ARRAY_LEN(adb_cmd)); + sc_pid pid = adb_execute(serial, adb_cmd, ARRAY_LEN(adb_cmd), flags); return process_check_success_intr(intr, pid, "adb forward"); } bool adb_forward_remove(struct sc_intr *intr, const char *serial, - uint16_t local_port) { + uint16_t local_port, unsigned flags) { char local[4 + 5 + 1]; // tcp:PORT sprintf(local, "tcp:%" PRIu16, local_port); const char *const adb_cmd[] = {"forward", "--remove", local}; - sc_pid pid = adb_execute(serial, adb_cmd, ARRAY_LEN(adb_cmd)); + sc_pid pid = adb_execute(serial, adb_cmd, ARRAY_LEN(adb_cmd), flags); return process_check_success_intr(intr, pid, "adb forward --remove"); } bool adb_reverse(struct sc_intr *intr, const char *serial, - const char *device_socket_name, uint16_t local_port) { + const char *device_socket_name, uint16_t local_port, + unsigned flags) { char local[4 + 5 + 1]; // tcp:PORT char remote[108 + 14 + 1]; // localabstract:NAME sprintf(local, "tcp:%" PRIu16, local_port); snprintf(remote, sizeof(remote), "localabstract:%s", device_socket_name); const char *const adb_cmd[] = {"reverse", remote, local}; - sc_pid pid = adb_execute(serial, adb_cmd, ARRAY_LEN(adb_cmd)); + sc_pid pid = adb_execute(serial, adb_cmd, ARRAY_LEN(adb_cmd), flags); return process_check_success_intr(intr, pid, "adb reverse"); } bool adb_reverse_remove(struct sc_intr *intr, const char *serial, - const char *device_socket_name) { + const char *device_socket_name, unsigned flags) { char remote[108 + 14 + 1]; // localabstract:NAME snprintf(remote, sizeof(remote), "localabstract:%s", device_socket_name); const char *const adb_cmd[] = {"reverse", "--remove", remote}; - sc_pid pid = adb_execute(serial, adb_cmd, ARRAY_LEN(adb_cmd)); + sc_pid pid = adb_execute(serial, adb_cmd, ARRAY_LEN(adb_cmd), flags); return process_check_success_intr(intr, pid, "adb reverse --remove"); } bool adb_push(struct sc_intr *intr, const char *serial, const char *local, - const char *remote) { + const char *remote, unsigned flags) { #ifdef __WINDOWS__ // Windows will parse the string, so the paths must be quoted // (see sys/win/command.c) @@ -262,7 +274,7 @@ adb_push(struct sc_intr *intr, const char *serial, const char *local, #endif const char *const adb_cmd[] = {"push", local, remote}; - sc_pid pid = adb_execute(serial, adb_cmd, ARRAY_LEN(adb_cmd)); + sc_pid pid = adb_execute(serial, adb_cmd, ARRAY_LEN(adb_cmd), flags); #ifdef __WINDOWS__ free((void *) remote); @@ -273,7 +285,8 @@ adb_push(struct sc_intr *intr, const char *serial, const char *local, } bool -adb_install(struct sc_intr *intr, const char *serial, const char *local) { +adb_install(struct sc_intr *intr, const char *serial, const char *local, + unsigned flags) { #ifdef __WINDOWS__ // Windows will parse the string, so the local name must be quoted // (see sys/win/command.c) @@ -284,7 +297,7 @@ adb_install(struct sc_intr *intr, const char *serial, const char *local) { #endif const char *const adb_cmd[] = {"install", "-r", local}; - sc_pid pid = adb_execute(serial, adb_cmd, ARRAY_LEN(adb_cmd)); + sc_pid pid = adb_execute(serial, adb_cmd, ARRAY_LEN(adb_cmd), flags); #ifdef __WINDOWS__ free((void *) local); @@ -294,11 +307,11 @@ adb_install(struct sc_intr *intr, const char *serial, const char *local) { } char * -adb_get_serialno(struct sc_intr *intr) { +adb_get_serialno(struct sc_intr *intr, unsigned flags) { const char *const adb_cmd[] = {"get-serialno"}; sc_pipe pout; - sc_pid pid = adb_execute_p(NULL, adb_cmd, ARRAY_LEN(adb_cmd), &pout); + sc_pid pid = adb_execute_p(NULL, adb_cmd, ARRAY_LEN(adb_cmd), flags, &pout); if (pid == SC_PROCESS_NONE) { LOGE("Could not execute \"adb get-serialno\""); return NULL; diff --git a/app/src/adb.h b/app/src/adb.h index f58bc16517..5033965bf4 100644 --- a/app/src/adb.h +++ b/app/src/adb.h @@ -8,31 +8,37 @@ #include "util/intr.h" +#define SC_ADB_NO_STDOUT (1 << 0) +#define SC_ADB_NO_STDERR (1 << 1) + sc_pid -adb_execute(const char *serial, const char *const adb_cmd[], size_t len); +adb_execute(const char *serial, const char *const adb_cmd[], size_t len, + unsigned flags); bool adb_forward(struct sc_intr *intr, const char *serial, uint16_t local_port, - const char *device_socket_name); + const char *device_socket_name, unsigned flags); bool adb_forward_remove(struct sc_intr *intr, const char *serial, - uint16_t local_port); + uint16_t local_port, unsigned flags); bool adb_reverse(struct sc_intr *intr, const char *serial, - const char *device_socket_name, uint16_t local_port); + const char *device_socket_name, uint16_t local_port, + unsigned flags); bool adb_reverse_remove(struct sc_intr *intr, const char *serial, - const char *device_socket_name); + const char *device_socket_name, unsigned flags); bool adb_push(struct sc_intr *intr, const char *serial, const char *local, - const char *remote); + const char *remote, unsigned flags); bool -adb_install(struct sc_intr *intr, const char *serial, const char *local); +adb_install(struct sc_intr *intr, const char *serial, const char *local, + unsigned flags); /** * Execute `adb get-serialno` @@ -40,6 +46,6 @@ adb_install(struct sc_intr *intr, const char *serial, const char *local); * Return the result, to be freed by the caller, or NULL on error. */ char * -adb_get_serialno(struct sc_intr *intr); +adb_get_serialno(struct sc_intr *intr, unsigned flags); #endif diff --git a/app/src/adb_tunnel.c b/app/src/adb_tunnel.c index fa86a8a5aa..0055259721 100644 --- a/app/src/adb_tunnel.c +++ b/app/src/adb_tunnel.c @@ -20,7 +20,8 @@ enable_tunnel_reverse_any_port(struct sc_adb_tunnel *tunnel, struct sc_port_range port_range) { uint16_t port = port_range.first; for (;;) { - if (!adb_reverse(intr, serial, SC_SOCKET_NAME, port)) { + if (!adb_reverse(intr, serial, SC_SOCKET_NAME, port, + SC_ADB_NO_STDOUT)) { // the command itself failed, it will fail on any port return false; } @@ -51,7 +52,8 @@ enable_tunnel_reverse_any_port(struct sc_adb_tunnel *tunnel, } // failure, disable tunnel and try another port - if (!adb_reverse_remove(intr, serial, SC_SOCKET_NAME)) { + if (!adb_reverse_remove(intr, serial, SC_SOCKET_NAME, + SC_ADB_NO_STDOUT)) { LOGW("Could not remove reverse tunnel on port %" PRIu16, port); } @@ -81,7 +83,7 @@ enable_tunnel_forward_any_port(struct sc_adb_tunnel *tunnel, uint16_t port = port_range.first; for (;;) { - if (adb_forward(intr, serial, port, SC_SOCKET_NAME)) { + if (adb_forward(intr, serial, port, SC_SOCKET_NAME, SC_ADB_NO_STDOUT)) { // success tunnel->local_port = port; tunnel->enabled = true; @@ -146,9 +148,11 @@ sc_adb_tunnel_close(struct sc_adb_tunnel *tunnel, struct sc_intr *intr, bool ret; if (tunnel->forward) { - ret = adb_forward_remove(intr, serial, tunnel->local_port); + ret = adb_forward_remove(intr, serial, tunnel->local_port, + SC_ADB_NO_STDOUT); } else { - ret = adb_reverse_remove(intr, serial, SC_SOCKET_NAME); + ret = adb_reverse_remove(intr, serial, SC_SOCKET_NAME, + SC_ADB_NO_STDOUT); assert(tunnel->server_socket != SC_SOCKET_NONE); if (!net_close(tunnel->server_socket)) { diff --git a/app/src/file_handler.c b/app/src/file_handler.c index 12498ccf7a..addbb9a580 100644 --- a/app/src/file_handler.c +++ b/app/src/file_handler.c @@ -128,7 +128,7 @@ run_file_handler(void *data) { if (req.action == ACTION_INSTALL_APK) { LOGI("Installing %s...", req.file); - bool ok = adb_install(intr, serial, req.file); + bool ok = adb_install(intr, serial, req.file, 0); if (ok) { LOGI("%s successfully installed", req.file); } else { @@ -136,7 +136,7 @@ run_file_handler(void *data) { } } else { LOGI("Pushing %s...", req.file); - bool ok = adb_push(intr, serial, req.file, push_target); + bool ok = adb_push(intr, serial, req.file, push_target, 0); if (ok) { LOGI("%s successfully pushed to %s", req.file, push_target); } else { diff --git a/app/src/server.c b/app/src/server.c index f0624a13b9..a62a093a30 100644 --- a/app/src/server.c +++ b/app/src/server.c @@ -112,7 +112,7 @@ push_server(struct sc_intr *intr, const char *serial) { free(server_path); return false; } - bool ok = adb_push(intr, serial, server_path, SC_DEVICE_SERVER_PATH); + bool ok = adb_push(intr, serial, server_path, SC_DEVICE_SERVER_PATH, 0); free(server_path); return ok; } @@ -234,7 +234,8 @@ execute_server(struct sc_server *server, // Port: 5005 // Then click on "Debug" #endif - pid = adb_execute(params->serial, cmd, count); + // Inherit both stdout and stderr (all server logs are printed to stdout) + pid = adb_execute(params->serial, cmd, count, 0); end: for (unsigned i = dyn_idx; i < count; ++i) { @@ -482,7 +483,7 @@ sc_server_fill_serial(struct sc_server *server) { // device/emulator" error) if (!server->params.serial) { // The serial is owned by sc_server_params, and will be freed on destroy - server->params.serial = adb_get_serialno(&server->intr); + server->params.serial = adb_get_serialno(&server->intr, 0); if (!server->params.serial) { LOGE("Could not get device serial"); return false; From e6e6f865a01879365f284fd3c8bfe7758ebf1bb5 Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Thu, 25 Nov 2021 22:05:38 +0100 Subject: [PATCH 03/13] Add adb flag to disable execution error logs In addition to disable stdout and stderr of the child process, add a flag to disable the error log printed by scrcpy if the command failed. This will we useful for commands which are expected to fail in some cases (like "adb disconnect" if the device is not connected). PR #2827 --- app/src/adb.c | 40 ++++++++++++++++++++++++---------------- app/src/adb.h | 1 + 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/app/src/adb.c b/app/src/adb.c index 13018f4dd0..ce6aedbf83 100644 --- a/app/src/adb.c +++ b/app/src/adb.c @@ -112,18 +112,25 @@ show_adb_err_msg(enum sc_process_result err, const char *const argv[]) { } static bool -process_check_success_internal(sc_pid pid, const char *name, bool close) { +process_check_success_internal(sc_pid pid, const char *name, bool close, + unsigned flags) { + bool log_errors = !(flags & SC_ADB_NO_LOGERR); + if (pid == SC_PROCESS_NONE) { - LOGE("Could not execute \"%s\"", name); + if (log_errors) { + LOGE("Could not execute \"%s\"", name); + } return false; } sc_exit_code exit_code = sc_process_wait(pid, close); if (exit_code) { - if (exit_code != SC_EXIT_CODE_NONE) { - LOGE("\"%s\" returned with value %" SC_PRIexitcode, name, - exit_code); - } else { - LOGE("\"%s\" exited unexpectedly", name); + if (log_errors) { + if (exit_code != SC_EXIT_CODE_NONE) { + LOGE("\"%s\" returned with value %" SC_PRIexitcode, name, + exit_code); + } else { + LOGE("\"%s\" exited unexpectedly", name); + } } return false; } @@ -131,14 +138,15 @@ process_check_success_internal(sc_pid pid, const char *name, bool close) { } static bool -process_check_success_intr(struct sc_intr *intr, sc_pid pid, const char *name) { +process_check_success_intr(struct sc_intr *intr, sc_pid pid, const char *name, + unsigned flags) { if (!sc_intr_set_process(intr, pid)) { // Already interrupted return false; } // Always pass close=false, interrupting would be racy otherwise - bool ret = process_check_success_internal(pid, name, false); + bool ret = process_check_success_internal(pid, name, false, flags); sc_intr_set_process(intr, SC_PROCESS_NONE); @@ -217,7 +225,7 @@ adb_forward(struct sc_intr *intr, const char *serial, uint16_t local_port, const char *const adb_cmd[] = {"forward", local, remote}; sc_pid pid = adb_execute(serial, adb_cmd, ARRAY_LEN(adb_cmd), flags); - return process_check_success_intr(intr, pid, "adb forward"); + return process_check_success_intr(intr, pid, "adb forward", flags); } bool @@ -228,7 +236,7 @@ adb_forward_remove(struct sc_intr *intr, const char *serial, const char *const adb_cmd[] = {"forward", "--remove", local}; sc_pid pid = adb_execute(serial, adb_cmd, ARRAY_LEN(adb_cmd), flags); - return process_check_success_intr(intr, pid, "adb forward --remove"); + return process_check_success_intr(intr, pid, "adb forward --remove", flags); } bool @@ -242,7 +250,7 @@ adb_reverse(struct sc_intr *intr, const char *serial, const char *const adb_cmd[] = {"reverse", remote, local}; sc_pid pid = adb_execute(serial, adb_cmd, ARRAY_LEN(adb_cmd), flags); - return process_check_success_intr(intr, pid, "adb reverse"); + return process_check_success_intr(intr, pid, "adb reverse", flags); } bool @@ -253,7 +261,7 @@ adb_reverse_remove(struct sc_intr *intr, const char *serial, const char *const adb_cmd[] = {"reverse", "--remove", remote}; sc_pid pid = adb_execute(serial, adb_cmd, ARRAY_LEN(adb_cmd), flags); - return process_check_success_intr(intr, pid, "adb reverse --remove"); + return process_check_success_intr(intr, pid, "adb reverse --remove", flags); } bool @@ -281,7 +289,7 @@ adb_push(struct sc_intr *intr, const char *serial, const char *local, free((void *) local); #endif - return process_check_success_intr(intr, pid, "adb push"); + return process_check_success_intr(intr, pid, "adb push", flags); } bool @@ -303,7 +311,7 @@ adb_install(struct sc_intr *intr, const char *serial, const char *local, free((void *) local); #endif - return process_check_success_intr(intr, pid, "adb install"); + return process_check_success_intr(intr, pid, "adb install", flags); } char * @@ -321,7 +329,7 @@ adb_get_serialno(struct sc_intr *intr, unsigned flags) { ssize_t r = sc_pipe_read_all_intr(intr, pid, pout, buf, sizeof(buf)); sc_pipe_close(pout); - bool ok = process_check_success_intr(intr, pid, "adb get-serialno"); + bool ok = process_check_success_intr(intr, pid, "adb get-serialno", flags); if (!ok) { return NULL; } diff --git a/app/src/adb.h b/app/src/adb.h index 5033965bf4..7391500dcc 100644 --- a/app/src/adb.h +++ b/app/src/adb.h @@ -10,6 +10,7 @@ #define SC_ADB_NO_STDOUT (1 << 0) #define SC_ADB_NO_STDERR (1 << 1) +#define SC_ADB_NO_LOGERR (1 << 2) sc_pid adb_execute(const char *serial, const char *const adb_cmd[], size_t len, From bfce22414fd3173b9671140d77d2e8a927d5ff94 Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Wed, 24 Nov 2021 22:39:46 +0100 Subject: [PATCH 04/13] Add adb connect and disconnect PR #2827 --- app/src/adb.c | 18 ++++++++++++++++++ app/src/adb.h | 17 +++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/app/src/adb.c b/app/src/adb.c index ce6aedbf83..cb4f14dae4 100644 --- a/app/src/adb.c +++ b/app/src/adb.c @@ -314,6 +314,24 @@ adb_install(struct sc_intr *intr, const char *serial, const char *local, return process_check_success_intr(intr, pid, "adb install", flags); } +bool +adb_connect(struct sc_intr *intr, const char *ip_port, unsigned flags) { + const char *const adb_cmd[] = {"connect", ip_port}; + + sc_pid pid = adb_execute(NULL, adb_cmd, ARRAY_LEN(adb_cmd), flags); + return process_check_success_intr(intr, pid, "adb connect", flags); +} + +bool +adb_disconnect(struct sc_intr *intr, const char *ip_port, unsigned flags) { + const char *const adb_cmd[] = {"disconnect", ip_port}; + size_t len = ip_port ? ARRAY_LEN(adb_cmd) + : ARRAY_LEN(adb_cmd) - 1; + + sc_pid pid = adb_execute(NULL, adb_cmd, len, flags); + return process_check_success_intr(intr, pid, "adb disconnect", flags); +} + char * adb_get_serialno(struct sc_intr *intr, unsigned flags) { const char *const adb_cmd[] = {"get-serialno"}; diff --git a/app/src/adb.h b/app/src/adb.h index 7391500dcc..d5d9bb37f3 100644 --- a/app/src/adb.h +++ b/app/src/adb.h @@ -41,6 +41,23 @@ bool adb_install(struct sc_intr *intr, const char *serial, const char *local, unsigned flags); +/** + * Execute `adb connect ` + * + * `ip_port` may not be NULL. + */ +bool +adb_connect(struct sc_intr *intr, const char *ip_port, unsigned flags); + +/** + * Execute `adb disconnect []` + * + * If `ip_port` is NULL, execute `adb disconnect`. + * Otherwise, execute `adb disconnect `. + */ +bool +adb_disconnect(struct sc_intr *intr, const char *ip_port, unsigned flags); + /** * Execute `adb get-serialno` * From 3bf6fd2894c283a976320b8f0862a63be0d78635 Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Thu, 25 Nov 2021 21:33:50 +0100 Subject: [PATCH 05/13] Workaround "adb connect" error detection "adb connect" always returns successfully (with exit code 0), even in case of failure. As a workaround, capture its output and check if it starts with "connected". PR #2827 --- app/src/adb.c | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/app/src/adb.c b/app/src/adb.c index cb4f14dae4..819066c0e4 100644 --- a/app/src/adb.c +++ b/app/src/adb.c @@ -318,8 +318,37 @@ bool adb_connect(struct sc_intr *intr, const char *ip_port, unsigned flags) { const char *const adb_cmd[] = {"connect", ip_port}; - sc_pid pid = adb_execute(NULL, adb_cmd, ARRAY_LEN(adb_cmd), flags); - return process_check_success_intr(intr, pid, "adb connect", flags); + sc_pipe pout; + sc_pid pid = adb_execute_p(NULL, adb_cmd, ARRAY_LEN(adb_cmd), flags, &pout); + if (pid == SC_PROCESS_NONE) { + LOGE("Could not execute \"adb connect\""); + return false; + } + + // "adb connect" always returns successfully (with exit code 0), even in + // case of failure. As a workaround, check if its output starts with + // "connected". + char buf[128]; + ssize_t r = sc_pipe_read_all_intr(intr, pid, pout, buf, sizeof(buf)); + sc_pipe_close(pout); + + bool ok = process_check_success_intr(intr, pid, "adb connect", flags); + if (!ok) { + return false; + } + + if (r == -1) { + return false; + } + + ok = !strncmp("connected", buf, sizeof("connected") - 1); + if (!ok && !(flags & SC_ADB_NO_STDERR)) { + // "adb connect" also prints errors to stdout. Since we capture it, + // re-print the error to stderr. + sc_str_truncate(buf, r, "\r\n"); + fprintf(stderr, "%s\n", buf); + } + return ok; } bool From b52f87a892ec97beb2afb1e0a711632038ccfe4a Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Thu, 18 Nov 2021 09:32:21 +0100 Subject: [PATCH 06/13] Add util function to locate a column in a string This will help to parse the result of "adb shell ip route" to find the device IP address. PR #2827 --- app/src/util/str.c | 26 ++++++++++++++++++++++++++ app/src/util/str.h | 20 ++++++++++++++++++++ app/tests/test_str.c | 21 +++++++++++++++++++++ 3 files changed, 67 insertions(+) diff --git a/app/src/util/str.c b/app/src/util/str.c index ab1c8783ad..1564ffe2b1 100644 --- a/app/src/util/str.c +++ b/app/src/util/str.c @@ -304,3 +304,29 @@ sc_str_truncate(char *data, size_t len, const char *endchars) { data[idx] = '\0'; return idx; } + +ssize_t +sc_str_index_of_column(const char *s, unsigned col, const char *seps) { + size_t colidx = 0; + + size_t idx = 0; + while (s[idx] != '\0' && colidx != col) { + size_t r = strcspn(&s[idx], seps); + idx += r; + + if (s[idx] == '\0') { + // Not found + return -1; + } + + size_t consecutive_seps = strspn(&s[idx], seps); + assert(consecutive_seps); // At least one + idx += consecutive_seps; + + if (s[idx] != '\0') { + ++colidx; + } + } + + return col == colidx ? (ssize_t) idx : -1; +} diff --git a/app/src/util/str.h b/app/src/util/str.h index 521dfff549..2c7d7829dd 100644 --- a/app/src/util/str.h +++ b/app/src/util/str.h @@ -114,4 +114,24 @@ sc_str_wrap_lines(const char *input, unsigned columns, unsigned indent); size_t sc_str_truncate(char *data, size_t len, const char *endchars); +/** + * Find the start of a column in a string + * + * A string may represent several columns, separated by some "spaces" + * (separators). This function aims to find the start of the column number + * `col`. + * + * For example, to find the 4th column (column number 3): + * + * // here + * // v + * const char *s = "abc def ghi jk"; + * ssize_t index = sc_str_index_of_column(s, 3, " "); + * assert(index == 16); // points to "jk" + * + * Return -1 if no such column exists. + */ +ssize_t +sc_str_index_of_column(const char *s, unsigned col, const char *seps); + #endif diff --git a/app/tests/test_str.c b/app/tests/test_str.c index c66bb2f4c9..bd976b3c87 100644 --- a/app/tests/test_str.c +++ b/app/tests/test_str.c @@ -364,6 +364,26 @@ static void test_truncate(void) { assert(!strcmp("hello", s4)); } +static void test_index_of_column(void) { + assert(sc_str_index_of_column("a bc d", 0, " ") == 0); + assert(sc_str_index_of_column("a bc d", 1, " ") == 2); + assert(sc_str_index_of_column("a bc d", 2, " ") == 6); + assert(sc_str_index_of_column("a bc d", 3, " ") == -1); + + assert(sc_str_index_of_column("a ", 0, " ") == 0); + assert(sc_str_index_of_column("a ", 1, " ") == -1); + + assert(sc_str_index_of_column("", 0, " ") == 0); + assert(sc_str_index_of_column("", 1, " ") == -1); + + assert(sc_str_index_of_column("a \t \t bc \t d\t", 0, " \t") == 0); + assert(sc_str_index_of_column("a \t \t bc \t d\t", 1, " \t") == 8); + assert(sc_str_index_of_column("a \t \t bc \t d\t", 2, " \t") == 15); + assert(sc_str_index_of_column("a \t \t bc \t d\t", 3, " \t") == -1); + + assert(sc_str_index_of_column(" a bc d", 1, " ") == 2); +} + int main(int argc, char *argv[]) { (void) argc; (void) argv; @@ -384,5 +404,6 @@ int main(int argc, char *argv[]) { test_strlist_contains(); test_wrap_lines(); test_truncate(); + test_index_of_column(); return 0; } From b7e631791cc0c9af407c9f625047754f6adf369c Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Sun, 28 Nov 2021 22:02:40 +0100 Subject: [PATCH 07/13] Add util function to remove trailing '\r' Depending on the platform and adb versions, the lines output by adb could end with "\r\r\n". This util function helps to remove all trailing '\r'. PR #2827 --- app/src/util/str.c | 11 +++++++++++ app/src/util/str.h | 11 +++++++++++ app/tests/test_str.c | 15 +++++++++++++++ 3 files changed, 37 insertions(+) diff --git a/app/src/util/str.c b/app/src/util/str.c index 1564ffe2b1..70e3f1debe 100644 --- a/app/src/util/str.c +++ b/app/src/util/str.c @@ -330,3 +330,14 @@ sc_str_index_of_column(const char *s, unsigned col, const char *seps) { return col == colidx ? (ssize_t) idx : -1; } + +size_t +sc_str_remove_trailing_cr(char *s, size_t len) { + while (len) { + if (s[len - 1] != '\r') { + break; + } + s[--len] = '\0'; + } + return len; +} diff --git a/app/src/util/str.h b/app/src/util/str.h index 2c7d7829dd..b81764ef6b 100644 --- a/app/src/util/str.h +++ b/app/src/util/str.h @@ -134,4 +134,15 @@ sc_str_truncate(char *data, size_t len, const char *endchars); ssize_t sc_str_index_of_column(const char *s, unsigned col, const char *seps); +/** + * Remove all `\r` at the end of the line + * + * The line length is provided by `len` (this avoids a call to `strlen()` when + * the caller already knows the length). + * + * Return the new length. + */ +size_t +sc_str_remove_trailing_cr(char *s, size_t len); + #endif diff --git a/app/tests/test_str.c b/app/tests/test_str.c index bd976b3c87..cc3039e714 100644 --- a/app/tests/test_str.c +++ b/app/tests/test_str.c @@ -384,6 +384,20 @@ static void test_index_of_column(void) { assert(sc_str_index_of_column(" a bc d", 1, " ") == 2); } +static void test_remove_trailing_cr() { + char s[] = "abc\r"; + sc_str_remove_trailing_cr(s, sizeof(s) - 1); + assert(!strcmp(s, "abc")); + + char s2[] = "def\r\r\r\r"; + sc_str_remove_trailing_cr(s2, sizeof(s2) - 1); + assert(!strcmp(s2, "def")); + + char s3[] = "adb\rdef\r"; + sc_str_remove_trailing_cr(s3, sizeof(s3) - 1); + assert(!strcmp(s3, "adb\rdef")); +} + int main(int argc, char *argv[]) { (void) argc; (void) argv; @@ -405,5 +419,6 @@ int main(int argc, char *argv[]) { test_wrap_lines(); test_truncate(); test_index_of_column(); + test_remove_trailing_cr(); return 0; } From f609b406c9226144d9a7fdd7cdd12919ceaa2c73 Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Thu, 25 Nov 2021 22:11:59 +0100 Subject: [PATCH 08/13] Add function to find the device IP address Parse the result of "adb shell ip route" to find the device IP address. PR #2827 --- app/meson.build | 9 +++- app/src/adb.c | 38 +++++++++++++++++ app/src/adb.h | 9 ++++ app/src/adb_parser.c | 65 +++++++++++++++++++++++++++++ app/src/adb_parser.h | 14 +++++++ app/tests/test_adb_parser.c | 83 +++++++++++++++++++++++++++++++++++++ 6 files changed, 217 insertions(+), 1 deletion(-) create mode 100644 app/src/adb_parser.c create mode 100644 app/src/adb_parser.h create mode 100644 app/tests/test_adb_parser.c diff --git a/app/meson.build b/app/meson.build index 1baf34fc50..720d9c8c09 100644 --- a/app/meson.build +++ b/app/meson.build @@ -1,6 +1,7 @@ src = [ 'src/main.c', 'src/adb.c', + 'src/adb_parser.c', 'src/adb_tunnel.c', 'src/cli.c', 'src/clock.c', @@ -204,8 +205,14 @@ install_data('../data/icon.png', # do not build tests in release (assertions would not be executed at all) if get_option('buildtype') == 'debug' tests = [ + ['test_adb_parser', [ + 'tests/test_adb_parser.c', + 'src/adb_parser.c', + 'src/util/str.c', + 'src/util/strbuf.c', + ]], ['test_buffer_util', [ - 'tests/test_buffer_util.c' + 'tests/test_buffer_util.c', ]], ['test_cbuf', [ 'tests/test_cbuf.c', diff --git a/app/src/adb.c b/app/src/adb.c index 819066c0e4..69bf551a8f 100644 --- a/app/src/adb.c +++ b/app/src/adb.c @@ -5,6 +5,7 @@ #include #include +#include "adb_parser.h" #include "util/file.h" #include "util/log.h" #include "util/process_intr.h" @@ -389,3 +390,40 @@ adb_get_serialno(struct sc_intr *intr, unsigned flags) { return strdup(buf); } + +char * +adb_get_device_ip(struct sc_intr *intr, const char *serial, unsigned flags) { + const char *const cmd[] = {"shell", "ip", "route"}; + + sc_pipe pout; + sc_pid pid = adb_execute_p(serial, cmd, ARRAY_LEN(cmd), flags, &pout); + if (pid == SC_PROCESS_NONE) { + LOGD("Could not execute \"ip route\""); + return NULL; + } + + // "adb shell ip route" output should contain only a few lines + char buf[1024]; + ssize_t r = sc_pipe_read_all_intr(intr, pid, pout, buf, sizeof(buf)); + sc_pipe_close(pout); + + bool ok = process_check_success_intr(intr, pid, "ip route", flags); + if (!ok) { + return NULL; + } + + if (r == -1) { + return false; + } + + assert((size_t) r <= sizeof(buf)); + if (r == sizeof(buf) && buf[sizeof(buf) - 1] != '\0') { + // The implementation assumes that the output of "ip route" fits in the + // buffer in a single pass + LOGW("Result of \"ip route\" does not fit in 1Kb. " + "Please report an issue.\n"); + return NULL; + } + + return sc_adb_parse_device_ip_from_output(buf, r); +} diff --git a/app/src/adb.h b/app/src/adb.h index d5d9bb37f3..ba0c2bde80 100644 --- a/app/src/adb.h +++ b/app/src/adb.h @@ -66,4 +66,13 @@ adb_disconnect(struct sc_intr *intr, const char *ip_port, unsigned flags); char * adb_get_serialno(struct sc_intr *intr, unsigned flags); +/** + * Attempt to retrieve the device IP + * + * Return the IP as a string of the form "xxx.xxx.xxx.xxx", to be freed by the + * caller, or NULL on error. + */ +char * +adb_get_device_ip(struct sc_intr *intr, const char *serial, unsigned flags); + #endif diff --git a/app/src/adb_parser.c b/app/src/adb_parser.c new file mode 100644 index 0000000000..5ac21ede6f --- /dev/null +++ b/app/src/adb_parser.c @@ -0,0 +1,65 @@ +#include "adb_parser.h" + +#include +#include + +#include "util/log.h" +#include "util/str.h" + +static char * +sc_adb_parse_device_ip_from_line(char *line, size_t len) { + // One line from "ip route" looks lile: + // "192.168.1.0/24 dev wlan0 proto kernel scope link src 192.168.1.x" + + // Get the location of the device name (index of "wlan0" in the example) + ssize_t idx_dev_name = sc_str_index_of_column(line, 2, " "); + if (idx_dev_name == -1) { + return NULL; + } + + // Get the location of the ip address (column 8, but column 6 if we start + // from column 2). Must be computed before truncating individual columns. + ssize_t idx_ip = sc_str_index_of_column(&line[idx_dev_name], 6, " "); + if (idx_ip == -1) { + return NULL; + } + // idx_ip is searched from &line[idx_dev_name] + idx_ip += idx_dev_name; + + char *dev_name = &line[idx_dev_name]; + sc_str_truncate(dev_name, len - idx_dev_name + 1, " \t"); + + char *ip = &line[idx_ip]; + sc_str_truncate(ip, len - idx_ip + 1, " \t"); + + // Only consider lines where the device name starts with "wlan" + if (strncmp(dev_name, "wlan", sizeof("wlan") - 1)) { + LOGD("Device ip lookup: ignoring %s (%s)", ip, dev_name); + return NULL; + } + + return strdup(ip); +} + +char * +sc_adb_parse_device_ip_from_output(char *buf, size_t buf_len) { + size_t idx_line = 0; + while (idx_line < buf_len && buf[idx_line] != '\0') { + char *line = &buf[idx_line]; + size_t len = sc_str_truncate(line, buf_len - idx_line, "\n"); + + // The same, but without any trailing '\r' + size_t line_len = sc_str_remove_trailing_cr(line, len); + + char *ip = sc_adb_parse_device_ip_from_line(line, line_len); + if (ip) { + // Found + return ip; + } + + // The next line starts after the '\n' (replaced by `\0`) + idx_line += len + 1; + } + + return NULL; +} diff --git a/app/src/adb_parser.h b/app/src/adb_parser.h new file mode 100644 index 0000000000..79f266311c --- /dev/null +++ b/app/src/adb_parser.h @@ -0,0 +1,14 @@ +#ifndef SC_ADB_PARSER_H +#define SC_ADB_PARSER_H + +#include "common.h" + +#include "stddef.h" + +/** + * Parse the ip from the output of `adb shell ip route` + */ +char * +sc_adb_parse_device_ip_from_output(char *buf, size_t buf_len); + +#endif diff --git a/app/tests/test_adb_parser.c b/app/tests/test_adb_parser.c new file mode 100644 index 0000000000..fbc65649f3 --- /dev/null +++ b/app/tests/test_adb_parser.c @@ -0,0 +1,83 @@ +#include "common.h" + +#include + +#include "adb_parser.h" + +static void test_get_ip_single_line() { + char ip_route[] = "192.168.1.0/24 dev wlan0 proto kernel scope link src " + "192.168.12.34\r\r\n"; + + char *ip = sc_adb_parse_device_ip_from_output(ip_route, sizeof(ip_route)); + assert(ip); + assert(!strcmp(ip, "192.168.12.34")); +} + +static void test_get_ip_single_line_without_eol() { + char ip_route[] = "192.168.1.0/24 dev wlan0 proto kernel scope link src " + "192.168.12.34"; + + char *ip = sc_adb_parse_device_ip_from_output(ip_route, sizeof(ip_route)); + assert(ip); + assert(!strcmp(ip, "192.168.12.34")); +} + +static void test_get_ip_single_line_with_trailing_space() { + char ip_route[] = "192.168.1.0/24 dev wlan0 proto kernel scope link src " + "192.168.12.34 \n"; + + char *ip = sc_adb_parse_device_ip_from_output(ip_route, sizeof(ip_route)); + assert(ip); + assert(!strcmp(ip, "192.168.12.34")); +} + +static void test_get_ip_multiline_first_ok() { + char ip_route[] = "192.168.1.0/24 dev wlan0 proto kernel scope link src " + "192.168.1.2\r\n" + "10.0.0.0/24 dev rmnet proto kernel scope link src " + "10.0.0.2\r\n"; + + char *ip = sc_adb_parse_device_ip_from_output(ip_route, sizeof(ip_route)); + assert(ip); + assert(!strcmp(ip, "192.168.1.2")); +} + +static void test_get_ip_multiline_second_ok() { + char ip_route[] = "10.0.0.0/24 dev rmnet proto kernel scope link src " + "10.0.0.3\r\n" + "192.168.1.0/24 dev wlan0 proto kernel scope link src " + "192.168.1.3\r\n"; + + char *ip = sc_adb_parse_device_ip_from_output(ip_route, sizeof(ip_route)); + assert(ip); + assert(!strcmp(ip, "192.168.1.3")); +} + +static void test_get_ip_no_wlan() { + char ip_route[] = "192.168.1.0/24 dev rmnet proto kernel scope link src " + "192.168.12.34\r\r\n"; + + char *ip = sc_adb_parse_device_ip_from_output(ip_route, sizeof(ip_route)); + assert(!ip); +} + +static void test_get_ip_truncated() { + char ip_route[] = "192.168.1.0/24 dev rmnet proto kernel scope link src " + "\n"; + + char *ip = sc_adb_parse_device_ip_from_output(ip_route, sizeof(ip_route)); + assert(!ip); +} + +int main(int argc, char *argv[]) { + (void) argc; + (void) argv; + + test_get_ip_single_line(); + test_get_ip_single_line_without_eol(); + test_get_ip_single_line_with_trailing_space(); + test_get_ip_multiline_first_ok(); + test_get_ip_multiline_second_ok(); + test_get_ip_no_wlan(); + test_get_ip_truncated(); +} From 8543d842ea4d73d6912c38f9702f7a18651b3d49 Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Thu, 25 Nov 2021 22:13:39 +0100 Subject: [PATCH 09/13] Add function to switch device to TCP/IP mode Expose a function to execute "adb tcpip ". PR #2827 --- app/src/adb.c | 11 +++++++++++ app/src/adb.h | 7 +++++++ 2 files changed, 18 insertions(+) diff --git a/app/src/adb.c b/app/src/adb.c index 69bf551a8f..747e5ac91c 100644 --- a/app/src/adb.c +++ b/app/src/adb.c @@ -315,6 +315,17 @@ adb_install(struct sc_intr *intr, const char *serial, const char *local, return process_check_success_intr(intr, pid, "adb install", flags); } +bool +adb_tcpip(struct sc_intr *intr, const char *serial, uint16_t port, + unsigned flags) { + char port_string[5 + 1]; + sprintf(port_string, "%" PRIu16, port); + const char *const adb_cmd[] = {"tcpip", port_string}; + + sc_pid pid = adb_execute(serial, adb_cmd, ARRAY_LEN(adb_cmd), flags); + return process_check_success_intr(intr, pid, "adb tcpip", flags); +} + bool adb_connect(struct sc_intr *intr, const char *ip_port, unsigned flags) { const char *const adb_cmd[] = {"connect", ip_port}; diff --git a/app/src/adb.h b/app/src/adb.h index ba0c2bde80..f56c98c49d 100644 --- a/app/src/adb.h +++ b/app/src/adb.h @@ -41,6 +41,13 @@ bool adb_install(struct sc_intr *intr, const char *serial, const char *local, unsigned flags); +/** + * Execute `adb tcpip ` + */ +bool +adb_tcpip(struct sc_intr *intr, const char *serial, uint16_t port, + unsigned flags); + /** * Execute `adb connect ` * From 800ba33ff42be775a279e02deef4b655ec3e991f Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Thu, 25 Nov 2021 22:18:36 +0100 Subject: [PATCH 10/13] Add function to read an adb property This will allow to read the property "service.adb.tcp.port" to know if the TCP/IP mode is enabled on the device, and which listening port is used. PR #2827 --- app/src/adb.c | 31 +++++++++++++++++++++++++++++++ app/src/adb.h | 7 +++++++ 2 files changed, 38 insertions(+) diff --git a/app/src/adb.c b/app/src/adb.c index 747e5ac91c..598b331f48 100644 --- a/app/src/adb.c +++ b/app/src/adb.c @@ -373,6 +373,37 @@ adb_disconnect(struct sc_intr *intr, const char *ip_port, unsigned flags) { return process_check_success_intr(intr, pid, "adb disconnect", flags); } +char * +adb_getprop(struct sc_intr *intr, const char *serial, const char *prop, + unsigned flags) { + const char *const adb_cmd[] = {"shell", "getprop", prop}; + + sc_pipe pout; + sc_pid pid = + adb_execute_p(serial, adb_cmd, ARRAY_LEN(adb_cmd), flags, &pout); + if (pid == SC_PROCESS_NONE) { + LOGE("Could not execute \"adb getprop\""); + return NULL; + } + + char buf[128]; + ssize_t r = sc_pipe_read_all_intr(intr, pid, pout, buf, sizeof(buf)); + sc_pipe_close(pout); + + bool ok = process_check_success_intr(intr, pid, "adb getprop", flags); + if (!ok) { + return NULL; + } + + if (r == -1) { + return NULL; + } + + sc_str_truncate(buf, r, " \r\n"); + + return strdup(buf); +} + char * adb_get_serialno(struct sc_intr *intr, unsigned flags) { const char *const adb_cmd[] = {"get-serialno"}; diff --git a/app/src/adb.h b/app/src/adb.h index f56c98c49d..8d7f3ea104 100644 --- a/app/src/adb.h +++ b/app/src/adb.h @@ -65,6 +65,13 @@ adb_connect(struct sc_intr *intr, const char *ip_port, unsigned flags); bool adb_disconnect(struct sc_intr *intr, const char *ip_port, unsigned flags); +/** + * Execute `adb getprop ` + */ +char * +adb_getprop(struct sc_intr *intr, const char *serial, const char *prop, + unsigned flags); + /** * Execute `adb get-serialno` * From 3b310f8317e4f62ee5aa76786aa75e3d610d34e0 Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Wed, 24 Nov 2021 22:57:36 +0100 Subject: [PATCH 11/13] Extract interruptible sleep for server This improves readability, and makes the function reusable. PR #2827 --- app/src/server.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/app/src/server.c b/app/src/server.c index a62a093a30..6211b08b42 100644 --- a/app/src/server.c +++ b/app/src/server.c @@ -136,6 +136,20 @@ log_level_to_server_string(enum sc_log_level level) { } } +static bool +sc_server_sleep(struct sc_server *server, sc_tick deadline) { + sc_mutex_lock(&server->mutex); + bool timed_out = false; + while (!server->stopped && !timed_out) { + timed_out = !sc_cond_timedwait(&server->cond_stopped, + &server->mutex, deadline); + } + bool stopped = server->stopped; + sc_mutex_unlock(&server->mutex); + + return !stopped; +} + static sc_pid execute_server(struct sc_server *server, const struct sc_server_params *params) { @@ -286,17 +300,9 @@ connect_to_server(struct sc_server *server, unsigned attempts, sc_tick delay, } if (attempts) { - sc_mutex_lock(&server->mutex); sc_tick deadline = sc_tick_now() + delay; - bool timed_out = false; - while (!server->stopped && !timed_out) { - timed_out = !sc_cond_timedwait(&server->cond_stopped, - &server->mutex, deadline); - } - bool stopped = server->stopped; - sc_mutex_unlock(&server->mutex); - - if (stopped) { + bool ok = sc_server_sleep(server, deadline); + if (!ok) { LOGI("Connection attempt stopped"); break; } From 19858e6aeb99b57de28f042053b8c3d4372e56ca Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Thu, 25 Nov 2021 22:22:49 +0100 Subject: [PATCH 12/13] Add --tcpip feature Expose an option to automatically configure and reconnect the device over TCP/IP, to simplify wireless connection without using adb explicitly. There are two variants: - If a destination address is provided, then scrcpy connects to this address before starting. The device must listen on the given TCP port (default is 5555). - If no destination address is provided, then scrcpy attempts to find the IP address of the current device (typically connected over USB), enables TCP/IP mode, then connects to this address before starting. PR #2827 --- app/scrcpy.1 | 8 ++ app/src/adb.h | 2 + app/src/cli.c | 27 ++++++ app/src/options.c | 2 + app/src/options.h | 2 + app/src/scrcpy.c | 2 + app/src/server.c | 203 +++++++++++++++++++++++++++++++++++++++++++++- app/src/server.h | 2 + 8 files changed, 244 insertions(+), 4 deletions(-) diff --git a/app/scrcpy.1 b/app/scrcpy.1 index 122af75713..715000b66a 100644 --- a/app/scrcpy.1 +++ b/app/scrcpy.1 @@ -199,6 +199,14 @@ For example, to use either LCtrl+LAlt or LSuper for scrcpy shortcuts, pass "lctr Default is "lalt,lsuper" (left-Alt or left-Super). +.TP +.BI "\-\-tcpip[=ip[:port]] +Configure and reconnect the device over TCP/IP. + +If a destination address is provided, then scrcpy connects to this address before starting. The device must listen on the given TCP port (default is 5555). + +If no destination address is provided, then scrcpy attempts to find the IP address of the current device (typically connected over USB), enables TCP/IP mode, then connects to this address before starting. + .TP .B \-S, \-\-turn\-screen\-off Turn the device screen off immediately. diff --git a/app/src/adb.h b/app/src/adb.h index 8d7f3ea104..4d1278cf82 100644 --- a/app/src/adb.h +++ b/app/src/adb.h @@ -12,6 +12,8 @@ #define SC_ADB_NO_STDERR (1 << 1) #define SC_ADB_NO_LOGERR (1 << 2) +#define SC_ADB_SILENT (SC_ADB_NO_STDOUT | SC_ADB_NO_STDERR | SC_ADB_NO_LOGERR) + sc_pid adb_execute(const char *serial, const char *const adb_cmd[], size_t len, unsigned flags); diff --git a/app/src/cli.c b/app/src/cli.c index 791d3c43be..8cfaf57b3b 100644 --- a/app/src/cli.c +++ b/app/src/cli.c @@ -50,6 +50,7 @@ #define OPT_TUNNEL_HOST 1030 #define OPT_TUNNEL_PORT 1031 #define OPT_NO_CLIPBOARD_AUTOSYNC 1032 +#define OPT_TCPIP 1033 struct sc_option { char shortopt; @@ -404,6 +405,20 @@ static const struct sc_option options[] = { .text = "Keep the device on while scrcpy is running, when the device " "is plugged in.", }, + { + .longopt_id = OPT_TCPIP, + .longopt = "tcpip", + .argdesc = "ip[:port]", + .optional_arg = true, + .text = "Configure and reconnect the device over TCP/IP.\n" + "If a destination address is provided, then scrcpy connects to " + "this address before starting. The device must listen on the " + "given TCP port (default is 5555).\n" + "If no destination address is provided, then scrcpy attempts " + "to find the IP address of the current device (typically " + "connected over USB), enables TCP/IP mode, then connects to " + "this address before starting.", + }, { .longopt_id = OPT_WINDOW_BORDERLESS, .longopt = "window-borderless", @@ -1378,6 +1393,10 @@ parse_args_with_getopt(struct scrcpy_cli_args *args, int argc, char *argv[], case OPT_NO_CLIPBOARD_AUTOSYNC: opts->clipboard_autosync = false; break; + case OPT_TCPIP: + opts->tcpip = true; + opts->tcpip_dst = optarg; + break; #ifdef HAVE_V4L2 case OPT_V4L2_SINK: opts->v4l2_device = optarg; @@ -1400,6 +1419,14 @@ parse_args_with_getopt(struct scrcpy_cli_args *args, int argc, char *argv[], return false; } + // If a TCP/IP address is provided, then tcpip must be enabled + assert(opts->tcpip || !opts->tcpip_dst); + + if (opts->serial && opts->tcpip_dst) { + LOGE("Incompatible options: -s/--serial and --tcpip with an argument"); + return false; + } + #ifdef HAVE_V4L2 if (!opts->display && !opts->record_filename && !opts->v4l2_device) { LOGE("-N/--no-display requires either screen recording (-r/--record)" diff --git a/app/src/options.c b/app/src/options.c index a99b09da9d..a14bda9afc 100644 --- a/app/src/options.c +++ b/app/src/options.c @@ -54,4 +54,6 @@ const struct scrcpy_options scrcpy_options_default = { .legacy_paste = false, .power_off_on_close = false, .clipboard_autosync = true, + .tcpip = false, + .tcpip_dst = NULL, }; diff --git a/app/src/options.h b/app/src/options.h index d10b2e8a2a..f183bd739d 100644 --- a/app/src/options.h +++ b/app/src/options.h @@ -109,6 +109,8 @@ struct scrcpy_options { bool legacy_paste; bool power_off_on_close; bool clipboard_autosync; + bool tcpip; + const char *tcpip_dst; }; extern const struct scrcpy_options scrcpy_options_default; diff --git a/app/src/scrcpy.c b/app/src/scrcpy.c index f0142b4692..99317ffce8 100644 --- a/app/src/scrcpy.c +++ b/app/src/scrcpy.c @@ -365,6 +365,8 @@ scrcpy(struct scrcpy_options *options) { .force_adb_forward = options->force_adb_forward, .power_off_on_close = options->power_off_on_close, .clipboard_autosync = options->clipboard_autosync, + .tcpip = options->tcpip, + .tcpip_dst = options->tcpip_dst, }; static const struct sc_server_callbacks cbs = { diff --git a/app/src/server.c b/app/src/server.c index 6211b08b42..06cb7b7268 100644 --- a/app/src/server.c +++ b/app/src/server.c @@ -69,6 +69,7 @@ sc_server_params_destroy(struct sc_server_params *params) { free((char *) params->crop); free((char *) params->codec_options); free((char *) params->encoder_name); + free((char *) params->tcpip_dst); } static bool @@ -92,6 +93,7 @@ sc_server_params_copy(struct sc_server_params *dst, COPY(crop); COPY(codec_options); COPY(encoder_name); + COPY(tcpip_dst); #undef COPY return true; @@ -494,22 +496,215 @@ sc_server_fill_serial(struct sc_server *server) { LOGE("Could not get device serial"); return false; } + + LOGD("Device serial: %s", server->params.serial); + } + + return true; +} + +static bool +is_tcpip_mode_enabled(struct sc_server *server) { + struct sc_intr *intr = &server->intr; + const char *serial = server->params.serial; + + char *current_port = + adb_getprop(intr, serial, "service.adb.tcp.port", SC_ADB_SILENT); + if (!current_port) { + return false; + } + + // Is the device is listening on TCP on port 5555? + bool enabled = !strcmp("5555", current_port); + free(current_port); + return enabled; +} + +static bool +wait_tcpip_mode_enabled(struct sc_server *server, unsigned attempts, + sc_tick delay) { + if (is_tcpip_mode_enabled(server)) { + LOGI("TCP/IP mode enabled"); + return true; + } + + // Only print this log if TCP/IP is not enabled + LOGI("Waiting for TCP/IP mode enabled..."); + + do { + sc_tick deadline = sc_tick_now() + delay; + if (!sc_server_sleep(server, deadline)) { + LOGI("TCP/IP mode waiting interrupted"); + return false; + } + + if (is_tcpip_mode_enabled(server)) { + LOGI("TCP/IP mode enabled"); + return true; + } + } while (--attempts); + return false; +} + +char * +append_port_5555(const char *ip) { + size_t len = strlen(ip); + + // sizeof counts the final '\0' + char *ip_port = malloc(len + sizeof(":5555")); + if (!ip_port) { + LOG_OOM(); + return NULL; + } + + memcpy(ip_port, ip, len); + memcpy(ip_port + len, ":5555", sizeof(":5555")); + + return ip_port; +} + +static bool +sc_server_switch_to_tcpip(struct sc_server *server, char **out_ip_port) { + const char *serial = server->params.serial; + assert(serial); + + struct sc_intr *intr = &server->intr; + + char *ip = adb_get_device_ip(intr, serial, 0); + if (!ip) { + LOGE("Device IP not found"); + return false; + } + + char *ip_port = append_port_5555(ip); + free(ip); + if (!ip_port) { + return false; + } + + bool tcp_mode = is_tcpip_mode_enabled(server); + + if (!tcp_mode) { + bool ok = adb_tcpip(intr, serial, 5555, SC_ADB_NO_STDOUT); + if (!ok) { + LOGE("Could not restart adbd in TCP/IP mode"); + goto error; + } + + unsigned attempts = 40; + sc_tick delay = SC_TICK_FROM_MS(250); + ok = wait_tcpip_mode_enabled(server, attempts, delay); + if (!ok) { + goto error; + } + } + + *out_ip_port = ip_port; + + return true; + +error: + free(ip_port); + return false; +} + +static bool +sc_server_connect_to_tcpip(struct sc_server *server, const char *ip_port) { + struct sc_intr *intr = &server->intr; + + // Error expected if not connected, do not report any error + adb_disconnect(intr, ip_port, SC_ADB_SILENT); + + bool ok = adb_connect(intr, ip_port, 0); + if (!ok) { + LOGE("Could not connect to %s", ip_port); + return false; } + // Override the serial, owned by the sc_server_params + free((void *) server->params.serial); + server->params.serial = strdup(ip_port); + if (!server->params.serial) { + LOG_OOM(); + return false; + } + + LOGI("Connected to %s", ip_port); return true; } + +static bool +sc_server_configure_tcpip(struct sc_server *server) { + char *ip_port; + + const struct sc_server_params *params = &server->params; + + // If tcpip parameter is given, then it must connect to this address. + // Therefore, the device is unknown, so serial is meaningless at this point. + assert(!params->serial || !params->tcpip_dst); + + if (params->tcpip_dst) { + // Append ":5555" if no port is present + bool contains_port = strchr(params->tcpip_dst, ':'); + ip_port = contains_port ? strdup(params->tcpip_dst) + : append_port_5555(params->tcpip_dst); + if (!ip_port) { + LOG_OOM(); + return false; + } + } else { + // The device IP address must be retrieved from the current + // connected device + if (!sc_server_fill_serial(server)) { + return false; + } + + // The serial is either the real serial when connected via USB, or + // the IP:PORT when connected over TCP/IP. Only the latter contains + // a colon. + bool is_already_tcpip = strchr(params->serial, ':'); + if (is_already_tcpip) { + // Nothing to do + LOGI("Device already connected via TCP/IP: %s", params->serial); + return true; + } + + bool ok = sc_server_switch_to_tcpip(server, &ip_port); + if (!ok) { + return false; + } + } + + // On success, this call changes params->serial + bool ok = sc_server_connect_to_tcpip(server, ip_port); + free(ip_port); + return ok; +} + static int run_server(void *data) { struct sc_server *server = data; - if (!sc_server_fill_serial(server)) { - goto error_connection_failed; + const struct sc_server_params *params = &server->params; + + if (params->serial) { + LOGD("Device serial: %s", params->serial); } - const struct sc_server_params *params = &server->params; + if (params->tcpip) { + // params->serial may be changed after this call + bool ok = sc_server_configure_tcpip(server); + if (!ok) { + goto error_connection_failed; + } + } - LOGD("Device serial: %s", params->serial); + // It is ok to call this function even if the device serial has been + // changed by switching over TCP/IP + if (!sc_server_fill_serial(server)) { + goto error_connection_failed; + } bool ok = push_server(&server->intr, params->serial); if (!ok) { diff --git a/app/src/server.h b/app/src/server.h index 5b25ff46ac..8ea20dc7a0 100644 --- a/app/src/server.h +++ b/app/src/server.h @@ -42,6 +42,8 @@ struct sc_server_params { bool force_adb_forward; bool power_off_on_close; bool clipboard_autosync; + bool tcpip; + const char *tcpip_dst; }; struct sc_server { From 1a6caeb18c06ddd3a1116a0e82d65be15b2f48b4 Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Thu, 25 Nov 2021 22:42:34 +0100 Subject: [PATCH 13/13] Document --tcpip in README PR #2827 --- README.md | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index fa4aff6869..ef9fcc9842 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ [Read in another language](#translations) This application provides display and control of Android devices connected via -USB (or [over TCP/IP](#wireless)). It does not require any _root_ access. +USB (or [over TCP/IP](#tcpip-wireless)). It does not require any _root_ access. It works on _GNU/Linux_, _Windows_ and _macOS_. ![screenshot](assets/screenshot-debian-600.jpg) @@ -356,10 +356,38 @@ scrcpy --v4l2-buffer=500 # add 500 ms buffering for v4l2 sink ### Connection -#### Wireless +#### TCP/IP (wireless) _Scrcpy_ uses `adb` to communicate with the device, and `adb` can [connect] to a -device over TCP/IP: +device over TCP/IP. + +##### Automatic + +An option `--tcpip` allows to configure the connection automatically. There are +two variants. + +If the device (accessible at 192.168.1.1 in this example) already listens on a +port (typically 5555) for incoming adb connections, then run: + +```bash +scrcpy --tcpip=192.168.1.1 # default port is 5555 +scrcpy --tcpip=192.168.1.1:5555 +``` + +If the device TCP/IP mode is disabled (or if you don't know the IP address), +connect the device over USB, then run: + +```bash +scrcpy --tcpip # without arguments +``` + +It will automatically find the device IP address, enable TCP/IP mode, then +connect to the device before starting. + +##### Manual + +Alternatively, it is possible to enable the TCP/IP connection manually using +`adb`: 1. Connect the device to the same Wi-Fi as your computer. 2. Get your device IP address, in Settings → About phone → Status, or by