Remove sc_str_quote()

It is no longer used.

PR #6663 <https://github.com/Genymobile/scrcpy/pull/6663>

app/meson.build

@@ -75,6 +75,7 @@ conf.set('_GNU_SOURCE', true)
 if host_machine.system() == 'windows'
     windows = import('windows')
     src += [
+        'src/util/command.c',
         'src/sys/win/file.c',
         'src/sys/win/process.c',
         windows.compile_resources('scrcpy-windows.rc'),
@@ -238,6 +239,12 @@ if get_option('buildtype') == 'debug'
             'src/util/strbuf.c',
             'src/util/term.c',
         ]],
+        ['test_command_windows', [
+          'tests/test_command_windows.c',
+          'src/util/command.c',
+          'src/util/str.c',
+          'src/util/strbuf.c',
+        ]],
         ['test_control_msg_serialize', [
             'tests/test_control_msg_serialize.c',
             'src/control_msg.c',

app/src/adb/adb.c

@@ -331,56 +331,24 @@ sc_adb_reverse_remove(struct sc_intr *intr, const char *serial,
 bool
 sc_adb_push(struct sc_intr *intr, const char *serial, const char *local,
             const char *remote, unsigned flags) {
-#ifdef _WIN32
-    // Windows will parse the string, so the paths must be quoted
-    // (see sys/win/command.c)
-    local = sc_str_quote(local);
-    if (!local) {
-        return SC_PROCESS_NONE;
-    }
-    remote = sc_str_quote(remote);
-    if (!remote) {
-        free((void *) local);
-        return SC_PROCESS_NONE;
-    }
-#endif
-
     assert(serial);
     const char *const argv[] =
         SC_ADB_COMMAND("-s", serial, "push", local, remote);
 
     sc_pid pid = sc_adb_execute(argv, flags);
 
-#ifdef _WIN32
-    free((void *) remote);
-    free((void *) local);
-#endif
-
     return process_check_success_intr(intr, pid, "adb push", flags);
 }
 
 bool
 sc_adb_install(struct sc_intr *intr, const char *serial, const char *local,
                unsigned flags) {
-#ifdef _WIN32
-    // Windows will parse the string, so the local name must be quoted
-    // (see sys/win/command.c)
-    local = sc_str_quote(local);
-    if (!local) {
-        return SC_PROCESS_NONE;
-    }
-#endif
-
     assert(serial);
     const char *const argv[] =
         SC_ADB_COMMAND("-s", serial, "install", "-r", local);
 
     sc_pid pid = sc_adb_execute(argv, flags);
 
-#ifdef _WIN32
-    free((void *) local);
-#endif
-
     return process_check_success_intr(intr, pid, "adb install", flags);
 }
 

app/src/scrcpy.c

@@ -199,8 +199,8 @@ event_loop(struct scrcpy *s, bool has_screen) {
                 break;
             }
             default:
-                if (has_screen && !sc_screen_handle_event(&s->screen, &event)) {
-                    return SCRCPY_EXIT_FAILURE;
+                if (has_screen) {
+                    sc_screen_handle_event(&s->screen, &event);
                 }
                 break;
         }

app/src/screen.c

@@ -462,28 +462,26 @@ sc_screen_init(struct sc_screen *screen,
         if (!SDL_SetWindowIcon(screen->window, icon)) {
             LOGW("Could not set window icon: %s", SDL_GetError());
         }
-    } else if (params->video) {
-        // just a warning
-        LOGW("Could not load icon");
-    } else {
-        // without video, the icon is used as window content, it must be present
-        LOGE("Could not load icon");
-        goto error_destroy_texture;
-    }
 
-    if (!params->video) {
-        assert(icon);
-        screen->content_size.width = icon->w;
-        screen->content_size.height = icon->h;
-        ok = sc_texture_set_from_surface(&screen->tex, icon);
-        if (!ok) {
-            scrcpy_icon_destroy(icon);
-            goto error_destroy_texture;
+        if (!params->video) {
+            screen->content_size.width = icon->w;
+            screen->content_size.height = icon->h;
+            ok = sc_texture_set_from_surface(&screen->tex, icon);
+            if (!ok) {
+                LOGE("Could not set icon: %s", SDL_GetError());
+            }
         }
-    }
 
-    if (icon) {
         scrcpy_icon_destroy(icon);
+    } else {
+        // not fatal
+        LOGE("Could not load icon");
+
+        if (!params->video) {
+            // Make sure the content size is initialized
+            screen->content_size.width = 256;
+            screen->content_size.height = 256;
+        }
     }
 
     screen->frame = av_frame_alloc();
@@ -854,7 +852,7 @@ sc_screen_resize_to_pixel_perfect(struct sc_screen *screen) {
                                             content_size.height);
 }
 
-bool
+void
 sc_screen_handle_event(struct sc_screen *screen, const SDL_Event *event) {
     // !video implies !has_video_window
     assert(screen->video || !screen->has_video_window);
@@ -864,28 +862,28 @@ sc_screen_handle_event(struct sc_screen *screen, const SDL_Event *event) {
             if (!ok) {
                 LOGE("Frame update failed\n");
             }
-            return true;
+            return;
         }
         case SDL_EVENT_WINDOW_EXPOSED:
             if (!screen->video || screen->has_video_window) {
                 sc_screen_render(screen, true);
             }
-            return true;
+            return;
         case SDL_EVENT_WINDOW_PIXEL_SIZE_CHANGED:
             if (screen->has_video_window) {
                 sc_screen_render(screen, true);
             }
-            return true;
+            return;
         case SDL_EVENT_WINDOW_RESTORED:
             if (screen->has_video_window && is_windowed(screen)) {
                 apply_pending_resize(screen);
                 sc_screen_render(screen, true);
             }
-            return true;
+            return;
         case SDL_EVENT_WINDOW_ENTER_FULLSCREEN:
             LOGD("Switched to fullscreen mode");
             assert(screen->has_video_window);
-            return true;
+            return;
         case SDL_EVENT_WINDOW_LEAVE_FULLSCREEN:
             LOGD("Switched to windowed mode");
             assert(screen->has_video_window);
@@ -893,17 +891,16 @@ sc_screen_handle_event(struct sc_screen *screen, const SDL_Event *event) {
                 apply_pending_resize(screen);
                 sc_screen_render(screen, true);
             }
-            return true;
+            return;
     }
 
     if (sc_screen_is_relative_mode(screen)
             && sc_mouse_capture_handle_event(&screen->mc, event)) {
         // The mouse capture handler consumed the event
-        return true;
+        return;
     }
 
     sc_input_manager_handle_event(&screen->im, event);
-    return true;
 }
 
 struct sc_point

app/src/screen.h

@@ -157,8 +157,7 @@ void
 sc_screen_set_paused(struct sc_screen *screen, bool paused);
 
 // react to SDL events
-// If this function returns false, scrcpy must exit with an error.
-bool
+void
 sc_screen_handle_event(struct sc_screen *screen, const SDL_Event *event);
 
 // convert point from window coordinates to frame coordinates

app/src/sys/win/process.c

@@ -3,26 +3,14 @@
 #include <processthreadsapi.h>
 
 #include <assert.h>
+#include <stdlib.h>
 
+#include "util/command.h"
 #include "util/log.h"
 #include "util/str.h"
 
 #define CMD_MAX_LEN 8192
 
-static bool
-build_cmd(char *cmd, size_t len, const char *const argv[]) {
-    // Windows command-line parsing is WTF:
-    // <http://daviddeley.com/autohotkey/parameters/parameters.htm#WINPASS>
-    // only make it work for this very specific program
-    // (don't handle escaping nor quotes)
-    size_t ret = sc_str_join(cmd, argv, ' ', len);
-    if (ret >= len) {
-        LOGE("Command too long (%" SC_PRIsizet " chars)", len - 1);
-        return false;
-    }
-    return true;
-}
-
 enum sc_process_result
 sc_process_execute_p(const char *const argv[], HANDLE *handle, unsigned flags,
                      HANDLE *pin, HANDLE *pout, HANDLE *perr) {
@@ -137,8 +125,9 @@ sc_process_execute_p(const char *const argv[], HANDLE *handle, unsigned flags,
         si.lpAttributeList = lpAttributeList;
     }
 
-    char *cmd = malloc(CMD_MAX_LEN);
-    if (!cmd || !build_cmd(cmd, CMD_MAX_LEN, argv)) {
+    assert(argv && *argv);
+    char *cmd = sc_command_serialize_windows(argv);
+    if (!cmd) {
         LOG_OOM();
         goto error_free_attribute_list;
     }

app/src/util/command.c

@@ -0,0 +1,98 @@
+#include "command.h"
+
+#include <stdlib.h>
+
+#include "util/strbuf.h"
+
+char *
+sc_command_serialize_windows(const char *const argv[]) {
+    // <https://learn.microsoft.com/en-us/cpp/c-language/parsing-c-command-line-arguments>
+    // <https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw>
+
+    struct sc_strbuf buf;
+    bool ok = sc_strbuf_init(&buf, 1024);
+    if (!ok) {
+        return NULL;
+    }
+
+#define BUF_PUSH(C) \
+    do { \
+        if (!sc_strbuf_append_char(&buf, C)) { \
+            goto end; \
+        } \
+    } while (0)
+
+    while (*argv) {
+        const char *arg = *argv;
+
+        BUF_PUSH('"');
+
+        // <https://learn.microsoft.com/en-us/cpp/c-language/parsing-c-command-line-arguments>
+        //
+        // """
+        // If an even number of backslashes is followed by a double quote mark,
+        // then one backslash (\) is placed in the argv array for every pair of
+        // backslashes (\\), and the double quote mark (") is interpreted as a
+        // string delimiter.
+        //
+        // If an odd number of backslashes is followed by a double quote mark,
+        // then one backslash (\) is placed in the argv array for every pair of
+        // backslashes (\\). The double quote mark is interpreted as an escape
+        // sequence by the remaining backslash, causing a literal double quote
+        // mark (") to be placed in argv.
+        // """
+        //
+        // To produce correct escaping according to what the parser will do, we
+        // must count the number of successive backslashes.
+        unsigned backslashes = 0;
+
+        for (const char *c = arg; *c; c++) {
+            switch (*c) {
+                case '"':
+                    while (backslashes) {
+                        // Double all backslashes before a quote
+                        BUF_PUSH('\\');
+                        BUF_PUSH('\\');
+                        --backslashes;
+                    }
+                    BUF_PUSH('\\');
+                    BUF_PUSH('"');
+                    backslashes = 0;
+                    break;
+                case '\\':
+                    ++backslashes;
+                    break;
+                default:
+                    while (backslashes) {
+                        // Put all backslashes as literals
+                        BUF_PUSH('\\');
+                        --backslashes;
+                    }
+                    BUF_PUSH(*c);
+                    break;
+            }
+        }
+
+        while (backslashes) {
+            // Double all backslashes before a quote
+            BUF_PUSH('\\');
+            BUF_PUSH('\\');
+            --backslashes;
+        }
+
+        BUF_PUSH('"');
+
+        ++argv;
+
+        // Argument separator
+        if (*argv) {
+            BUF_PUSH(' ');
+        }
+    }
+
+    return buf.s;
+
+end:
+    free(buf.s);
+    return NULL;
+}

app/src/util/command.h

@@ -0,0 +1,17 @@
+#ifndef SC_COMMAND_H
+#define SC_COMMAND_H
+
+#include "common.h"
+
+/**
+ * Serialize an argv array for Windows
+ *
+ * Convert a NULL-terminated argument array into a single escaped string
+ * suitable for massing to CreateProcess() on Windows.
+ *
+ * The returned value must be freed by the caller.
+ */
+char *
+sc_command_serialize_windows(const char *const argv[]);
+
+#endif

app/src/util/log.c

@@ -151,6 +151,10 @@ sc_sdl_log_print(void *userdata, int category, SDL_LogPriority priority,
 
 void
 sc_log_configure(void) {
+#ifdef _WIN32
+    SetConsoleOutputCP(CP_UTF8);
+#endif
+
     SDL_SetLogOutputFunction(sc_sdl_log_print, NULL);
     // Redirect FFmpeg logs to SDL logs
     av_log_set_callback(sc_av_log_callback);

app/src/util/str.c

@@ -49,21 +49,6 @@ truncated:
     return n;
 }
 
-char *
-sc_str_quote(const char *src) {
-    size_t len = strlen(src);
-    char *quoted = malloc(len + 3);
-    if (!quoted) {
-        LOG_OOM();
-        return NULL;
-    }
-    memcpy(&quoted[1], src, len);
-    quoted[0] = '"';
-    quoted[len + 1] = '"';
-    quoted[len + 2] = '\0';
-    return quoted;
-}
-
 char *
 sc_str_concat(const char *start, const char *end) {
     assert(start);

app/src/util/str.h

@@ -32,14 +32,6 @@ sc_strncpy(char *dest, const char *src, size_t n);
 size_t
 sc_str_join(char *dst, const char *const tokens[], char sep, size_t n);
 
-/**
- * Quote a string
- *
- * Return a new allocated string, surrounded with quotes (`"`).
- */
-char *
-sc_str_quote(const char *src);
-
 /**
  * Concat two strings
  *

app/tests/test_command_windows.c

@@ -0,0 +1,61 @@
+#include "common.h"
+
+#include <assert.h>
+#include <stdlib.h>
+
+#include "util/command.h"
+
+static void test_command_with_spaces(void) {
+    const char *const argv[] = {
+        "C:\\Program Files\\scrcpy\\adb",
+        "-s",
+        "serial with spaces",
+        "push",
+        "E:\\some folder\\scrcpy-server",
+        "/data/local/tmp/scrcpy-server.jar",
+        NULL,
+    };
+    char *cmd = sc_command_serialize_windows(argv);
+    const char *expected = "\"C:\\Program Files\\scrcpy\\adb\" "
+                           "\"-s\" "
+                           "\"serial with spaces\" "
+                           "\"push\" "
+                           "\"E:\\some folder\\scrcpy-server\" "
+                           "\"/data/local/tmp/scrcpy-server.jar\"";
+
+    assert(!strcmp(expected, cmd));
+    free(cmd);
+}
+
+static void test_command_with_backslashes(void) {
+    const char *const argv[] = {
+        "a\\\\ b\\",
+        "def \\",
+        "gh\"i\" \\\\",
+        "jkl\\\\",
+        "mno\\",
+        "p\\\"qr",
+        NULL,
+    };
+
+    char *cmd = sc_command_serialize_windows(argv);
+    const char *expected = "\"a\\\\ b\\\\\" "
+                           "\"def \\\\\" "
+                           "\"gh\\\"i\\\" \\\\\\\\\" "
+                           "\"jkl\\\\\\\\\" "
+                           "\"mno\\\\\" "
+                           "\"p\\\\\\\"qr\"";
+
+    assert(!strcmp(expected, cmd));
+    free(cmd);
+}
+
+int main(int argc, char *argv[]) {
+    (void) argc;
+    (void) argv;
+
+    test_command_with_spaces();
+    test_command_with_backslashes();
+
+    return 0;
+}

app/tests/test_str.c

@@ -131,16 +131,6 @@ static void test_join_truncated_after_sep(void) {
     assert(!strcmp("abc de ", s));
 }
 
-static void test_quote(void) {
-    const char *s = "abcde";
-    char *out = sc_str_quote(s);
-
-    // add '"' at the beginning and the end
-    assert(!strcmp("\"abcde\"", out));
-
-    free(out);
-}
-
 static void test_concat(void) {
     const char *s = "2024:11";
     char *out = sc_str_concat("my-prefix:", s);
@@ -398,7 +388,6 @@ int main(int argc, char *argv[]) {
     test_join_truncated_in_token();
     test_join_truncated_before_sep();
     test_join_truncated_after_sep();
-    test_quote();
     test_concat();
     test_utf8_truncate();
     test_parse_integer();