From a06ddd182d94f759d9b40fb1684b4b6bb6748a21 Mon Sep 17 00:00:00 2001 From: Nirbheek Chauhan Date: Thu, 27 Feb 2020 11:39:08 +0530 Subject: [PATCH] dash: Don't use sscanf + glib format modifiers We do not have a way to know the format modifiers to use with string functions provided by the system. `G_GUINT64_FORMAT` and other string modifiers only work for glib string formatting functions. We cannot use them for string functions provided by the stdlib. See: https://developer.gnome.org/glib/stable/glib-Basic-Types.html#glib-Basic-Types.description F.ex. ``` ../ext/dash/gstxmlhelper.c: In function 'gst_xml_helper_get_prop_unsigned_integer_64': ../ext/dash/gstxmlhelper.c:473:40: error: unknown conversion type character 'l' in format [-Werror=format=] if (sscanf ((gchar *) prop_string, "%" G_GUINT64_FORMAT, ^~~ In file included from /builds/nirbheek/cerbero/cerbero-build/dist/windows_x86/include/glib-2.0/glib/gtypes.h:32, from /builds/nirbheek/cerbero/cerbero-build/dist/windows_x86/include/glib-2.0/glib/galloca.h:32, from /builds/nirbheek/cerbero/cerbero-build/dist/windows_x86/include/glib-2.0/glib.h:30, from /builds/nirbheek/cerbero/cerbero-build/dist/windows_x86/include/gstreamer-1.0/gst/gst.h:27, from ../ext/dash/gstxmlhelper.h:26, from ../ext/dash/gstxmlhelper.c:22: /builds/nirbheek/cerbero/cerbero-build/dist/windows_x86/lib/glib-2.0/include/glibconfig.h:69:28: note: format string is defined here #define G_GUINT64_FORMAT "llu" ^ ../ext/dash/gstxmlhelper.c:473:40: error: too many arguments for format [-Werror=format-extra-args] if (sscanf ((gchar *) prop_string, "%" G_GUINT64_FORMAT, ^~~ ``` In the process, we're also following the DASH MPD spec more closely now, which specifies that ranges must follow RFC 2616 section 14.35.1: https://tools.ietf.org/html/rfc2616#page-138 --- ext/dash/gstxmlhelper.c | 74 +++++++++++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 25 deletions(-) diff --git a/ext/dash/gstxmlhelper.c b/ext/dash/gstxmlhelper.c index 97adc6ac9a..966b243ba1 100644 --- a/ext/dash/gstxmlhelper.c +++ b/ext/dash/gstxmlhelper.c @@ -460,6 +460,32 @@ gst_xml_helper_get_prop_unsigned_integer (xmlNode * a_node, return exists; } +/* g_ascii_string_to_unsigned is available since 2.54. Get rid of this wrapper + * when we bump the version in 1.18 */ +#if !GLIB_CHECK_VERSION(2,54,0) +#define g_ascii_string_to_unsigned gst_xml_helper_ascii_string_to_unsigned +static gboolean +gst_xml_helper_ascii_string_to_unsigned (const gchar * str, guint base, + guint64 min, guint64 max, guint64 * out_num, GError ** error) +{ + guint64 number; + gchar *endptr = NULL; + + number = g_ascii_strtoull (str, &endptr, base); + + /* Be as strict as the implementation of g_ascii_string_to_unsigned in glib */ + if (errno) + return FALSE; + if (g_ascii_isspace (str[0]) || str[0] == '-' || str[0] == '+') + return FALSE; + if (*endptr != '\0' || endptr == NULL) + return FALSE; + + *out_num = number; + return TRUE; +} +#endif + gboolean gst_xml_helper_get_prop_unsigned_integer_64 (xmlNode * a_node, const gchar * property_name, guint64 default_val, guint64 * property_value) @@ -470,17 +496,14 @@ gst_xml_helper_get_prop_unsigned_integer_64 (xmlNode * a_node, *property_value = default_val; prop_string = xmlGetProp (a_node, (const xmlChar *) property_name); if (prop_string) { - if (sscanf ((gchar *) prop_string, "%" G_GUINT64_FORMAT, - property_value) == 1 && - strstr ((gchar *) prop_string, "-") == NULL) { + if (g_ascii_string_to_unsigned ((gchar *) prop_string, 10, 0, G_MAXUINT64, + property_value, NULL)) { exists = TRUE; GST_LOG (" - %s: %" G_GUINT64_FORMAT, property_name, *property_value); } else { GST_WARNING ("failed to parse unsigned integer property %s from xml string %s", property_name, prop_string); - /* sscanf might have written to *property_value. Restore to default */ - *property_value = default_val; } xmlFree (prop_string); } @@ -605,35 +628,36 @@ gst_xml_helper_get_prop_range (xmlNode * a_node, str = (gchar *) prop_string; GST_TRACE ("range: %s, len %d", str, len); - /* read "-" */ + /* find "-" */ pos = strcspn (str, "-"); if (pos >= len) { GST_TRACE ("pos %d >= len %d", pos, len); goto error; } + if (pos == 0) { + GST_TRACE ("pos == 0, but first_byte_pos is not optional"); + goto error; + } + /* read first_byte_pos */ - if (pos != 0) { - /* replace str[pos] with '\0' to allow sscanf to not be confused by - * the minus sign (eg " -1" (observe the space before -) would otherwise - * be interpreted as range -1 to 1) - */ - str[pos] = 0; - if (sscanf (str, "%" G_GUINT64_FORMAT, &first_byte_pos) != 1 || - strstr (str, "-") != NULL) { - /* sscanf failed or it found a negative number */ - /* restore the '-' sign */ - str[pos] = '-'; - goto error; - } + + /* replace str[pos] with '\0' since we only want to read the + * first_byte_pos, and g_ascii_string_to_unsigned requires the entire + * string to be a single number, which is exactly what we want */ + str[pos] = '\0'; + if (!g_ascii_string_to_unsigned (str, 10, 0, G_MAXUINT64, &first_byte_pos, + NULL)) { /* restore the '-' sign */ str[pos] = '-'; + goto error; } - /* read last_byte_pos */ - if (pos < (len - 1)) { - if (sscanf (str + pos + 1, "%" G_GUINT64_FORMAT, &last_byte_pos) != 1 || - strstr (str + pos + 1, "-") != NULL) { - goto error; - } + /* restore the '-' sign */ + str[pos] = '-'; + + /* read last_byte_pos, which is optional */ + if (pos < (len - 1) && !g_ascii_string_to_unsigned (str + pos + 1, 10, 0, + G_MAXUINT64, &last_byte_pos, NULL)) { + goto error; } /* malloc return data structure */ *property_value = g_slice_new0 (GstXMLRange);