Re: [PATCH] Do not use %m in common code to print error messages

[Daniel P . Berrangé]

On Fri, Oct 18, 2019 at 12:44:38PM +0200, Thomas Huth wrote:
> The %m format specifier is an extension from glibc - and when compiling
> QEMU for NetBSD, the compiler correctly complains, e.g.:
> 
> /home/qemu/qemu-test.ELjfrQ/src/util/main-loop.c: In function 'sigfd_handler':
> /home/qemu/qemu-test.ELjfrQ/src/util/main-loop.c:64:13: warning: %m is only
>  allowed in syslog(3) like functions [-Wformat=]
>              printf("read from sigfd returned %zd: %m\n", len);
>              ^
> Let's use g_strerror() here instead, which is an easy-to-use wrapper
> around the thread-safe strerror_r() function.
> 
> While we're at it, also convert the "printf()" in main-loop.c into
> the preferred "error_report()".
> 
> Signed-off-by: Thomas Huth <address@hidden>
> ---
>  hw/misc/tmp421.c | 8 ++++++--
>  util/main-loop.c | 4 +++-
>  util/systemd.c   | 5 +++--
>  3 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/misc/tmp421.c b/hw/misc/tmp421.c
> index 9f044705fa..f23c46a40a 100644
> --- a/hw/misc/tmp421.c
> +++ b/hw/misc/tmp421.c
> @@ -120,7 +120,9 @@ static void tmp421_get_temperature(Object *obj, Visitor 
> *v, const char *name,
>      int tempid;
>  
>      if (sscanf(name, "temperature%d", &tempid) != 1) {
> -        error_setg(errp, "error reading %s: %m", name);
> +        const char *errmsg = g_strerror(errno);
> +        error_setg(errp, "error reading %s: %s", name, errmsg);
> +        g_free((gpointer)errmsg);

Kaboom crash. This is trying to free a const string that is the caller
doesn't own. It remains under ownership of g_strerror forever.

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