[03/19] buffer: add buffer_move_empty

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Peter Lieven <pl@kamp.de>
---
 include/qemu/buffer.h | 10 ++++++++++
 util/buffer.c         | 14 ++++++++++++++
 2 files changed, 24 insertions(+)

On Fri, Oct 30, 2015 at 12:09:58PM +0100, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Reviewed-by: Peter Lieven <pl@kamp.de>
> ---
>  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"

> + * 'from' buffer is empty and zero-sized on return.
> + */
> +void buffer_move_empty(Buffer *to, Buffer *from);
> +
>  #endif /* QEMU_BUFFER_H__ */
> diff --git a/util/buffer.c b/util/buffer.c
> index 12bf2d7..c7a39ec 100644
> --- a/util/buffer.c
> +++ b/util/buffer.c
> @@ -77,3 +77,17 @@ void buffer_advance(Buffer *buffer, size_t len)
>              (buffer->offset - len));
>      buffer->offset -= len;
>  }
> +
> +void buffer_move_empty(Buffer *to, Buffer *from)
> +{
> +    assert(to->offset == 0);
> +
> +    g_free(to->buffer);
> +    to->offset = from->offset;
> +    to->capacity = from->capacity;
> +    to->buffer = from->buffer;
> +
> +    from->offset = 0;
> +    from->capacity = 0;
> +    from->buffer = NULL;
> +}

Regards,
Daniel

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 <kraxel@redhat.com>
> > Reviewed-by: Peter Lieven <pl@kamp.de>
> > ---
> >  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 <berrange@redhat.com>

Regards,
Daniel