Re: [libvirt] [PATCH v2] glibcompat: Reimplement g_strdup_printf() and g_strdup_vprintf()

On Fri, Oct 18, 2019 at 11:49:05 +0200, Michal Privoznik wrote: > 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; > --- > > v2 of: > > https://www.redhat.com/archives/libvir-list/2019-October/msg01153.html > > diff to v1: > - moved the code to src/utils/glibcompat.c > > src/internal.h | 1 + > src/libvirt_private.syms | 5 +++++ > src/util/Makefile.inc.am | 2 ++ > src/util/glibcompat.c | 48 ++++++++++++++++++++++++++++++++++++++++ > src/util/glibcompat.h | 31 ++++++++++++++++++++++++++ > 5 files changed, 87 insertions(+) > create mode 100644 src/util/glibcompat.c > create mode 100644 src/util/glibcompat.h > > diff --git a/src/internal.h b/src/internal.h > index fb17b87baa..5b0a2335f5 100644 > --- a/src/internal.h > +++ b/src/internal.h > @@ -64,6 +64,7 @@ > #include "libvirt/virterror.h" > > #include "c-strcase.h" > +#include "glibcompat.h" Please include this right below glib.h > > /* Merely casting to (void) is not sufficient since the > * introduction of the "warn_unused_result" attribute [...] > diff --git a/src/util/glibcompat.c b/src/util/glibcompat.c > new file mode 100644 > index 0000000000..af638f4d8c > --- /dev/null > +++ b/src/util/glibcompat.c > @@ -0,0 +1,48 @@ > +/* > + * Copyright (C) 2019 Red Hat, Inc. > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library. If not, see > + * <http://www.gnu.org/licenses/>;. > + */ > + > +#include <config.h> > + > +#include "glibcompat.h" Sooo, this includes the macros ... > + > +/* 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); So this will become vir_g_strdup_vprintf.. thus become infinite recursion. > + if (!ret) > + abort(); > + return ret; > +} I think the kludge header must be separate and not included into this implementation, otherwise it will not work as expected.