Log server pushed

Now that "adb push" stdout is disabled, add a log to notify server pushed.
Use inherit flags for adb commands
2021-11-20 00:02:30 +01:00 · 2021-11-20 00:02:30 +01:00 · 2021-11-20 00:02:30 +01:00
6 changed files with 79 additions and 76 deletions
--- a/app/src/adb_tunnel.c
+++ b/app/src/adb_tunnel.c
@@ -20,8 +20,7 @@ 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,
-                         SC_INHERIT_STDERR)) {
+        if (!adb_reverse(intr, serial, SC_SOCKET_NAME, port, SC_STDERR)) {
            // the command itself failed, it will fail on any port
            return false;
        }
@@ -52,8 +51,7 @@ 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,
-                                SC_INHERIT_STDERR)) {
+        if (!adb_reverse_remove(intr, serial, SC_SOCKET_NAME, SC_STDERR)) {
            LOGW("Could not remove reverse tunnel on port %" PRIu16, port);
        }

@@ -83,8 +81,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,
-                        SC_INHERIT_STDERR)) {
+        if (adb_forward(intr, serial, port, SC_SOCKET_NAME, SC_STDERR)) {
            // success
            tunnel->local_port = port;
            tunnel->enabled = true;
@@ -149,11 +146,9 @@ 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,
-                                 SC_INHERIT_STDERR);
+        ret = adb_forward_remove(intr, serial, tunnel->local_port, SC_STDERR);
    } else {
-        ret = adb_reverse_remove(intr, serial, SC_SOCKET_NAME,
-                                 SC_INHERIT_STDERR);
+        ret = adb_reverse_remove(intr, serial, SC_SOCKET_NAME, SC_STDERR);

        assert(tunnel->server_socket != SC_SOCKET_NONE);
        if (!net_close(tunnel->server_socket)) {
--- a/app/src/file_handler.c
+++ b/app/src/file_handler.c
@@ -128,8 +128,8 @@ run_file_handler(void *data) {

        if (req.action == ACTION_INSTALL_APK) {
            LOGI("Installing %s...", req.file);
-            bool ok = adb_install(intr, serial, req.file,
-                                  SC_INHERIT_STDOUT | SC_INHERIT_STDERR);
+            bool ok =
+                adb_install(intr, serial, req.file, SC_STDOUT | SC_STDERR);
            if (ok) {
                LOGI("%s successfully installed", req.file);
            } else {
@@ -138,7 +138,7 @@ run_file_handler(void *data) {
        } else {
            LOGI("Pushing %s...", req.file);
            bool ok = adb_push(intr, serial, req.file, push_target,
-                               SC_INHERIT_STDOUT | SC_INHERIT_STDERR);
+                               SC_STDOUT | SC_STDERR);
            if (ok) {
                LOGI("%s successfully pushed to %s", req.file, push_target);
            } else {
--- a/app/src/server.c
+++ b/app/src/server.c
@@ -113,7 +113,7 @@ push_server(struct sc_intr *intr, const char *serial) {
        return false;
    }
    bool ok = adb_push(intr, serial, server_path, SC_DEVICE_SERVER_PATH,
-                       SC_INHERIT_STDERR);
+                       SC_STDERR);
    free(server_path);
    return ok;
 }
@@ -200,8 +200,7 @@ execute_server(struct sc_server *server,
    // Then click on "Debug"
 #endif
    // Inherit both stdout and stderr (all server logs are printed to stdout)
-    return adb_execute(serial, cmd, ARRAY_LEN(cmd),
-                       SC_INHERIT_STDOUT | SC_INHERIT_STDERR);
+    return adb_execute(serial, cmd, ARRAY_LEN(cmd), SC_STDOUT | SC_STDERR);
 }

 static bool
@@ -433,8 +432,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,
-                                                 SC_INHERIT_STDERR);
+        server->params.serial = adb_get_serialno(&server->intr, SC_STDERR);
        if (!server->params.serial) {
            LOGE("Could not get device serial");
            return false;
--- a/app/src/sys/unix/process.c
+++ b/app/src/sys/unix/process.c
@@ -13,12 +13,12 @@
 enum sc_process_result
 sc_process_execute_p(const char *const argv[], sc_pid *pid, unsigned inherit,
                     int *pin, int *pout, int *perr) {
-    bool inherit_stdout = inherit & SC_INHERIT_STDOUT;
-    bool inherit_stderr = inherit & SC_INHERIT_STDERR;
+    bool inherit_stdout = inherit & SC_STDOUT;
+    bool inherit_stderr = inherit & SC_STDERR;

-    // If pout is defined, then inherit MUST NOT contain SC_INHERIT_STDOUT.
+    // If pout is defined, then inherit MUST NOT contain SC_STDOUT.
    assert(!pout || !inherit_stdout);
-    // If perr is defined, then inherit MUST NOT contain SC_INHERIT_STDERR.
+    // If perr is defined, then inherit MUST NOT contain SC_STDERR.
    assert(!perr || !inherit_stderr);

    int in[2];
--- a/app/src/sys/win/process.c
+++ b/app/src/sys/win/process.c
@@ -29,14 +29,19 @@ 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, unsigned inherit,
                     HANDLE *pin, HANDLE *pout, HANDLE *perr) {
-    bool inherit_stdout = inherit & SC_INHERIT_STDOUT;
-    bool inherit_stderr = inherit & SC_INHERIT_STDERR;
+    bool inherit_stdout = inherit & SC_STDOUT;
+    bool inherit_stderr = inherit & SC_STDERR;

-    // If pout is defined, then inherit MUST NOT contain SC_INHERIT_STDOUT.
+    // If pout is defined, then inherit MUST NOT contain SC_STDOUT.
    assert(!pout || !inherit_stdout);
-    // If perr is defined, then inherit MUST NOT contain SC_INHERIT_STDERR.
+    // If perr is defined, then inherit MUST NOT contain SC_STDERR.
    assert(!perr || !inherit_stderr);

+    // Add 1 per non-NULL pointer
+    unsigned handle_count = !!pin
+                          + (pout || inherit_stdout)
+                          + (perr || inherit_stderr);
+
    enum sc_process_result ret = SC_PROCESS_ERROR_GENERIC;

    SECURITY_ATTRIBUTES sa;
@@ -85,52 +90,57 @@ sc_process_execute_p(const char *const argv[], HANDLE *handle, unsigned inherit,
    HANDLE handles[3];

    LPPROC_THREAD_ATTRIBUTE_LIST lpAttributeList = NULL;
+    // Must be set even if handle_count == 0, so that stdin, stdout and stderr
+    // are NOT inherited in that case
    si.StartupInfo.dwFlags = STARTF_USESTDHANDLES;

-    unsigned handle_count = 0;
-    if (pin) {
-        si.StartupInfo.hStdInput = stdin_read_handle;
-        handles[handle_count++] = si.StartupInfo.hStdInput;
-    }
-    if (pout || inherit_stdout) {
-        si.StartupInfo.hStdOutput = pout ? stdout_write_handle
-                                         : GetStdHandle(STD_OUTPUT_HANDLE);
-        handles[handle_count++] = si.StartupInfo.hStdOutput;
-    }
-    if (perr || inherit_stderr) {
-        si.StartupInfo.hStdError = perr ? stderr_write_handle
-                                        : GetStdHandle(STD_ERROR_HANDLE);
-        handles[handle_count++] = si.StartupInfo.hStdError;
-    }
+    if (handle_count) {
+        unsigned i = 0;
+        if (pin) {
+            si.StartupInfo.hStdInput = stdin_read_handle;
+            handles[i++] = si.StartupInfo.hStdInput;
+        }
+        if (pout || inherit_stdout) {
+            si.StartupInfo.hStdOutput = pout ? stdout_write_handle
+                                             : GetStdHandle(STD_OUTPUT_HANDLE);
+            handles[i++] = si.StartupInfo.hStdOutput;
+        }
+        if (perr || inherit_stderr) {
+            si.StartupInfo.hStdError = perr ? stderr_write_handle
+                                            : GetStdHandle(STD_ERROR_HANDLE);
+            handles[i++] = si.StartupInfo.hStdError;
+        }

-    SIZE_T size;
-    // Call it once to know the required buffer size
-    BOOL ok =
-        InitializeProcThreadAttributeList(NULL, 1, 0, &size)
-            || GetLastError() == ERROR_INSUFFICIENT_BUFFER;
-    if (!ok) {
-        goto error_close_stderr;
-    }
+        SIZE_T size;
+        // Call it once to know the required buffer size
+        BOOL ok =
+            InitializeProcThreadAttributeList(NULL, 1, 0, &size)
+                || GetLastError() == ERROR_INSUFFICIENT_BUFFER;
+        if (!ok) {
+            goto error_close_stderr;
+        }

-    lpAttributeList = malloc(size);
-    if (!lpAttributeList) {
-        goto error_close_stderr;
-    }
+        lpAttributeList = malloc(size);
+        if (!lpAttributeList) {
+            goto error_close_stderr;
+        }

-    ok = InitializeProcThreadAttributeList(lpAttributeList, 1, 0, &size);
-    if (!ok) {
-        free(lpAttributeList);
-        goto error_close_stderr;
-    }
+        ok = InitializeProcThreadAttributeList(lpAttributeList, 1, 0, &size);
+        if (!ok) {
+            free(lpAttributeList);
+            goto error_close_stderr;
+        }

-    ok = UpdateProcThreadAttribute(lpAttributeList, 0,
-                                   PROC_THREAD_ATTRIBUTE_HANDLE_LIST, handles,
-                                   handle_count * sizeof(HANDLE), NULL, NULL);
-    if (!ok) {
-        goto error_free_attribute_list;
-    }
+        ok = UpdateProcThreadAttribute(lpAttributeList, 0,
+                                       PROC_THREAD_ATTRIBUTE_HANDLE_LIST,
+                                       handles, handle_count * sizeof(HANDLE),
+                                       NULL, NULL);
+        if (!ok) {
+            goto error_free_attribute_list;
+        }

-    si.lpAttributeList = lpAttributeList;
+        si.lpAttributeList = lpAttributeList;
+    }

    char *cmd = malloc(CMD_MAX_LEN);
    if (!cmd || !build_cmd(cmd, CMD_MAX_LEN, argv)) {
@@ -144,9 +154,10 @@ sc_process_execute_p(const char *const argv[], HANDLE *handle, unsigned inherit,
        goto error_free_attribute_list;
    }

-    ok = CreateProcessW(NULL, wide, NULL, NULL, TRUE,
-                        EXTENDED_STARTUPINFO_PRESENT, NULL, NULL,
-                        &si.StartupInfo, &pi);
+    BOOL bInheritHandles = handle_count > 0;
+    DWORD dwCreationFlags = handle_count > 0 ? EXTENDED_STARTUPINFO_PRESENT : 0;
+    BOOL ok = CreateProcessW(NULL, wide, NULL, NULL, bInheritHandles,
+                             dwCreationFlags, NULL, NULL, &si.StartupInfo, &pi);
    free(wide);
    if (!ok) {
        if (GetLastError() == ERROR_FILE_NOT_FOUND) {
--- a/app/src/util/process.h
+++ b/app/src/util/process.h
@@ -67,16 +67,15 @@ enum sc_process_result {
    SC_PROCESS_ERROR_MISSING_BINARY,
 };

-#define SC_INHERIT_NONE    0
-#define SC_INHERIT_STDOUT (1 << 0)
-#define SC_INHERIT_STDERR (1 << 1)
+#define SC_STDOUT (1 << 0)
+#define SC_STDERR (1 << 1)

 /**
 * Execute the command and write the process id to `pid`
 *
- * The parameter `inherit` is a ORed value of SC_INHERIT_* flags. It indicates
- * if stdout and stderr must be inherited from the scrcpy process (in other
- * words, if the process must output to the scrcpy console).
+ * The parameter `inherit` is a OR of any of SC_STDOUT and SC_STDERR. It
+ * indicates if stdout and stderr must be inherited from the scrcpy process (in
+ * other words, if the process must output to the scrcpy console).
 */
 enum sc_process_result
 sc_process_execute(const char *const argv[], sc_pid *pid, unsigned inherit);
@@ -88,8 +87,8 @@ sc_process_execute(const char *const argv[], sc_pid *pid, unsigned inherit);
 * (`perr`).
 *
 * The parameter `inherit` has the same semantics as in `sc_process_execute()`.
- * If `pout` is not NULL, then `inherit` MUST NOT contain SC_INHERIT_STDOUT.
- * If `perr` is not NULL, then `inherit` MUST NOT contain SC_INHERIT_STDERR.
+ * If `pout` is not NULL, then `inherit` MUST NOT contain SC_STDOUT.
+ * If `perr` is not NULL, then `inherit` MUST NOT contain SC_STDERR.
 */
 enum sc_process_result
 sc_process_execute_p(const char *const argv[], sc_pid *pid, unsigned inherit,