[Virtio-fs] [PATCH] virtiofsd: fix incorrect error handling in lo_do_lookup

[Dr. David Alan Gilbert]

* Liu Bo (bo.liu at linux.alibaba.com) wrote:
> On Tue, Jun 11, 2019 at 06:38:33PM +0100, Dr. David Alan Gilbert wrote:
> > * Eric Ren (renzhen at linux.alibaba.com) wrote:
> > > Signed-off-by: Eric Ren <renzhen at linux.alibaba.com>
> > > ---
> > >  contrib/virtiofsd/passthrough_ll.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/contrib/virtiofsd/passthrough_ll.c b/contrib/virtiofsd/passthrough_ll.c
> > > index 574e209d95..78716c8aca 100644
> > > --- a/contrib/virtiofsd/passthrough_ll.c
> > > +++ b/contrib/virtiofsd/passthrough_ll.c
> > > @@ -670,7 +670,6 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name,
> > >  		close(newfd);
> > >  		newfd = -1;
> > >  	} else {
> > > -		saverr = ENOMEM;
> > >  		inode = calloc(1, sizeof(struct lo_inode));
> > >  		if (!inode)
> > >  			goto out_err;
> > 
> > Can you explain why you remove that?   How does it
> > get a sensible errno in this case?
> >
> 
> Are you suggesting that calloc() doesn't set errno on failure?
> 
> the manual reads that
> 
> "The UNIX 98 standard requires malloc(), calloc(), and realloc() to
> set errno to ENOMEM upon failure. Glibc assumes that this is done (and
> the glibc versions of these routines do this); if you use a private
> malloc implementation that does not set errno, then certain library
> routines may fail without having a reason in errno."

Oh OK, and the spec I just found also says it should; the linux manpage
wasn't clear to me.

In that case,

Reviewed-by: Dr. David Alan Gilbert <dgilbert at redhat.com>

and I'll merge it.

> 
> 
> > > @@ -691,7 +690,9 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name,
> > >  	res = fstatat(inode->fd, "", &e->attr,
> > >  		      AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW);
> > >  	if (res == -1) {
> > > +		saverr = errno;
> > >  		unref_inode(lo, inode, 1);
> > > +		errno = saverr;
> > 
> > That one I agree with.
> >
> 
> Well...can we instead set errno in place to avoid any surprise in the future?
> 
> thanks,
> -liubo
> 
> > 
> > >  		goto out_err;
> > >  	}
> > >  
> > > -- 
> > > 2.17.2 (Apple Git-113)
> > > 
> > > _______________________________________________
> > > Virtio-fs mailing list
> > > Virtio-fs at redhat.com
> > > https://www.redhat.com/mailman/listinfo/virtio-fs
> > --
> > Dr. David Alan Gilbert / dgilbert at redhat.com / Manchester, UK
> > 
> > _______________________________________________
> > Virtio-fs mailing list
> > Virtio-fs at redhat.com
> > https://www.redhat.com/mailman/listinfo/virtio-fs
--
Dr. David Alan Gilbert / dgilbert at redhat.com / Manchester, UK