[4/4] unix: don't use %m in pr_warn

Submitted by Dmitry Safonov on May 15, 2018, 11:16 p.m.

Details

Message ID CAJwJo6aVbrJq-Dbya8FYmM0czhcdL_A1zz=KR3hRWBCH_mk8-g@mail.gmail.com
State New
Series "Series without cover letter"
Headers show

Commit Message

Dmitry Safonov May 15, 2018, 11:16 p.m.
2018-05-16 0:10 GMT+01:00 Dmitry Safonov <0x7f454c46@gmail.com>:
> 2018-05-15 23:55 GMT+01:00 Andrei Vagin <avagin@virtuozzo.com>:
>> pr_warn calls a few system calls before printing a message,
>> so it will override errno.
>>
>> CID 190176 (#1 of 1): Printf format string issue (PW.BAD_PRINTF_FORMAT_STRING)
>> 1. bad_printf_format_string: invalid format string conversion
>>
>> Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
>> ---
>>  criu/sk-unix.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/criu/sk-unix.c b/criu/sk-unix.c
>> index ea314bb9f..2838958d6 100644
>> --- a/criu/sk-unix.c
>> +++ b/criu/sk-unix.c
>> @@ -595,8 +595,8 @@ static int unix_resolve_name(int lfd, u32 id, struct unix_sk_desc *d,
>>         snprintf(rpath, sizeof(rpath), ".%s", name);
>>         if (fstatat(mntns_root, rpath, &st, 0)) {
>>                 if (errno != ENOENT) {
>> -                       pr_warn("Can't stat socket %#x(%s), skipping: %m (err %d)\n",
>> -                               id, rpath, errno);
>> +                       pr_warn("Can't stat socket %#x(%s), skipping: %s (err %d)\n",
>> +                               id, rpath, strerror(errno), errno);
>
> I don't get this one.
> pr_warn()=>print_on_level()=>vprint_on_level():
> :        int __errno = errno;
>
> Care to explain?
> I might not spot something obvious, then should we correct all pr_warn/pr_*,
> like the first in the same file:
> :                pr_warn("Unable to open a socket file: %m\n");
>
> Probably, Coverity just doesn't know %m modifier, not really sure if it worth
> to correct ALL %m in criu..

Oh, I probably got what you're talking about..
Maybe something like this?
--->8---
        size += buf_off;

Patch hide | download patch | download mbox

--- a/criu/log.c
+++ b/criu/log.c
@@ -275,6 +275,7 @@  void vprint_on_level(unsigned int loglevel, const
char *format, va_list params)
                        print_ts();
        }

+       errno = __errno;
        size  = vsnprintf(buffer + buf_off, sizeof buffer - buf_off,
format, params);

Comments

Dmitry Safonov May 15, 2018, 11:19 p.m.
2018-05-16 0:16 GMT+01:00 Dmitry Safonov <0x7f454c46@gmail.com>:
> 2018-05-16 0:10 GMT+01:00 Dmitry Safonov <0x7f454c46@gmail.com>:
>> 2018-05-15 23:55 GMT+01:00 Andrei Vagin <avagin@virtuozzo.com>:
>>> pr_warn calls a few system calls before printing a message,
>>> so it will override errno.
>>>
>>> CID 190176 (#1 of 1): Printf format string issue (PW.BAD_PRINTF_FORMAT_STRING)
>>> 1. bad_printf_format_string: invalid format string conversion
>>>
>>> Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
>>> ---
>>>  criu/sk-unix.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/criu/sk-unix.c b/criu/sk-unix.c
>>> index ea314bb9f..2838958d6 100644
>>> --- a/criu/sk-unix.c
>>> +++ b/criu/sk-unix.c
>>> @@ -595,8 +595,8 @@ static int unix_resolve_name(int lfd, u32 id, struct unix_sk_desc *d,
>>>         snprintf(rpath, sizeof(rpath), ".%s", name);
>>>         if (fstatat(mntns_root, rpath, &st, 0)) {
>>>                 if (errno != ENOENT) {
>>> -                       pr_warn("Can't stat socket %#x(%s), skipping: %m (err %d)\n",
>>> -                               id, rpath, errno);
>>> +                       pr_warn("Can't stat socket %#x(%s), skipping: %s (err %d)\n",
>>> +                               id, rpath, strerror(errno), errno);
>>
>> I don't get this one.
>> pr_warn()=>print_on_level()=>vprint_on_level():
>> :        int __errno = errno;
>>
>> Care to explain?
>> I might not spot something obvious, then should we correct all pr_warn/pr_*,
>> like the first in the same file:
>> :                pr_warn("Unable to open a socket file: %m\n");
>>
>> Probably, Coverity just doesn't know %m modifier, not really sure if it worth
>> to correct ALL %m in criu..
>
> Oh, I probably got what you're talking about..
> Maybe something like this?

Otherwise, it might be challenging:
[criu]$ git grep '%m\>' | wc -l
213

Thanks,
             Dmitry