Il 16/03/2012 22:34, Michael Tokarev ha scritto: > This changes implementations of all iov_* > functions, completing the previous step. > > All iov_* functions now ensure that this offset > argument is within the iovec (using assertion), > but lets to specify `bytes' value larger than > actual length of the iovec - in this case they > stops at the actual end of iovec. It is also > suggested to use convinient `-1' value as `bytes' > to mean just this -- "up to the end". > > There's one very minor semantic change here: new > requiriment is that `offset' points to inside of > iovec. This is checked just at the end of functions > (assert()), it does not actually need to be enforced, > but using any of these functions with offset pointing > past the end of iovec is wrong anyway. > > Note: the new code in iov.c uses arithmetic with > void pointers. I thought this is not supported > everywhere and is a GCC extension (indeed, the C > standard does not define void arithmetic). However, > the original code already use void arith in > iov_from_buf() function: > (memcpy(..., buf + buf_off,...) > which apparently works well so far (it is this > way in qemu 1.0). So I left it this way and used > it in other places. > > While at it, add a unit-test file test-iov.c, > to check various corner cases with iov_from_buf() > and iov_to_buf() (all 3 functions uses exactly > the same logic so iov_memset() should be covered > too). > > Signed-off-by: Michael Tokarev <m...@tls.msk.ru> > --- > iov.c | 91 +++++++++++++++-------------------- > iov.h | 12 ++++- > test-iov.c | 158 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Makefile changes missing. Also, please use g_test_rand_int_range to generate random numbers inside GTester. The .xml output will include the seed so that failures are reproducible. http://developer.gnome.org/glib/2.30/glib-Testing.html#g-test-rand-int Paolo > 3 files changed, 207 insertions(+), 54 deletions(-) > > diff --git a/iov.c b/iov.c > index bc58cab..9657d28 100644 > --- a/iov.c > +++ b/iov.c > @@ -7,6 +7,7 @@ > * Author(s): > * Anthony Liguori <aligu...@us.ibm.com> > * Amit Shah <amit.s...@redhat.com> > + * Michael Tokarev <m...@tls.msk.ru> > * > * This work is licensed under the terms of the GNU GPL, version 2. See > * the COPYING file in the top-level directory. > @@ -17,75 +18,61 @@ > > #include "iov.h" > > -size_t iov_from_buf(struct iovec *iov, unsigned int iov_cnt, size_t iov_off, > - const void *buf, size_t size) > +size_t iov_from_buf(struct iovec *iov, unsigned int iov_cnt, > + size_t offset, const void *buf, size_t bytes) > { > - size_t iovec_off, buf_off; > + size_t done; > unsigned int i; > - > - iovec_off = 0; > - buf_off = 0; > - for (i = 0; i < iov_cnt && size; i++) { > - if (iov_off < (iovec_off + iov[i].iov_len)) { > - size_t len = MIN((iovec_off + iov[i].iov_len) - iov_off, size); > - > - memcpy(iov[i].iov_base + (iov_off - iovec_off), buf + buf_off, > len); > - > - buf_off += len; > - iov_off += len; > - size -= len; > + for (i = 0, done = 0; (offset || done < bytes) && i < iov_cnt; i++) { > + if (offset < iov[i].iov_len) { > + size_t len = MIN(iov[i].iov_len - offset, bytes - done); > + memcpy(iov[i].iov_base + offset, buf + done, len); > + done += len; > + offset = 0; > + } else { > + offset -= iov[i].iov_len; > } > - iovec_off += iov[i].iov_len; > } > - return buf_off; > + assert(offset == 0); > + return done; > } > > -size_t iov_to_buf(const struct iovec *iov, const unsigned int iov_cnt, > size_t iov_off, > - void *buf, size_t size) > +size_t iov_to_buf(const struct iovec *iov, const unsigned int iov_cnt, > + size_t offset, void *buf, size_t bytes) > { > - uint8_t *ptr; > - size_t iovec_off, buf_off; > + size_t done; > unsigned int i; > - > - ptr = buf; > - iovec_off = 0; > - buf_off = 0; > - for (i = 0; i < iov_cnt && size; i++) { > - if (iov_off < (iovec_off + iov[i].iov_len)) { > - size_t len = MIN((iovec_off + iov[i].iov_len) - iov_off , size); > - > - memcpy(ptr + buf_off, iov[i].iov_base + (iov_off - iovec_off), > len); > - > - buf_off += len; > - iov_off += len; > - size -= len; > + for (i = 0, done = 0; (offset || done < bytes) && i < iov_cnt; i++) { > + if (offset < iov[i].iov_len) { > + size_t len = MIN(iov[i].iov_len - offset, bytes - done); > + memcpy(buf + done, iov[i].iov_base + offset, len); > + done += len; > + offset = 0; > + } else { > + offset -= iov[i].iov_len; > } > - iovec_off += iov[i].iov_len; > } > - return buf_off; > + assert(offset == 0); > + return done; > } > > size_t iov_memset(const struct iovec *iov, const unsigned int iov_cnt, > - size_t iov_off, int fillc, size_t size) > + size_t offset, int fillc, size_t bytes) > { > - size_t iovec_off, buf_off; > + size_t done; > unsigned int i; > - > - iovec_off = 0; > - buf_off = 0; > - for (i = 0; i < iov_cnt && size; i++) { > - if (iov_off < (iovec_off + iov[i].iov_len)) { > - size_t len = MIN((iovec_off + iov[i].iov_len) - iov_off , size); > - > - memset(iov[i].iov_base + (iov_off - iovec_off), fillc, len); > - > - buf_off += len; > - iov_off += len; > - size -= len; > + for (i = 0, done = 0; (offset || done < bytes) && i < iov_cnt; i++) { > + if (offset < iov[i].iov_len) { > + size_t len = MIN(iov[i].iov_len - offset, bytes - done); > + memset(iov[i].iov_base + offset, fillc, len); > + done += len; > + offset = 0; > + } else { > + offset -= iov[i].iov_len; > } > - iovec_off += iov[i].iov_len; > } > - return buf_off; > + assert(offset == 0); > + return done; > } > > size_t iov_size(const struct iovec *iov, const unsigned int iov_cnt) > diff --git a/iov.h b/iov.h > index 0b4acf4..19ee3b3 100644 > --- a/iov.h > +++ b/iov.h > @@ -1,10 +1,11 @@ > /* > - * Helpers for getting linearized buffers from iov / filling buffers into > iovs > + * Helpers for using (partial) iovecs. > * > * Copyright (C) 2010 Red Hat, Inc. > * > * Author(s): > * Amit Shah <amit.s...@redhat.com> > + * Michael Tokarev <m...@tls.msk.ru> > * > * This work is licensed under the terms of the GNU GPL, version 2. See > * the COPYING file in the top-level directory. > @@ -28,6 +29,12 @@ size_t iov_size(const struct iovec *iov, const unsigned > int iov_cnt); > * only part of data will be copied, up to the end of the iovec. > * Number of bytes actually copied will be returned, which is > * min(bytes, iov_size(iov)-offset) > + * `Offset' must point to the inside of iovec. > + * It is okay to use very large value for `bytes' since we're > + * limited by the size of the iovec anyway, provided that the > + * buffer pointed to by buf has enough space. One possible > + * such "large" value is -1 (sinice size_t is unsigned), > + * so specifying `-1' as `bytes' means 'up to the end of iovec'. > */ > size_t iov_from_buf(struct iovec *iov, unsigned int iov_cnt, > size_t offset, const void *buf, size_t bytes); > @@ -37,11 +44,12 @@ size_t iov_to_buf(const struct iovec *iov, const unsigned > int iov_cnt, > /** > * Set data bytes pointed out by iovec `iov' of size `iov_cnt' elements, > * starting at byte offset `start', to value `fillc', repeating it > - * `bytes' number of times. > + * `bytes' number of times. `Offset' must point to the inside of iovec. > * If `bytes' is large enough, only last bytes portion of iovec, > * up to the end of it, will be filled with the specified value. > * Function return actual number of bytes processed, which is > * min(size, iov_size(iov) - offset). > + * Again, it is okay to use large value for `bytes' to mean "up to the end". > */ > size_t iov_memset(const struct iovec *iov, const unsigned int iov_cnt, > size_t offset, int fillc, size_t bytes); > diff --git a/test-iov.c b/test-iov.c > new file mode 100644 > index 0000000..fd8f9fe > --- /dev/null > +++ b/test-iov.c > @@ -0,0 +1,158 @@ > +#include <glib.h> > +#include "qemu-common.h" > +#include "iov.h" > + > +/* return a (pseudo-)random number in [min, max] range */ > + > +static inline int rannum(int min, int max) > +{ > + return min + random() % (max - min + 1); > +} > + > +/* create a randomly-sized iovec with random vectors */ > +static void iov_random(struct iovec **iovp, unsigned *iov_cntp) > +{ > + unsigned niov = rannum(3, 8); > + struct iovec *iov = g_malloc(niov * sizeof(*iov)); > + unsigned i; > + for (i = 0; i < niov; ++i) { > + iov[i].iov_len = rannum(2, 10); > + iov[i].iov_base = g_malloc(iov[i].iov_len); > + } > + *iovp = iov; > + *iov_cntp = niov; > +} > + > +static void iov_free(struct iovec *iov, unsigned niov) > +{ > + unsigned i; > + for (i = 0; i < niov; ++i) { > + g_free(iov[i].iov_base); > + } > + g_free(iov); > +} > + > +static void test_iov_bytes(struct iovec *iov, unsigned niov, > + size_t offset, size_t bytes) > +{ > + unsigned i; > + size_t j, o; > + unsigned char *b; > + o = 0; > + > + /* we walk over all elements, */ > + for (i = 0; i < niov; ++i) { > + b = iov[i].iov_base; > + /* over each char of each element, */ > + for (j = 0; j < iov[i].iov_len; ++j) { > + /* counting each of them and > + * verifying that the ones within [offset,offset+bytes) > + * range are equal to the position number (o) */ > + if (o >= offset && o < offset + bytes) { > + g_assert(b[j] == (o & 255)); > + } > + ++o; > + } > + } > +} > + > +static void test_to_from_buf_1(void) > +{ > + struct iovec *iov; > + unsigned niov; > + size_t sz; > + unsigned char *ibuf, *obuf; > + unsigned i, j, n; > + > + iov_random(&iov, &niov); > + sz = iov_size(iov, niov); > + > + ibuf = g_malloc(sz + 8) + 4; > + memcpy(ibuf-4, "aaaa", 4); memcpy(ibuf + sz, "bbbb", 4); > + obuf = g_malloc(sz + 8) + 4; > + memcpy(obuf-4, "xxxx", 4); memcpy(obuf + sz, "yyyy", 4); > + > + /* fill in ibuf with 0123456... */ > + for (i = 0; i < sz; ++i) { > + ibuf[i] = i & 255; > + } > + > + for (i = 0; i <= sz; ++i) { > + > + /* Test from/to buf for offset(i) in [0..sz] up to the end of > buffer. > + * For last iteration with offset == sz, the procedure should > + * skip whole vector and process exactly 0 bytes */ > + > + /* first set bytes [i..sz) to some "random" value */ > + n = iov_memset(iov, niov, 0, i&255, -1); > + g_assert(n == sz); > + > + /* next copy bytes [i..sz) from ibuf to iovec */ > + n = iov_from_buf(iov, niov, i, ibuf + i, -1); > + g_assert(n == sz - i); > + > + /* clear part of obuf */ > + memset(obuf + i, 0, sz - i); > + /* and set this part of obuf to values from iovec */ > + n = iov_to_buf(iov, niov, i, obuf + i, -1); > + g_assert(n == sz - i); > + > + /* now compare resulting buffers */ > + g_assert(memcmp(ibuf, obuf, sz) == 0); > + > + /* test just one char */ > + n = iov_to_buf(iov, niov, i, obuf + i, 1); > + g_assert(n == (i < sz)); > + if (n) { > + g_assert(obuf[i] == (i & 255)); > + } > + > + for (j = i; j <= sz; ++j) { > + /* now test num of bytes cap up to byte no. j, > + * with j in [i..sz]. */ > + > + /* clear iovec */ > + n = iov_memset(iov, niov, 0, j&255, -1); > + g_assert(n == sz); > + > + /* copy bytes [i..j) from ibuf to iovec */ > + n = iov_from_buf(iov, niov, i, ibuf + i, j - i); > + g_assert(n == j - i); > + > + /* clear part of obuf */ > + memset(obuf + i, 0, j - i); > + > + /* copy bytes [i..j) from iovec to obuf */ > + n = iov_to_buf(iov, niov, i, obuf + i, j - i); > + g_assert(n == j - i); > + > + /* verify result */ > + g_assert(memcmp(ibuf, obuf, sz) == 0); > + > + /* now actually check if the iovec contains the right data */ > + test_iov_bytes(iov, niov, i, j - i); > + } > + } > + g_assert(!memcmp(ibuf-4, "aaaa", 4) && !memcmp(ibuf+sz, "bbbb", 4)); > + g_free(ibuf-4); > + g_assert(!memcmp(obuf-4, "xxxx", 4) && !memcmp(obuf+sz, "yyyy", 4)); > + g_free(obuf-4); > + iov_free(iov, niov); > +} > + > +static void test_to_from_buf(void) > +{ > + int x; > + /* repeat it several times with different (random) values */ > + for (x = 0; x < 4; ++x) { > + test_to_from_buf_1(); > + } > +} > + > +int main(int argc, char **argv) > +{ > + srandom(getpid()); > + g_test_init(&argc, &argv, NULL); > + g_test_add_func("/basic/iov/from-to-buf", test_to_from_buf); > + return g_test_run(); > +}
