Re: [Qemu-devel] [PATCHv2 1/7] Consolidate qemu_iovec_memset{, _skip}() into single, simplified function

[Kevin Wolf]

Am 11.03.2012 02:49, schrieb Michael Tokarev:
> This patch combines two functions into one, simplifies the
> implementation and adds some assert()s into place.
> 
> The new prototype of qemu_iovec_memset():
>   void qemu_iovec_memset(qiov, size_t offset, int c, size_t bytes)
> It is different from former qemu_iovec_memset_skip(), and
> I want to make other functions to be consistent with it too:
> first how much to skip, second what, and 3rd how many of it.
> 
> Signed-off-by: Michael Tokarev <address@hidden>
> ---
>  block/qcow2.c      |    4 ++--
>  block/qed.c        |    4 ++--
>  cutils.c           |   50 ++++++++++++++------------------------------------
>  linux-aio.c        |    4 ++--
>  posix-aio-compat.c |    2 +-
>  qemu-common.h      |    4 +---
>  6 files changed, 22 insertions(+), 46 deletions(-)

> diff --git a/cutils.c b/cutils.c
> index af308cd..9451c86 100644
> --- a/cutils.c
> +++ b/cutils.c
> @@ -260,46 +260,24 @@ void qemu_iovec_from_buffer(QEMUIOVector *qiov, const 
> void *buf, size_t count)
>      }
>  }
>  
> -void qemu_iovec_memset(QEMUIOVector *qiov, int c, size_t count)
> +void qemu_iovec_memset(QEMUIOVector *qiov, size_t offset, int c, size_t 
> bytes)
>  {
> -    size_t n;
> +    struct iovec *iov = qiov->iov;
>      int i;
> +    assert(qiov->size >= offset);
> +    assert(qiov->size - offset >= bytes);
>  
> -    for (i = 0; i < qiov->niov && count; ++i) {
> -        n = MIN(count, qiov->iov[i].iov_len);
> -        memset(qiov->iov[i].iov_base, c, n);
> -        count -= n;
> +    /* first skip initial full-sized elements */
> +    for(i = 0; offset >= iov[i].iov_len; ++i) {
> +     offset -= iov[i].iov_len;
>      }

This doesn't check i < qiov->niov any more. It's probably safe because
you added the assertions above. I didn't check if the assertions can
trigger anywhere, but they do constitute an interface change.

Also, indentation is off (tabs instead of spaces).

> -}
> -
> -void qemu_iovec_memset_skip(QEMUIOVector *qiov, int c, size_t count,
> -                            size_t skip)
> -{
> -    int i;
> -    size_t done;
> -    void *iov_base;
> -    uint64_t iov_len;
> -
> -    done = 0;
> -    for (i = 0; (i < qiov->niov) && (done != count); i++) {
> -        if (skip >= qiov->iov[i].iov_len) {
> -            /* Skip the whole iov */
> -            skip -= qiov->iov[i].iov_len;
> -            continue;
> -        } else {
> -            /* Skip only part (or nothing) of the iov */
> -            iov_base = (uint8_t*) qiov->iov[i].iov_base + skip;
> -            iov_len = qiov->iov[i].iov_len - skip;
> -            skip = 0;
> -        }
> -
> -        if (done + iov_len > count) {
> -            memset(iov_base, c, count - done);
> -            break;
> -        } else {
> -            memset(iov_base, c, iov_len);
> -        }
> -        done += iov_len;
> +    /* skip/memset partial element and memset the rest */
> +    while(bytes) {
> +     size_t n = MIN(bytes, iov[i].iov_len - offset);
> +     memset((char*)iov[i].iov_base + offset, c, n);
> +     bytes -= n;
> +     ++i;
> +     offset = 0;
>      }
>  }

The memsetting logic looks correct, but again indentation is off.

Kevin