[Spice-devel] [spice v1 3/3] reds: drop sscanf() in favour of g_strsplit()

[Victor Toso]

From: Victor Toso <me at victortoso.com>

In case of errors in sscanf(), we were returning (codecs + n) being n
an initialized integer.

While working on extending the codecs string, this seems a bit risky
although I did not find any real example that could trigger that path.

Signed-off-by: Victor Toso <victortoso at redhat.com>
---
 server/reds.c | 63 +++++++++++++++++++++++++++--------------------------------
 1 file changed, 29 insertions(+), 34 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index e061e4d..eaa18d9 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -3512,34 +3512,15 @@ static const int video_codec_caps[] = {
 };
 
 
-/* Expected string:  encoder:codec;encoder:codec */
-static const char* parse_video_codecs(const char *codecs, char **encoder,
-                                      char **codec)
-{
-    if (!codecs) {
-        return NULL;
-    }
-    while (*codecs == ';') {
-        codecs++;
-    }
-    if (!*codecs) {
-        return NULL;
-    }
-    int n;
-    *encoder = *codec = NULL;
-    if (sscanf(codecs, "%m[0-9a-zA-Z_]:%m[0-9a-zA-Z_]%n", encoder, codec, &n) != 2) {
-        while (*codecs != '\0' && *codecs != ';') {
-            codecs++;
-        }
-        return codecs;
-    }
-    return codecs + n;
-}
-
+/*
+ * @codecs should be described in a semi-colon separated list of encoder:codec.
+ * e.g: "gstreamer:vp8;spice:mjpeg"
+ */
 static void reds_set_video_codecs_from_string(RedsState *reds, const char *codecs)
 {
-    char *encoder_name, *codec_name;
     GArray *video_codecs;
+    gchar **preferences;
+    gint i;
 
     g_return_if_fail(codecs != NULL);
 
@@ -3548,13 +3529,29 @@ static void reds_set_video_codecs_from_string(RedsState *reds, const char *codec
     }
 
     video_codecs = g_array_new(FALSE, FALSE, sizeof(RedVideoCodec));
-    const char *c = codecs;
-    while ( (c = parse_video_codecs(c, &encoder_name, &codec_name)) ) {
+    preferences = g_strsplit(codecs, ";", -1);
+    for (i = 0; preferences[i] != NULL; i++) {
+        const gchar *encoder_name, *codec_name;
         uint32_t encoder_index, codec_index;
-        if (!encoder_name || !codec_name) {
-            spice_warning("spice: invalid encoder:codec value at %s", codecs);
+        gchar **pref;
+
+        /* Don't bother with empty strings that would be an extra ';' somewhere */
+        if (g_str_equal(preferences[i], ""))
+            continue;
+
+        pref = g_strsplit(preferences[i], ":", 2);
 
-        } else if (!get_name_index(video_encoder_names, encoder_name, &encoder_index)){
+        if (pref[0] == NULL || g_str_equal(pref[0], "") ||
+                pref[1] == NULL || g_str_equal(pref[1], "")) {
+            spice_warning("spice: invalid encoder:codec value at '%s'", preferences[i]);
+            g_strfreev(pref);
+            continue;
+        }
+
+        encoder_name = pref[0];
+        codec_name = pref[1];
+
+        if (!get_name_index(video_encoder_names, encoder_name, &encoder_index)){
             spice_warning("spice: unknown video encoder %s", encoder_name);
 
         } else if (!get_name_index(video_codec_names, codec_name, &codec_index)) {
@@ -3570,11 +3567,9 @@ static void reds_set_video_codecs_from_string(RedsState *reds, const char *codec
             new_codec.cap = video_codec_caps[codec_index];
             g_array_append_val(video_codecs, new_codec);
         }
-
-        free(encoder_name);
-        free(codec_name);
-        codecs = c;
+        g_strfreev(pref);
     }
+    g_strfreev(preferences);
 
     if (video_codecs->len == 0) {
         spice_warning("Failed to set video codecs, input string: '%s'", codecs);
-- 
2.9.3