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@xxxxxxxxxx> > > --- > > > > 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
