These functions don't really abort() on OOM. The fix was merged upstream, but not in the minimal version we require. Provide our own implementation which can be removed once we bump the minimal version. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- Dan claims this is fixed upstream, but I'm failing to see any abort() in current master: https://gitlab.gnome.org/GNOME/glib/blob/master/glib/gprintf.c#L320 There is g_new() called, but it's done so only in one case out of three. On my system, HAVE_VASPRINTF is defined meaning the function still won't abort(). src/libvirt_private.syms | 2 ++ src/util/virstring.c | 29 +++++++++++++++++++++++++++++ src/util/virstring.h | 11 +++++++++++ 3 files changed, 42 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0da02bb8bd..9eac489a32 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3060,6 +3060,8 @@ virStorageFileBackendRegister; # util/virstring.h +vir_g_strdup_printf; +vir_g_strdup_vprintf; virAsprintfInternal; virSkipSpaces; virSkipSpacesAndBackslash; diff --git a/src/util/virstring.c b/src/util/virstring.c index 6453a23ada..fa7b15d0b7 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -768,6 +768,35 @@ virAsprintfInternal(char **strp, return ret; } + +/* Due to a bug in glib, g_strdup_printf() nor g_strdup_vprintf() + * abort on OOM. It's fixed in glib's upstream. Provide our own + * implementation until the fix get's distributed. */ +char * +vir_g_strdup_printf(const char *msg, ...) +{ + va_list args; + char *ret; + va_start(args, msg); + ret = g_strdup_vprintf(msg, args); + if (!ret) + abort(); + va_end(args); + return ret; +} + + +char * +vir_g_strdup_vprintf(const char *msg, va_list args) +{ + char *ret; + ret = g_strdup_vprintf(msg, args); + if (!ret) + abort(); + return ret; +} + + /** * virStrncpy: * diff --git a/src/util/virstring.h b/src/util/virstring.h index f5e2302b8b..b3a85b9ac2 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -253,6 +253,17 @@ size_t virStringListLength(const char * const *strings); #define virAsprintfQuiet(strp, ...) virAsprintf(strp, __VA_ARGS__) +char *vir_g_strdup_printf(const char *msg, ...) + G_GNUC_PRINTF(1, 2); +char *vir_g_strdup_vprintf(const char *msg, va_list args) + G_GNUC_PRINTF(1, 0); + +#if !GLIB_CHECK_VERSION(2, 64, 0) +# define g_strdup_printf vir_g_strdup_printf +# define g_strdup_vprintf vir_g_strdup_vprintf +#endif + + int virStringSortCompare(const void *a, const void *b); int virStringSortRevCompare(const void *a, const void *b); int virStringToUpper(char **dst, const char *src); -- 2.21.0

On Fri, Oct 18, 2019 at 10:38:27AM +0200, Peter Krempa wrote: Calling this garbage is not really appropriate. We should have a glibcompat.{c,h} file pair though, where the header is #include from internal.h so it is guaranteed visible across the whole codebase. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

With glib inclusion, some of its functions have __attribute__((__malloc__)) which make compiler realize we want to use the same attribute for some trivial functions of ours. For instance qemuDomainManagedSavePath(). I don't see any real benefit into using the attribute, so disable that suggestion. In fact, wrong use of the attribute may lead to mysterious bugs: https://gitlab.gnome.org/GNOME/glib/issues/1465 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- m4/virt-compile-warnings.m4 | 1 + 1 file changed, 1 insertion(+) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 7c86fdd3c6..1318ca59b9 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -126,6 +126,7 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dontwarn="$dontwarn -Wunused-macros" dontwarn="$dontwarn -Woverlength-strings" dontwarn="$dontwarn -Wstack-protector" + dontwarn="$dontwarn -Wsuggest-attribute=malloc" # Get all possible GCC warnings gl_MANYWARN_ALL_GCC([maybewarn]) -- 2.21.0