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

[Andrei Vagin]

On Tue, May 15, 2018 at 04:22:15PM -0700, Andrei Vagin wrote:
> 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...

Acrually I know. In other cases, there is not other arguments except %m.

For example:

criu/cr-check.c:                        pr_msg("prctl: PR_SET_MM_EXE_FILE is not supported: %m\n");

> 
> > 
> > Thanks,
> >              Dmitry
> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu