| Message ID | 1343814538-27591-2-git-send-email-lilei@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
Lei Li <lilei@linux.vnet.ibm.com> writes: > Signed-off-by: Lei Li <lilei@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 c2aaaee..087c92d 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -2517,38 +2517,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; Probably should move the buffer into a separate structure and then you can drop the cbuf_ prefixes. > +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; > +} > + Typically, you would use a producer and a consumer index. To test for empty, you would check if (consumer == producer). To check for full, you would check if ((producer - consumer) == size). To get the actual index, you always modulus the indexes with size. This only works if size is a power of 2 but that's a reasonable restriction. > 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; > } > > - 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; Doing a mempcy() like this may work, but seems inefficient to me. I think reading like a ring queue works a bit nicer. > + } else { > + /* Completely overwrite */ > + if (mem_chr_is_full(chr)) { > + d->cbuf_out = (d->cbuf_out + len) % d->cbuf_capacity; > + } else { > + /* Enough cbuf to write */ > + memcpy(d->cbuf + d->cbuf_in, buf, len); > + d->cbuf_count += len; > + } Looks like indenting is off here. Regards, Anthony Liguori > + } > + > + 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) > { > 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; > @@ -2558,7 +2616,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()! */ > @@ -2566,7 +2624,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; > @@ -2575,7 +2633,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); > -- > 1.7.7.6
On 08/02/2012 05:30 AM, Anthony Liguori wrote: > Lei Li <lilei@linux.vnet.ibm.com> writes: > >> Signed-off-by: Lei Li <lilei@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 c2aaaee..087c92d 100644 >> --- a/qemu-char.c >> +++ b/qemu-char.c >> @@ -2517,38 +2517,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; > Probably should move the buffer into a separate structure and then you > can drop the cbuf_ prefixes. > >> +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; >> +} >> + > Typically, you would use a producer and a consumer index. To test for > empty, you would check if (consumer == producer). > > To check for full, you would check if ((producer - consumer) == size). > > To get the actual index, you always modulus the indexes with size. This > only works if size is a power of 2 but that's a reasonable restriction. > >> 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; >> } >> >> - 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; > Doing a mempcy() like this may work, but seems inefficient to me. I > think reading like a ring queue works a bit nicer. Hi Anthony, What do you mean "reading like a ring queue"? I am a little confused here. Could you please give more details? And thanks for your suggestions. :) >> + } else { >> + /* Completely overwrite */ >> + if (mem_chr_is_full(chr)) { >> + d->cbuf_out = (d->cbuf_out + len) % d->cbuf_capacity; >> + } else { >> + /* Enough cbuf to write */ >> + memcpy(d->cbuf + d->cbuf_in, buf, len); >> + d->cbuf_count += len; >> + } > Looks like indenting is off here. > > Regards, > > Anthony Liguori > >> + } >> + >> + 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) >> { >> 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; >> @@ -2558,7 +2616,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()! */ >> @@ -2566,7 +2624,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; >> @@ -2575,7 +2633,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); >> -- >> 1.7.7.6
diff --git a/qemu-char.c b/qemu-char.c index c2aaaee..087c92d 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2517,38 +2517,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; +} + 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; } - 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 */ + memcpy(d->cbuf + d->cbuf_in, buf, len); + d->cbuf_count += len; + } + } + + 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) { 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; @@ -2558,7 +2616,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()! */ @@ -2566,7 +2624,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; @@ -2575,7 +2633,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);
Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com> --- qemu-char.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++----------- qemu-char.h | 2 +- 2 files changed, 78 insertions(+), 20 deletions(-)