[libvirt] [PATCH 1/2] virstring: Reimplement g_strdup_printf() and g_strdup_vprintf()

[Daniel P. Berrangé]

On Fri, Oct 18, 2019 at 10:38:27AM +0200, Peter Krempa wrote:
> On Fri, Oct 18, 2019 at 10:16:49 +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 at redhat.com>
> > ---
> > 
> > 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/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;
> > +}
> 
> Please put this garbage into a separate file. This has nothing to do
> with libvirt and should not pollute our own code.

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 :|