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

On Fri, Oct 18, 2019 at 12:16:00PM +0200, Peter Krempa wrote: > 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. Just put a #undef g_strdup_vprintf in the .c file, since we need the #include in order to get the function prototype declared.