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@redhat.com>
---

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"
 
 /* Merely casting to (void) is not sufficient since the
  * introduction of the "warn_unused_result" attribute
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0da02bb8bd..206295bef0 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1485,6 +1485,11 @@ virSecurityManagerTransactionStart;
 virSecurityManagerVerify;
 
 
+# util/glibcompat.h
+vir_g_strdup_printf;
+vir_g_strdup_vprintf;
+
+
 # util/viralloc.h
 virAlloc;
 virAllocN;
diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am
index 705b93c93c..d8c8e61c4b 100644
--- a/src/util/Makefile.inc.am
+++ b/src/util/Makefile.inc.am
@@ -3,6 +3,8 @@
 # These files are not related to driver APIs. Simply generic
 # helper APIs for various purposes
 UTIL_SOURCES = \
+	util/glibcompat.c \
+	util/glibcompat.h \
 	util/viralloc.c \
 	util/viralloc.h \
 	util/virarch.c \
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"
+
+/* 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;
+}
diff --git a/src/util/glibcompat.h b/src/util/glibcompat.h
new file mode 100644
index 0000000000..9c5fef09bf
--- /dev/null
+++ b/src/util/glibcompat.h
@@ -0,0 +1,31 @@
+/*
+ * 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/>.
+ */
+
+#pragma once
+
+#include <glib.h>
+
+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
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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@redhat.com>
> ---
> 
> 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.


> diff --git a/src/util/glibcompat.h b/src/util/glibcompat.h
> new file mode 100644
> index 0000000000..9c5fef09bf
> --- /dev/null
> +++ b/src/util/glibcompat.h
> @@ -0,0 +1,31 @@
> +/*
> + * 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/>.
> + */
> +
> +#pragma once
> +
> +#include <glib.h>
> +
> +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
> -- 
> 2.21.0
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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@redhat.com>
>> ---
>>
>> 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
>>

>> +/* 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.
>

No problem, we can just turn off the warning:
diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4
index 7c86fdd3c6..e531eadc1e 100644
--- a/m4/virt-compile-warnings.m4
+++ b/m4/virt-compile-warnings.m4
@@ -127,6 +127,9 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[
     dontwarn="$dontwarn -Woverlength-strings"
     dontwarn="$dontwarn -Wstack-protector"
 
+    # Needed for our g_strdup_vprintf workaround
+    dontwarn="$dontwarn -Winfinite-recursion"
+
     # Get all possible GCC warnings
     gl_MANYWARN_ALL_GCC([maybewarn])
 

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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@redhat.com>
> > ---
> > 
> > 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.


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

On Fri, Oct 18, 2019 at 11:25:55 +0100, Daniel Berrange wrote:
> 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@redhat.com>
> > > ---
> > > 
> > > 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.

My idea was to have a special header just for the defines, but this
works as well.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list