On Thu, 23 Aug 2012 13:14:21 +0800 Lei Li <li...@linux.vnet.ibm.com> wrote:
> Signed-off-by: Lei Li <li...@linux.vnet.ibm.com> > --- > qemu-char.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++----------- > qemu-char.h | 2 +- > 2 files changed, 78 insertions(+), 20 deletions(-) > > diff --git a/qemu-char.c b/qemu-char.c > index 398baf1..b21b93a 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -2528,38 +2528,96 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts > *opts) > /***********************************************************/ > /* Memory chardev */ > typedef struct { > - size_t outbuf_size; > - size_t outbuf_capacity; > - uint8_t *outbuf; > + size_t cbuf_capacity; > + size_t cbuf_in; > + size_t cbuf_out; > + size_t cbuf_count; > + uint8_t *cbuf; > } MemoryDriver; > > +static int mem_chr_is_empty(CharDriverState *chr) > +{ > + MemoryDriver *d = chr->opaque; > + > + return d->cbuf_count == 0; > +} > + > +static int mem_chr_is_full(CharDriverState *chr) > +{ > + MemoryDriver *d = chr->opaque; > + > + return d->cbuf_count == d->cbuf_capacity; > +} Please, make them return a bool and chr can be const. > + > static int mem_chr_write(CharDriverState *chr, const uint8_t *buf, int len) > { > MemoryDriver *d = chr->opaque; > + int left; > > - /* TODO: the QString implementation has the same code, we should > - * introduce a generic way to do this in cutils.c */ > - if (d->outbuf_capacity < d->outbuf_size + len) { > - /* grow outbuf */ > - d->outbuf_capacity += len; > - d->outbuf_capacity *= 2; > - d->outbuf = g_realloc(d->outbuf, d->outbuf_capacity); > + if (d->cbuf_capacity < len) { > + return -1; > } This is the first time I look at a circular buffer implementation, but I'd expect this too work: ie. you just write the last bytes that fit on the buffer. > > - memcpy(d->outbuf + d->outbuf_size, buf, len); > - d->outbuf_size += len; > + left = d->cbuf_capacity - d->cbuf_count % d->cbuf_capacity; > + > + /* Some of cbuf need to be overwrited */ > + if (left < len) { > + memcpy(d->cbuf + d->cbuf_in, buf, left); > + memcpy(d->cbuf + d->cbuf_out, buf + left, len - left); > + d->cbuf_out = (d->cbuf_out + len - left) % d->cbuf_capacity; > + d->cbuf_count = d->cbuf_count + left; > + } else { > + /* Completely overwrite */ > + if (mem_chr_is_full(chr)) { > + d->cbuf_out = (d->cbuf_out + len) % d->cbuf_capacity; > + } else { > + /* Enough cbuf to write */ > + d->cbuf_count += len; > + } > + memcpy(d->cbuf + d->cbuf_in, buf, len); > + } Couldn't this be made simpler by having a pointer to d->cbuf that points to where we are, then we just made that pointer points to the beginning of the buffer every time we cross its end? Just an idea. > + > + d->cbuf_in = (d->cbuf_in + len) % d->cbuf_capacity; > > return len; > } > > -void qemu_chr_init_mem(CharDriverState *chr) > +static void mem_chr_read(CharDriverState *chr, uint8_t *buf, int len) > +{ > + MemoryDriver *d = chr->opaque; > + int left; > + > + if (mem_chr_is_empty(chr)) { > + return; > + } > + > + left = d->cbuf_capacity - d->cbuf_count % d->cbuf_capacity; > + > + if (d->cbuf_capacity < len) { > + len = d->cbuf_capacity; > + } > + > + if (left < len) { > + memcpy(buf, d->cbuf + d->cbuf_out, left); > + memcpy(buf + left, d->cbuf + d->cbuf_out + left, len - left); > + } else { > + memcpy(buf, d->cbuf + d->cbuf_out, len); > + } > + > + d->cbuf_out = (d->cbuf_out + len) % d->cbuf_capacity; > + d->cbuf_count -= len; > +} > + > +void qemu_chr_init_mem(CharDriverState *chr, size_t size) Won't this break bisect? > { > MemoryDriver *d; > > d = g_malloc(sizeof(*d)); > - d->outbuf_size = 0; > - d->outbuf_capacity = 4096; > - d->outbuf = g_malloc0(d->outbuf_capacity); > + d->cbuf_capacity = size; > + d->cbuf_in = 0; > + d->cbuf_out = 0; > + d->cbuf_count = 0; > + d->cbuf = g_malloc0(d->cbuf_capacity); > > memset(chr, 0, sizeof(*chr)); > chr->opaque = d; > @@ -2569,7 +2627,7 @@ void qemu_chr_init_mem(CharDriverState *chr) > QString *qemu_chr_mem_to_qs(CharDriverState *chr) > { > MemoryDriver *d = chr->opaque; > - return qstring_from_substr((char *) d->outbuf, 0, d->outbuf_size - 1); > + return qstring_from_substr((char *) d->cbuf, 0, d->cbuf_count - 1); > } > > /* NOTE: this driver can not be closed with qemu_chr_delete()! */ > @@ -2577,7 +2635,7 @@ void qemu_chr_close_mem(CharDriverState *chr) > { > MemoryDriver *d = chr->opaque; > > - g_free(d->outbuf); > + g_free(d->cbuf); > g_free(chr->opaque); > chr->opaque = NULL; > chr->chr_write = NULL; > @@ -2586,7 +2644,7 @@ void qemu_chr_close_mem(CharDriverState *chr) > size_t qemu_chr_mem_osize(const CharDriverState *chr) > { > const MemoryDriver *d = chr->opaque; > - return d->outbuf_size; > + return d->cbuf_count; > } > > QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename) > diff --git a/qemu-char.h b/qemu-char.h > index 486644b..d8d90cc 100644 > --- a/qemu-char.h > +++ b/qemu-char.h > @@ -243,7 +243,7 @@ CharDriverState *qemu_chr_open_eventfd(int eventfd); > extern int term_escape_char; > > /* memory chardev */ > -void qemu_chr_init_mem(CharDriverState *chr); > +void qemu_chr_init_mem(CharDriverState *chr, size_t size); > void qemu_chr_close_mem(CharDriverState *chr); > QString *qemu_chr_mem_to_qs(CharDriverState *chr); > size_t qemu_chr_mem_osize(const CharDriverState *chr);