On Mon, Mar 16, 2015 at 02:23:22PM +0000, Ian Campbell wrote:
> On Mon, 2015-03-16 at 10:11 -0400, Konrad Rzeszutek Wilk wrote:
> >  static void hypercall_buffer_cache_lock(xc_interface *xch)
> >  {
> > +    int saved_errno;
> >      if ( xch->flags & XC_OPENFLAG_NON_REENTRANT )
> >          return;
> > +    saved_errno = errno;
> 
> You can do the init on declaration.
> 
> > diff --git a/tools/libxc/xc_linux_osdep.c b/tools/libxc/xc_linux_osdep.c
> > index a19e4b6..15d772b 100644
> > --- a/tools/libxc/xc_linux_osdep.c
> > +++ b/tools/libxc/xc_linux_osdep.c
> > @@ -122,10 +122,13 @@ out:
> >  
> >  static void linux_privcmd_free_hypercall_buffer(xc_interface *xch, 
> > xc_osdep_handle h, void *ptr, int npages)
> >  {
> > +    int saved_errno = errno;
> >      /* Recover the VMA flags. Maybe it's not necessary */
> >      madvise(ptr, npages * XC_PAGE_SIZE, MADV_DOFORK);
> >  
> >      munmap(ptr, npages * XC_PAGE_SIZE);
> > +    /* We MUST propagate the hypercall errno, not unmap calls. */
> 
> Hrm, in general you would want to propagate failure here if the mmap had
> succeeded, otherwise you want the mmap failure. Which would involve
> pushing the saving into the callers.
> 
> But since this function is (for better or worse) void (mainly because it
> may trigger out of band due to the cache) I think you are right to
> preserve the errno as you've done.

OK. Will repost the whole series with the above mentioned fix as v1.

Do you want me to post it now (v1) or are you right now looking at the
patchset and would want to finish that?

> 
> > +    errno = saved_errno;
> >  }
> >  
> >  static int linux_privcmd_hypercall(xc_interface *xch, xc_osdep_handle h, 
> > privcmd_hypercall_t *hypercall)
> > > 
> > > 
> > > Ian.
> > > 
> 
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Reply via email to