From 14035010dd760d2202d03eba3ca392a488ff04eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Fri, 4 Oct 2019 13:52:39 +0100 Subject: [PATCH 1/2] glib: ensure consistent abort-on-OOM with g_vasprintf & its callers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The g_vasprintf method is called by g_strdup_vprintf, g_strdup_printf, g_string_append_vprintf and more. It has three different implementations depending on what the build target platform supports: 1. The gnulib impl appears to use the system malloc, but a '#define malloc g_malloc' causes it to use GLib's wrapper and thus abort on OOM. This mostly gets used on Windows platforms or UNIX platforms with broken printf formatting. 2. The main impl mostly used on modern Linux/UNIX calls the system vasprintf which uses the system malloc and does not abort on OOM. 3. The final impl used on remaining platforms calls system vsprintf on a buffer allocated by g_new, and thus always aborts on OOM. Of note is that impl 2 (using vasprintf) historically could abort on OOM, if the application had installed a non-system malloc impl with GLib. This was because the code would g_strndup the result from vasprintf() in that scenario. This was removed in: commit a3660532535f92cfac136435579ed4f23231f48c Author: Dan Winship Date: Fri Aug 7 09:46:49 2015 -0400 glib: remove deprecated g_mem_is_system_malloc() check in gprintf.c Having inconsistent OOM behaviour for the three impls is undesirable and aborting on OOM is normal pratice for GLib APIs. Thus we must thus ensure this happens in all impls of g_vasprintf. Fixes #1622 Signed-off-by: Daniel P. Berrangé --- glib/gprintf.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/glib/gprintf.c b/glib/gprintf.c index fc0a02a3b..d4d0b3e0c 100644 --- a/glib/gprintf.c +++ b/glib/gprintf.c @@ -20,6 +20,7 @@ #include #include #include +#include #include "gprintf.h" #include "gprintfint.h" @@ -327,9 +328,18 @@ g_vasprintf (gchar **string, #elif defined (HAVE_VASPRINTF) - len = vasprintf (string, format, args); - if (len < 0) - *string = NULL; + { + int saved_errno; + len = vasprintf (string, format, args); + saved_errno = errno; + if (len < 0) + { + if (saved_errno == ENOMEM) + g_error ("%s: failed to allocate memory", G_STRLOC); + else + *string = NULL; + } + } #else From 109be1e90d60ac58bd66a3a1992872537b12dafa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Fri, 4 Oct 2019 15:04:01 +0100 Subject: [PATCH 2/2] glib: add parameter annotations for g_vasprintf and callers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Document that g_vasprintf and g_strdup_printf are guaranteed to return a non-NULL string, unless the format string contains the locale sensitive conversions %lc or %ls. Further annotate that the output parameter for g_vasprintf and the format string for all functions must be non-NULL. Fixes #1622 Signed-off-by: Daniel P. Berrangé --- glib/gprintf.c | 8 ++++++-- glib/gstrfuncs.c | 12 ++++++++++-- glib/gstring.c | 4 ++-- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/glib/gprintf.c b/glib/gprintf.c index d4d0b3e0c..6be85c70f 100644 --- a/glib/gprintf.c +++ b/glib/gprintf.c @@ -295,8 +295,8 @@ g_vsnprintf (gchar *string, /** * g_vasprintf: - * @string: the return location for the newly-allocated string. - * @format: a standard printf() format string, but notice + * @string: (not optional) (nullable): the return location for the newly-allocated string. + * @format: (not nullable): a standard printf() format string, but notice * [string precision pitfalls][string-precision] * @args: the list of arguments to insert in the output. * @@ -306,6 +306,10 @@ g_vsnprintf (gchar *string, * string to hold the output, instead of putting the output in a buffer * you allocate in advance. * + * The returned value in @string is guaranteed to be non-NULL, unless + * @format contains `%lc` or `%ls` conversions, which can fail if no + * multibyte representation is available for the given character. + * * `glib/gprintf.h` must be explicitly included in order to use this function. * * Returns: the number of bytes printed. diff --git a/glib/gstrfuncs.c b/glib/gstrfuncs.c index a37ef899c..57aec6823 100644 --- a/glib/gstrfuncs.c +++ b/glib/gstrfuncs.c @@ -491,7 +491,7 @@ g_stpcpy (gchar *dest, /** * g_strdup_vprintf: - * @format: a standard printf() format string, but notice + * @format: (not nullable): a standard printf() format string, but notice * [string precision pitfalls][string-precision] * @args: the list of parameters to insert into the format string * @@ -500,6 +500,10 @@ g_stpcpy (gchar *dest, * the result. The returned string should be freed with g_free() when * no longer needed. * + * The returned string is guaranteed to be non-NULL, unless @format + * contains `%lc` or `%ls` conversions, which can fail if no multibyte + * representation is available for the given character. + * * See also g_vasprintf(), which offers the same functionality, but * additionally returns the length of the allocated string. * @@ -518,7 +522,7 @@ g_strdup_vprintf (const gchar *format, /** * g_strdup_printf: - * @format: a standard printf() format string, but notice + * @format: (not nullable): a standard printf() format string, but notice * [string precision pitfalls][string-precision] * @...: the parameters to insert into the format string * @@ -527,6 +531,10 @@ g_strdup_vprintf (const gchar *format, * the result. The returned string should be freed with g_free() when no * longer needed. * + * The returned string is guaranteed to be non-NULL, unless @format + * contains `%lc` or `%ls` conversions, which can fail if no multibyte + * representation is available for the given character. + * * Returns: a newly-allocated string holding the result */ gchar* diff --git a/glib/gstring.c b/glib/gstring.c index 24692f135..be456ffb4 100644 --- a/glib/gstring.c +++ b/glib/gstring.c @@ -1144,7 +1144,7 @@ g_string_up (GString *string) /** * g_string_append_vprintf: * @string: a #GString - * @format: the string format. See the printf() documentation + * @format: (not nullable): the string format. See the printf() documentation * @args: the list of arguments to insert in the output * * Appends a formatted string onto the end of a #GString. @@ -1179,7 +1179,7 @@ g_string_append_vprintf (GString *string, /** * g_string_vprintf: * @string: a #GString - * @format: the string format. See the printf() documentation + * @format: (not nullable): the string format. See the printf() documentation * @args: the parameters to insert into the format string * * Writes a formatted string into a #GString.