Re: [Xen-devel] [PATCH 1/2] libxc: introduce XC_SAVE_ID_TOOLSTACK

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2012-01-20 at 12:24 +0000, Stefano Stabellini wrote:
> On Fri, 20 Jan 2012, Ian Campbell wrote:
> > On Fri, 2012-01-20 at 11:18 +0000, Stefano Stabellini wrote:
> > > diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c
> > > index a6bb894..3d11c39 100644
> > > --- a/tools/libxc/xc_domain_save.c
> > > +++ b/tools/libxc/xc_domain_save.c
> > > @@ -1676,6 +1676,22 @@ int xc_domain_save(xc_interface *xch, int io_fd, 
> > > uint32_t dom, uint32_t max_iter
> > >          }
> > >      }
> > > 
> > > +    if ( callbacks != NULL && callbacks->toolstack_save != NULL )
> > > +    {
> > > +        int id = XC_SAVE_ID_TOOLSTACK;
> > > +        uint8_t *buf;
> > > +        uint32_t len;
> > > +
> > > +        if ( callbacks->toolstack_save(dom, &buf, &len, callbacks->data) 
> > > < 0 )
> > > +        {
> > > +            PERROR("Error calling toolstack_save");
> > > +            goto out;
> > > +        }
> > > +        wrexact(io_fd, &id, sizeof(id));
> > > +        wrexact(io_fd, &len, sizeof(len));
> > > +        wrexact(io_fd, buf, len);
> > 
> > Where is buf free'd? You say below "callee allocates and frees the
> > buffer" but there is no way that can be the case since the caller uses
> > the buffer after the callback.
> 
> The callee can free the buffer after the completion of xc_domain_save.
> So libxl allocates the buffer in toolstack_save and frees it after
> xc_domain_save returns.
> 
> 
> > I think it has to be callee-alloc, caller-free (perhaps callee-free on
> > error).
> 
> I though that it is more straightforward if the callee does both, but if
> you think that is actually more confusing, I'll change it.

Well, it enforces that the caller has some sort of infrastructure (like
gc) which they may not have. I suppose they could have a little struct
passed via the closure where they stash the allocation to free after
xc_domain_save but I'd contend that this is even weirder/more confusing
than doing caller-free -- you just don't see this because libxl's gc
hides it from you.

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel