Re: [Qemu-devel] [PATCH 1/3] iov: Add 'offset' parameter to iov_to_buf()

[Alexander Graf]

On 01.07.2011, at 09:42, Hannes Reinecke wrote:

> Occasionally, the buffer needs to be placed at a offset within
> the iovec when copying the buffer to the iovec.

So this is a buffer into the iovec, right? Wouldn't it make sense to also 
modify iov_to_buf respectively then, so the API stays similar? Also, it'd be 
nice to give the parameter a more obvious name, so potential users can easily 
recognize what it offsets.


Alex

> 
> Signed-off-by: Hannes Reinecke <address@hidden>
> ---
> hw/virtio-net.c        |    2 +-
> hw/virtio-serial-bus.c |    2 +-
> iov.c                  |   23 ++++++++++++++---------
> iov.h                  |    2 +-
> 4 files changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> index 6997e02..a32cc01 100644
> --- a/hw/virtio-net.c
> +++ b/hw/virtio-net.c
> @@ -657,7 +657,7 @@ static ssize_t virtio_net_receive(VLANClientState *nc, 
> const uint8_t *buf, size_
> 
>         /* copy in packet.  ugh */
>         len = iov_from_buf(sg, elem.in_num,
> -                           buf + offset, size - offset);
> +                           buf + offset, 0, size - offset);
>         total += len;
>         offset += len;
>         /* If buffers can't be merged, at this point we
> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> index 7f6db7b..53c58d0 100644
> --- a/hw/virtio-serial-bus.c
> +++ b/hw/virtio-serial-bus.c
> @@ -103,7 +103,7 @@ static size_t write_to_port(VirtIOSerialPort *port,
>         }
> 
>         len = iov_from_buf(elem.in_sg, elem.in_num,
> -                           buf + offset, size - offset);
> +                           buf + offset, 0, size - offset);
>         offset += len;
> 
>         virtqueue_push(vq, &elem, len);
> diff --git a/iov.c b/iov.c
> index 588cd04..9ead6ee 100644
> --- a/iov.c
> +++ b/iov.c
> @@ -15,21 +15,26 @@
> #include "iov.h"
> 
> size_t iov_from_buf(struct iovec *iov, unsigned int iovcnt,
> -                    const void *buf, size_t size)
> +                    const void *buf, size_t offset, size_t size)
> {
> -    size_t offset;
> +    size_t iov_off, buf_off;
>     unsigned int i;
> 
> -    offset = 0;
> -    for (i = 0; offset < size && i < iovcnt; i++) {
> -        size_t len;
> +    iov_off = 0;
> +    buf_off = 0;
> +    for (i = 0; i < iovcnt && size; i++) {
> +        if (offset < (iov_off + iov[i].iov_len)) {
> +            size_t len = MIN((iov_off + iov[i].iov_len) - offset, size);
> 
> -        len = MIN(iov[i].iov_len, size - offset);
> +            memcpy(iov[i].iov_base + (offset - iov_off), buf + buf_off, len);
> 
> -        memcpy(iov[i].iov_base, buf + offset, len);
> -        offset += len;
> +            buf_off += len;
> +            offset += len;
> +            size -= len;
> +        }
> +        iov_off += iov[i].iov_len;
>     }
> -    return offset;
> +    return buf_off;
> }
> 
> size_t iov_to_buf(const struct iovec *iov, const unsigned int iovcnt,
> diff --git a/iov.h b/iov.h
> index 60a8547..2677527 100644
> --- a/iov.h
> +++ b/iov.h
> @@ -13,7 +13,7 @@
> #include "qemu-common.h"
> 
> size_t iov_from_buf(struct iovec *iov, unsigned int iovcnt,
> -                    const void *buf, size_t size);
> +                    const void *buf, size_t offset, size_t size);
> size_t iov_to_buf(const struct iovec *iov, const unsigned int iovcnt,
>                   void *buf, size_t offset, size_t size);
> size_t iov_size(const struct iovec *iov, const unsigned int iovcnt);
> -- 
> 1.6.0.2
>