diff mbox

[PATCHv2,1/7] Consolidate qemu_iovec_memset{, _skip}() into single, simplified function

Message ID 1331430564-32745-2-git-send-email-mjt@msgid.tls.msk.ru
State New
Headers show

Commit Message

Michael Tokarev March 11, 2012, 1:49 a.m. UTC 
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 <mjt@tls.msk.ru>
---
 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(-)

Comments

Kevin Wolf March 12, 2012, 1:55 p.m. UTC | #1 
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 <mjt@tls.msk.ru>
> ---
>  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
diff mbox

Patch

diff --git a/block/qcow2.c b/block/qcow2.c
index eb5ea48..6d11bc0 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -406,7 +406,7 @@  int qcow2_backing_read1(BlockDriverState *bs, QEMUIOVector *qiov,
     else
         n1 = bs->total_sectors - sector_num;
 
-    qemu_iovec_memset_skip(qiov, 0, 512 * (nb_sectors - n1), 512 * n1);
+    qemu_iovec_memset(qiov, 512 * n1, 0, 512 * (nb_sectors - n1));
 
     return n1;
 }
@@ -466,7 +466,7 @@  static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num,
                 }
             } else {
                 /* Note: in this case, no need to wait */
-                qemu_iovec_memset(&hd_qiov, 0, 512 * cur_nr_sectors);
+		qemu_iovec_memset(&hd_qiov, 0, 0, 512 * cur_nr_sectors);
             }
         } else if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
             /* add AIO support for compressed blocks ? */
diff --git a/block/qed.c b/block/qed.c
index a041d31..6f9325b 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -738,7 +738,7 @@  static void qed_read_backing_file(BDRVQEDState *s, uint64_t pos,
     /* Zero all sectors if reading beyond the end of the backing file */
     if (pos >= backing_length ||
         pos + qiov->size > backing_length) {
-        qemu_iovec_memset(qiov, 0, qiov->size);
+        qemu_iovec_memset(qiov, 0, 0, qiov->size);
     }
 
     /* Complete now if there are no backing file sectors to read */
@@ -1253,7 +1253,7 @@  static void qed_aio_read_data(void *opaque, int ret,
 
     /* Handle zero cluster and backing file reads */
     if (ret == QED_CLUSTER_ZERO) {
-        qemu_iovec_memset(&acb->cur_qiov, 0, acb->cur_qiov.size);
+        qemu_iovec_memset(&acb->cur_qiov, 0, 0, acb->cur_qiov.size);
         qed_aio_next_io(acb, 0);
         return;
     } else if (ret != QED_CLUSTER_FOUND) {
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;
     }
-}
-
-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;
     }
 }
 
diff --git a/linux-aio.c b/linux-aio.c
index d2fc2e7..5a46c13 100644
--- a/linux-aio.c
+++ b/linux-aio.c
@@ -64,8 +64,8 @@  static void qemu_laio_process_completion(struct qemu_laio_state *s,
         } else if (ret >= 0) {
             /* Short reads mean EOF, pad with zeros. */
             if (laiocb->is_read) {
-                qemu_iovec_memset_skip(laiocb->qiov, 0,
-                    laiocb->qiov->size - ret, ret);
+                qemu_iovec_memset(laiocb->qiov, ret, 0,
+                    laiocb->qiov->size - ret);
             } else {
                 ret = -EINVAL;
             }
diff --git a/posix-aio-compat.c b/posix-aio-compat.c
index d311d13..f8efe98 100644
--- a/posix-aio-compat.c
+++ b/posix-aio-compat.c
@@ -355,7 +355,7 @@  static void *aio_thread(void *unused)
 
                 qemu_iovec_init_external(&qiov, aiocb->aio_iov,
                                          aiocb->aio_niov);
-                qemu_iovec_memset_skip(&qiov, 0, aiocb->aio_nbytes - ret, ret);
+                qemu_iovec_memset(&qiov, ret, 0, aiocb->aio_nbytes - ret);
 
                 ret = aiocb->aio_nbytes;
             }
diff --git a/qemu-common.h b/qemu-common.h
index dbfce6f..a1ff126 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -340,9 +340,7 @@  void qemu_iovec_destroy(QEMUIOVector *qiov);
 void qemu_iovec_reset(QEMUIOVector *qiov);
 void qemu_iovec_to_buffer(QEMUIOVector *qiov, void *buf);
 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_skip(QEMUIOVector *qiov, int c, size_t count,
-                            size_t skip);
+void qemu_iovec_memset(QEMUIOVector *qiov, size_t offset, int c, size_t bytes);
 
 bool buffer_is_zero(const void *buf, size_t len);