[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] Do not use %m in common code to print error messages
From: |
Kamil Rytarowski |
Subject: |
Re: [PATCH v2] Do not use %m in common code to print error messages |
Date: |
Fri, 18 Oct 2019 15:49:12 +0200 |
User-agent: |
Mozilla/5.0 (X11; NetBSD amd64; rv:68.0) Gecko/20100101 Thunderbird/68.1.2 |
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 <address@hidden>
>> ---
>> 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 <address@hidden>
>
> 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
>>
>>
>