Lei Li <li...@linux.vnet.ibm.com> writes:
> 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 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
