Re: [Qemu-devel] [PATCH 03/19] buffer: add buffer_move_empty

[Daniel P. Berrange]

On Fri, Oct 30, 2015 at 09:11:01PM +0900, Daniel P. Berrange wrote:
> On Fri, Oct 30, 2015 at 12:09:58PM +0100, Gerd Hoffmann wrote:
> > Signed-off-by: Gerd Hoffmann <address@hidden>
> > Reviewed-by: Peter Lieven <address@hidden>
> > ---
> >  include/qemu/buffer.h | 10 ++++++++++
> >  util/buffer.c         | 14 ++++++++++++++
> >  2 files changed, 24 insertions(+)
> > 
> > diff --git a/include/qemu/buffer.h b/include/qemu/buffer.h
> > index 0710e16..f53ee9e 100644
> > --- a/include/qemu/buffer.h
> > +++ b/include/qemu/buffer.h
> > @@ -127,4 +127,14 @@ uint8_t *buffer_end(Buffer *buffer);
> >   */
> >  gboolean buffer_empty(Buffer *buffer);
> >  
> > +/**
> > + * buffer_move_empty:
> > + * @to: destination buffer object
> > + * @from: source buffer object
> > + *
> > + * Moves buffer, without copying data.  'to' buffer must be empty.
> 
> Hmm, on the one hand the code do 'assert(to->offset) == 0', but on
> the other hand it does 'g_free(to->buffer)' as if it expected the
> buffer to have existing data.
> 
> If you just remove the 'assert', then there's no real requirement
> for 'to' to be empty, as we'd do the right thin in free'ing the
> data. Then we can just change this API doc to say
> 
>     "Any existing data in "to" will be freed"

Ignore this comment. I forget that we have an optimization to keep
buffer around when buffer_reset(), as distinct from buffer_free().

Reviewed-by: Daniel Berrange <address@hidden>

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|