All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] Do not use %m in common code to print error messages
@ 2019-10-18 13:07 Thomas Huth
  2019-10-18 13:10 ` Daniel P. Berrangé
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Thomas Huth @ 2019-10-18 13:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-trivial, Paolo Bonzini, Kamil Rytarowski, berrange

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 <thuth@redhat.com>
---
 v2: Do not try to g_free() the strings

 hw/misc/tmp421.c | 4 ++--
 util/main-loop.c | 3 ++-
 util/systemd.c   | 4 ++--
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/misc/tmp421.c b/hw/misc/tmp421.c
index 9f044705fa..c0bc150bca 100644
--- a/hw/misc/tmp421.c
+++ b/hw/misc/tmp421.c
@@ -120,7 +120,7 @@ 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);
+        error_setg(errp, "error reading %s: %s", name, g_strerror(errno));
         return;
     }
 
@@ -160,7 +160,7 @@ static void tmp421_set_temperature(Object *obj, Visitor *v, const char *name,
     }
 
     if (sscanf(name, "temperature%d", &tempid) != 1) {
-        error_setg(errp, "error reading %s: %m", name);
+        error_setg(errp, "error reading %s: %s", name, g_strerror(errno));
         return;
     }
 
diff --git a/util/main-loop.c b/util/main-loop.c
index e3eaa55866..eda63fe4e0 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -61,7 +61,8 @@ static void sigfd_handler(void *opaque)
         }
 
         if (len != sizeof(info)) {
-            printf("read from sigfd returned %zd: %m\n", len);
+            error_report("read from sigfd returned %zd: %s", len,
+                         g_strerror(errno));
             return;
         }
 
diff --git a/util/systemd.c b/util/systemd.c
index d22e86c707..1dd0367d9a 100644
--- a/util/systemd.c
+++ b/util/systemd.c
@@ -60,8 +60,8 @@ unsigned int check_socket_activation(void)
              * and we should exit.
              */
             error_report("Socket activation failed: "
-                         "invalid file descriptor fd = %d: %m",
-                         fd);
+                         "invalid file descriptor fd = %d: %s",
+                         fd, g_strerror(errno));
             exit(EXIT_FAILURE);
         }
     }
-- 
2.18.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] Do not use %m in common code to print error messages
  2019-10-18 13:07 [PATCH v2] Do not use %m in common code to print error messages Thomas Huth
@ 2019-10-18 13:10 ` Daniel P. Berrangé
  2019-10-18 13:42 ` Stefano Garzarella
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Daniel P. Berrangé @ 2019-10-18 13:10 UTC (permalink / raw)
  To: Thomas Huth; +Cc: qemu-trivial, Paolo Bonzini, Kamil Rytarowski, qemu-devel

On Fri, Oct 18, 2019 at 03:07:16PM +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 <thuth@redhat.com>
> ---
>  v2: Do not try to g_free() the strings
> 
>  hw/misc/tmp421.c | 4 ++--
>  util/main-loop.c | 3 ++-
>  util/systemd.c   | 4 ++--
>  3 files changed, 6 insertions(+), 5 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] Do not use %m in common code to print error messages
  2019-10-18 13:07 [PATCH v2] Do not use %m in common code to print error messages Thomas Huth
  2019-10-18 13:10 ` Daniel P. Berrangé
@ 2019-10-18 13:42 ` Stefano Garzarella
  2019-10-18 13:49   ` Kamil Rytarowski
  2019-10-21 16:28 ` Paolo Bonzini
  2019-10-21 16:29 ` Laurent Vivier
  3 siblings, 1 reply; 9+ messages in thread
From: Stefano Garzarella @ 2019-10-18 13:42 UTC (permalink / raw)
  To: Thomas Huth
  Cc: qemu-trivial, Paolo Bonzini, Kamil Rytarowski, berrange, qemu-devel

On Fri, Oct 18, 2019 at 03:07:16PM +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 <thuth@redhat.com>
> ---
>  v2: Do not try to g_free() the strings
> 
>  hw/misc/tmp421.c | 4 ++--
>  util/main-loop.c | 3 ++-
>  util/systemd.c   | 4 ++--
>  3 files changed, 6 insertions(+), 5 deletions(-)

There are many uses of %m also in hw/vfio/ but that's Linux stuff.
Should we change those too or it doesn't matter since it never really
compiled on NetBSD?

Anyway, this patch LGTM:
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

Thanks,
Stefano

> 
> diff --git a/hw/misc/tmp421.c b/hw/misc/tmp421.c
> index 9f044705fa..c0bc150bca 100644
> --- a/hw/misc/tmp421.c
> +++ b/hw/misc/tmp421.c
> @@ -120,7 +120,7 @@ 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);
> +        error_setg(errp, "error reading %s: %s", name, g_strerror(errno));
>          return;
>      }
>  
> @@ -160,7 +160,7 @@ static void tmp421_set_temperature(Object *obj, Visitor *v, const char *name,
>      }
>  
>      if (sscanf(name, "temperature%d", &tempid) != 1) {
> -        error_setg(errp, "error reading %s: %m", name);
> +        error_setg(errp, "error reading %s: %s", name, g_strerror(errno));
>          return;
>      }
>  
> diff --git a/util/main-loop.c b/util/main-loop.c
> index e3eaa55866..eda63fe4e0 100644
> --- a/util/main-loop.c
> +++ b/util/main-loop.c
> @@ -61,7 +61,8 @@ static void sigfd_handler(void *opaque)
>          }
>  
>          if (len != sizeof(info)) {
> -            printf("read from sigfd returned %zd: %m\n", len);
> +            error_report("read from sigfd returned %zd: %s", len,
> +                         g_strerror(errno));
>              return;
>          }
>  
> diff --git a/util/systemd.c b/util/systemd.c
> index d22e86c707..1dd0367d9a 100644
> --- a/util/systemd.c
> +++ b/util/systemd.c
> @@ -60,8 +60,8 @@ unsigned int check_socket_activation(void)
>               * and we should exit.
>               */
>              error_report("Socket activation failed: "
> -                         "invalid file descriptor fd = %d: %m",
> -                         fd);
> +                         "invalid file descriptor fd = %d: %s",
> +                         fd, g_strerror(errno));
>              exit(EXIT_FAILURE);
>          }
>      }
> -- 
> 2.18.1
> 
> 

-- 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] Do not use %m in common code to print error messages
  2019-10-18 13:42 ` Stefano Garzarella
@ 2019-10-18 13:49   ` Kamil Rytarowski
  2019-10-18 16:01     ` Thomas Huth
  0 siblings, 1 reply; 9+ messages in thread
From: Kamil Rytarowski @ 2019-10-18 13:49 UTC (permalink / raw)
  To: Stefano Garzarella, Thomas Huth
  Cc: qemu-trivial, Paolo Bonzini, Kamil Rytarowski, berrange, qemu-devel

On 18.10.2019 15:42, Stefano Garzarella wrote:
> On Fri, Oct 18, 2019 at 03:07:16PM +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 <thuth@redhat.com>
>> ---
>>  v2: Do not try to g_free() the strings
>>
>>  hw/misc/tmp421.c | 4 ++--
>>  util/main-loop.c | 3 ++-
>>  util/systemd.c   | 4 ++--
>>  3 files changed, 6 insertions(+), 5 deletions(-)
>
> There are many uses of %m also in hw/vfio/ but that's Linux stuff.
> Should we change those too or it doesn't matter since it never really
> compiled on NetBSD?
>

It's a gnu (glibc) extension and linux can use alternative libc
implementations. Probably most of them capable to host qemu use %m.

> Anyway, this patch LGTM:
> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>
> Thanks,
> Stefano
>
>>
>> diff --git a/hw/misc/tmp421.c b/hw/misc/tmp421.c
>> index 9f044705fa..c0bc150bca 100644
>> --- a/hw/misc/tmp421.c
>> +++ b/hw/misc/tmp421.c
>> @@ -120,7 +120,7 @@ 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);
>> +        error_setg(errp, "error reading %s: %s", name, g_strerror(errno));
>>          return;
>>      }
>>
>> @@ -160,7 +160,7 @@ static void tmp421_set_temperature(Object *obj, Visitor *v, const char *name,
>>      }
>>
>>      if (sscanf(name, "temperature%d", &tempid) != 1) {
>> -        error_setg(errp, "error reading %s: %m", name);
>> +        error_setg(errp, "error reading %s: %s", name, g_strerror(errno));
>>          return;
>>      }
>>
>> diff --git a/util/main-loop.c b/util/main-loop.c
>> index e3eaa55866..eda63fe4e0 100644
>> --- a/util/main-loop.c
>> +++ b/util/main-loop.c
>> @@ -61,7 +61,8 @@ static void sigfd_handler(void *opaque)
>>          }
>>
>>          if (len != sizeof(info)) {
>> -            printf("read from sigfd returned %zd: %m\n", len);
>> +            error_report("read from sigfd returned %zd: %s", len,
>> +                         g_strerror(errno));
>>              return;
>>          }
>>
>> diff --git a/util/systemd.c b/util/systemd.c
>> index d22e86c707..1dd0367d9a 100644
>> --- a/util/systemd.c
>> +++ b/util/systemd.c
>> @@ -60,8 +60,8 @@ unsigned int check_socket_activation(void)
>>               * and we should exit.
>>               */
>>              error_report("Socket activation failed: "
>> -                         "invalid file descriptor fd = %d: %m",
>> -                         fd);
>> +                         "invalid file descriptor fd = %d: %s",
>> +                         fd, g_strerror(errno));
>>              exit(EXIT_FAILURE);
>>          }
>>      }
>> --
>> 2.18.1
>>
>>
>



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] Do not use %m in common code to print error messages
  2019-10-18 13:49   ` Kamil Rytarowski
@ 2019-10-18 16:01     ` Thomas Huth
  2019-10-19 14:00       ` Stefano Garzarella
  0 siblings, 1 reply; 9+ messages in thread
From: Thomas Huth @ 2019-10-18 16:01 UTC (permalink / raw)
  To: Kamil Rytarowski, Stefano Garzarella
  Cc: qemu-trivial, Paolo Bonzini, Kamil Rytarowski, berrange, qemu-devel

On 18/10/2019 15.49, Kamil Rytarowski wrote:
> On 18.10.2019 15:42, Stefano Garzarella wrote:
>> On Fri, Oct 18, 2019 at 03:07:16PM +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 <thuth@redhat.com>
>>> ---
>>>  v2: Do not try to g_free() the strings
>>>
>>>  hw/misc/tmp421.c | 4 ++--
>>>  util/main-loop.c | 3 ++-
>>>  util/systemd.c   | 4 ++--
>>>  3 files changed, 6 insertions(+), 5 deletions(-)
>>
>> There are many uses of %m also in hw/vfio/ but that's Linux stuff.
>> Should we change those too or it doesn't matter since it never really
>> compiled on NetBSD?
> 
> It's a gnu (glibc) extension and linux can use alternative libc
> implementations. Probably most of them capable to host qemu use %m.

I think I read somewhere that other libcs on Linux also support %m (like
musl), but I just can't find that reference anymore. Anyway, we can
still fix that later in case someone hits the issue.

>> Anyway, this patch LGTM:
>> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

 Thanks,
  Thomas


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] Do not use %m in common code to print error messages
  2019-10-18 16:01     ` Thomas Huth
@ 2019-10-19 14:00       ` Stefano Garzarella
  0 siblings, 0 replies; 9+ messages in thread
From: Stefano Garzarella @ 2019-10-19 14:00 UTC (permalink / raw)
  To: Thomas Huth
  Cc: berrange, qemu-trivial, qemu-devel, Kamil Rytarowski,
	Paolo Bonzini, Kamil Rytarowski

On Fri, Oct 18, 2019 at 06:01:22PM +0200, Thomas Huth wrote:
> On 18/10/2019 15.49, Kamil Rytarowski wrote:
> > On 18.10.2019 15:42, Stefano Garzarella wrote:
> >> On Fri, Oct 18, 2019 at 03:07:16PM +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 <thuth@redhat.com>
> >>> ---
> >>>  v2: Do not try to g_free() the strings
> >>>
> >>>  hw/misc/tmp421.c | 4 ++--
> >>>  util/main-loop.c | 3 ++-
> >>>  util/systemd.c   | 4 ++--
> >>>  3 files changed, 6 insertions(+), 5 deletions(-)
> >>
> >> There are many uses of %m also in hw/vfio/ but that's Linux stuff.
> >> Should we change those too or it doesn't matter since it never really
> >> compiled on NetBSD?
> > 
> > It's a gnu (glibc) extension and linux can use alternative libc
> > implementations. Probably most of them capable to host qemu use %m.
> 
> I think I read somewhere that other libcs on Linux also support %m (like

Good to know!

> musl), but I just can't find that reference anymore. Anyway, we can
> still fix that later in case someone hits the issue.
> 

Yes, make sense to me.

Thanks,
Stefano


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] Do not use %m in common code to print error messages
  2019-10-18 13:07 [PATCH v2] Do not use %m in common code to print error messages Thomas Huth
  2019-10-18 13:10 ` Daniel P. Berrangé
  2019-10-18 13:42 ` Stefano Garzarella
@ 2019-10-21 16:28 ` Paolo Bonzini
  2019-10-21 16:29 ` Laurent Vivier
  3 siblings, 0 replies; 9+ messages in thread
From: Paolo Bonzini @ 2019-10-21 16:28 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel; +Cc: qemu-trivial, Kamil Rytarowski, berrange

On 18/10/19 15:07, 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 <thuth@redhat.com>
> ---
>  v2: Do not try to g_free() the strings
> 
>  hw/misc/tmp421.c | 4 ++--
>  util/main-loop.c | 3 ++-
>  util/systemd.c   | 4 ++--
>  3 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/misc/tmp421.c b/hw/misc/tmp421.c
> index 9f044705fa..c0bc150bca 100644
> --- a/hw/misc/tmp421.c
> +++ b/hw/misc/tmp421.c
> @@ -120,7 +120,7 @@ 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);
> +        error_setg(errp, "error reading %s: %s", name, g_strerror(errno));
>          return;
>      }
>  
> @@ -160,7 +160,7 @@ static void tmp421_set_temperature(Object *obj, Visitor *v, const char *name,
>      }
>  
>      if (sscanf(name, "temperature%d", &tempid) != 1) {
> -        error_setg(errp, "error reading %s: %m", name);
> +        error_setg(errp, "error reading %s: %s", name, g_strerror(errno));
>          return;
>      }
>  
> diff --git a/util/main-loop.c b/util/main-loop.c
> index e3eaa55866..eda63fe4e0 100644
> --- a/util/main-loop.c
> +++ b/util/main-loop.c
> @@ -61,7 +61,8 @@ static void sigfd_handler(void *opaque)
>          }
>  
>          if (len != sizeof(info)) {
> -            printf("read from sigfd returned %zd: %m\n", len);
> +            error_report("read from sigfd returned %zd: %s", len,
> +                         g_strerror(errno));
>              return;
>          }
>  
> diff --git a/util/systemd.c b/util/systemd.c
> index d22e86c707..1dd0367d9a 100644
> --- a/util/systemd.c
> +++ b/util/systemd.c
> @@ -60,8 +60,8 @@ unsigned int check_socket_activation(void)
>               * and we should exit.
>               */
>              error_report("Socket activation failed: "
> -                         "invalid file descriptor fd = %d: %m",
> -                         fd);
> +                         "invalid file descriptor fd = %d: %s",
> +                         fd, g_strerror(errno));
>              exit(EXIT_FAILURE);
>          }
>      }
> 

Queued, thanks.

Paolo


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] Do not use %m in common code to print error messages
  2019-10-18 13:07 [PATCH v2] Do not use %m in common code to print error messages Thomas Huth
                   ` (2 preceding siblings ...)
  2019-10-21 16:28 ` Paolo Bonzini
@ 2019-10-21 16:29 ` Laurent Vivier
  2019-10-21 16:35   ` Laurent Vivier
  3 siblings, 1 reply; 9+ messages in thread
From: Laurent Vivier @ 2019-10-21 16:29 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel
  Cc: qemu-trivial, Paolo Bonzini, Kamil Rytarowski, berrange

Le 18/10/2019 à 15:07, Thomas Huth a écrit :
> 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 <thuth@redhat.com>
> ---
>  v2: Do not try to g_free() the strings
> 
>  hw/misc/tmp421.c | 4 ++--
>  util/main-loop.c | 3 ++-
>  util/systemd.c   | 4 ++--
>  3 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/misc/tmp421.c b/hw/misc/tmp421.c
> index 9f044705fa..c0bc150bca 100644
> --- a/hw/misc/tmp421.c
> +++ b/hw/misc/tmp421.c
> @@ -120,7 +120,7 @@ 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);
> +        error_setg(errp, "error reading %s: %s", name, g_strerror(errno));
>          return;
>      }
>  
> @@ -160,7 +160,7 @@ static void tmp421_set_temperature(Object *obj, Visitor *v, const char *name,
>      }
>  
>      if (sscanf(name, "temperature%d", &tempid) != 1) {
> -        error_setg(errp, "error reading %s: %m", name);
> +        error_setg(errp, "error reading %s: %s", name, g_strerror(errno));
>          return;
>      }
>  
> diff --git a/util/main-loop.c b/util/main-loop.c
> index e3eaa55866..eda63fe4e0 100644
> --- a/util/main-loop.c
> +++ b/util/main-loop.c
> @@ -61,7 +61,8 @@ static void sigfd_handler(void *opaque)
>          }
>  
>          if (len != sizeof(info)) {
> -            printf("read from sigfd returned %zd: %m\n", len);
> +            error_report("read from sigfd returned %zd: %s", len,
> +                         g_strerror(errno));
>              return;
>          }
>  
> diff --git a/util/systemd.c b/util/systemd.c
> index d22e86c707..1dd0367d9a 100644
> --- a/util/systemd.c
> +++ b/util/systemd.c
> @@ -60,8 +60,8 @@ unsigned int check_socket_activation(void)
>               * and we should exit.
>               */
>              error_report("Socket activation failed: "
> -                         "invalid file descriptor fd = %d: %m",
> -                         fd);
> +                         "invalid file descriptor fd = %d: %s",
> +                         fd, g_strerror(errno));
>              exit(EXIT_FAILURE);
>          }
>      }
> 

Applied to my trivial-patches branch.

Thanks,
Laurent


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] Do not use %m in common code to print error messages
  2019-10-21 16:29 ` Laurent Vivier
@ 2019-10-21 16:35   ` Laurent Vivier
  0 siblings, 0 replies; 9+ messages in thread
From: Laurent Vivier @ 2019-10-21 16:35 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel
  Cc: qemu-trivial, Paolo Bonzini, Kamil Rytarowski, berrange

Le 21/10/2019 à 18:29, Laurent Vivier a écrit :
> Le 18/10/2019 à 15:07, Thomas Huth a écrit :
>> 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 <thuth@redhat.com>
>> ---
>>  v2: Do not try to g_free() the strings
>>
>>  hw/misc/tmp421.c | 4 ++--
>>  util/main-loop.c | 3 ++-
>>  util/systemd.c   | 4 ++--
>>  3 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/misc/tmp421.c b/hw/misc/tmp421.c
>> index 9f044705fa..c0bc150bca 100644
>> --- a/hw/misc/tmp421.c
>> +++ b/hw/misc/tmp421.c
>> @@ -120,7 +120,7 @@ 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);
>> +        error_setg(errp, "error reading %s: %s", name, g_strerror(errno));
>>          return;
>>      }
>>  
>> @@ -160,7 +160,7 @@ static void tmp421_set_temperature(Object *obj, Visitor *v, const char *name,
>>      }
>>  
>>      if (sscanf(name, "temperature%d", &tempid) != 1) {
>> -        error_setg(errp, "error reading %s: %m", name);
>> +        error_setg(errp, "error reading %s: %s", name, g_strerror(errno));
>>          return;
>>      }
>>  
>> diff --git a/util/main-loop.c b/util/main-loop.c
>> index e3eaa55866..eda63fe4e0 100644
>> --- a/util/main-loop.c
>> +++ b/util/main-loop.c
>> @@ -61,7 +61,8 @@ static void sigfd_handler(void *opaque)
>>          }
>>  
>>          if (len != sizeof(info)) {
>> -            printf("read from sigfd returned %zd: %m\n", len);
>> +            error_report("read from sigfd returned %zd: %s", len,
>> +                         g_strerror(errno));
>>              return;
>>          }
>>  
>> diff --git a/util/systemd.c b/util/systemd.c
>> index d22e86c707..1dd0367d9a 100644
>> --- a/util/systemd.c
>> +++ b/util/systemd.c
>> @@ -60,8 +60,8 @@ unsigned int check_socket_activation(void)
>>               * and we should exit.
>>               */
>>              error_report("Socket activation failed: "
>> -                         "invalid file descriptor fd = %d: %m",
>> -                         fd);
>> +                         "invalid file descriptor fd = %d: %s",
>> +                         fd, g_strerror(errno));
>>              exit(EXIT_FAILURE);
>>          }
>>      }
>>
> 
> Applied to my trivial-patches branch.

As Paolo has already queued it, I remove it from my queue.

Thanks,
Laurent



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-10-21 16:39 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-18 13:07 [PATCH v2] Do not use %m in common code to print error messages Thomas Huth
2019-10-18 13:10 ` Daniel P. Berrangé
2019-10-18 13:42 ` Stefano Garzarella
2019-10-18 13:49   ` Kamil Rytarowski
2019-10-18 16:01     ` Thomas Huth
2019-10-19 14:00       ` Stefano Garzarella
2019-10-21 16:28 ` Paolo Bonzini
2019-10-21 16:29 ` Laurent Vivier
2019-10-21 16:35   ` Laurent Vivier

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.