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

Andrei Vagin avagin at virtuozzo.com
Wed May 16 02:22:15 MSK 2018


On Wed, May 16, 2018 at 12:10:04AM +0100, Dmitry Safonov wrote:
> 2018-05-15 23:55 GMT+01:00 Andrei Vagin <avagin at 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 at 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?

vprint_on_level
	print_ts
		gettimeofday
		snprintf
	        size  = vsnprintf(buffer + buf_off, sizeof buffer - buf_off, format, params);


gettimeofday() and snprintf() can change errno. We probably can fix this
by restoring errno before vsnprintf, but I would prefer to use stderror()
to avoid side effects.

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

I don't know why it shows this warning here, but I remember that we have
issues with %m...

> 
> Thanks,
>              Dmitry


More information about the CRIU mailing list