[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 31/34] qapi: make string output visitor parse
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH v3 31/34] qapi: make string output visitor parse int list |
Date: |
Wed, 26 Mar 2014 11:48:54 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 |
Il 26/03/2014 11:37, address@hidden ha scritto:
> Signed-off-by: Hu Tao <address@hidden>
Just a small comment below.
> ---
> qapi/string-output-visitor.c | 236
> +++++++++++++++++++++++++++++++++++--
> tests/test-string-output-visitor.c | 35 ++++++
> 2 files changed, 260 insertions(+), 11 deletions(-)
>
> diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
> index fb1d2e8..edfc0a4 100644
> --- a/qapi/string-output-visitor.c
> +++ b/qapi/string-output-visitor.c
> @@ -16,32 +16,177 @@
> #include "qapi/qmp/qerror.h"
> #include "qemu/host-utils.h"
> #include <math.h>
> +#include "qemu/range.h"
> +
> +enum ListMode {
> + LM_NONE, /* not traversing a list of repeated options */
> + LM_STARTED, /* start_list() succeeded */
> +
> + LM_IN_PROGRESS, /* next_list() has been called.
> + *
> + * Generating the next list link will consume the
> most
> + * recently parsed QemuOpt instance of the repeated
> + * option.
> + *
> + * Parsing a value into the list link will examine
> the
> + * next QemuOpt instance of the repeated option, and
> + * possibly enter LM_SIGNED_INTERVAL or
> + * LM_UNSIGNED_INTERVAL.
> + */
> +
> + LM_SIGNED_INTERVAL, /* next_list() has been called.
> + *
> + * Generating the next list link will consume the
> most
> + * recently stored element from the signed interval,
> + * parsed from the most recent QemuOpt instance of
> the
> + * repeated option. This may consume QemuOpt itself
> + * and return to LM_IN_PROGRESS.
> + *
> + * Parsing a value into the list link will store the
> + * next element of the signed interval.
> + */
> +
> + LM_UNSIGNED_INTERVAL,/* Same as above, only for an unsigned interval. */
> +
> + LM_END
> +};
> +
> +typedef enum ListMode ListMode;
>
> struct StringOutputVisitor
> {
> Visitor visitor;
> bool human;
> - char *string;
> + GString *string;
> + bool head;
> + ListMode list_mode;
> + union {
> + int64_t s;
> + uint64_t u;
> + } range_start, range_end;
> + SignedRangeList *ranges;
> };
>
> static void string_output_set(StringOutputVisitor *sov, char *string)
> {
> - g_free(sov->string);
> - sov->string = string;
> + if (sov->string) {
> + g_string_free(sov->string, true);
> + }
> + sov->string = g_string_new(string);
> + g_free(string);
> +}
> +
> +static void string_output_append(StringOutputVisitor *sov, int64_t a)
> +{
> + range_list_add(sov->ranges, a, 1);
> +}
> +
> +static void string_output_append_range(StringOutputVisitor *sov,
> + int64_t s, int64_t e)
> +{
> + range_list_add(sov->ranges, s, e);
> }
>
> static void print_type_int(Visitor *v, int64_t *obj, const char *name,
> Error **errp)
> {
> StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
> - char *out;
> + SignedRange *r;
>
> - if (sov->human) {
> - out = g_strdup_printf("%lld (%#llx)", (long long) *obj, (long long)
> *obj);
> - } else {
> - out = g_strdup_printf("%lld", (long long) *obj);
> + if (!sov->ranges) {
> + sov->ranges = g_malloc0(sizeof(*sov->ranges));
> + QTAILQ_INIT(sov->ranges);
> + }
> +
> + switch (sov->list_mode) {
> + case LM_NONE:
> + string_output_append(sov, *obj);
> + goto format_string;
> + break;
> +
No need for the goto if...
> + case LM_STARTED:
> + sov->range_start.s = *obj;
> + sov->range_end.s = *obj;
> + sov->list_mode = LM_IN_PROGRESS;
> + break;
... you have "return;" here
> + case LM_IN_PROGRESS:
> + if (sov->range_end.s + 1 == *obj) {
> + sov->range_end.s++;
> + break;
(unnecessary break)
> + } else {
> + if (sov->range_start.s == sov->range_end.s) {
> + string_output_append(sov, sov->range_end.s);
> + } else {
> + assert(sov->range_start.s < sov->range_end.s);
> + string_output_append_range(sov, sov->range_start.s,
> + sov->range_end.s -
> + sov->range_start.s + 1);
> + }
> +
> + sov->range_start.s = *obj;
> + sov->range_end.s = *obj;
> + }
> + break;
... and "return;" here...
> + case LM_END:
> + if (sov->range_end.s + 1 == *obj) {
> + sov->range_end.s++;
> + assert(sov->range_start.s < sov->range_end.s);
> + string_output_append_range(sov, sov->range_start.s,
> + sov->range_end.s -
> + sov->range_start.s + 1);
> + } else {
> + if (sov->range_start.s == sov->range_end.s) {
> + string_output_append(sov, sov->range_end.s);
> + } else {
> + assert(sov->range_start.s < sov->range_end.s);
> +
> + string_output_append_range(sov, sov->range_start.s,
> + sov->range_end.s -
> + sov->range_start.s + 1);
> + }
> + string_output_append(sov, *obj);
> + }
... and "break;" here" and move the formatting code outside the "switch".
> +format_string:
> + QTAILQ_FOREACH(r, sov->ranges, entry) {
> + if (r->length > 1) {
> + g_string_append_printf(sov->string, "%" PRId64 "-%" PRId64,
> + r->start,
> + s_range_end(r->start, r->length));
> + } else {
> + g_string_append_printf(sov->string, "%" PRId64,
> + r->start);
> + }
> + if (r->entry.tqe_next) {
> + g_string_append(sov->string, ",");
> + }
> + }
> +
> + if (sov->human) {
> + g_string_append(sov->string, " (");
> + QTAILQ_FOREACH(r, sov->ranges, entry) {
> + if (r->length > 1) {
> + g_string_append_printf(sov->string, "%" PRIx64 "-%"
> PRIx64,
> + r->start,
> + s_range_end(r->start, r->length));
> + } else {
> + g_string_append_printf(sov->string, "%" PRIx64,
> + r->start);
> + }
> + if (r->entry.tqe_next) {
> + g_string_append(sov->string, ",");
> + }
> + }
> + g_string_append(sov->string, ")");
> + }
> +
> + break;
> +
> + default:
> + abort();
> }
> - string_output_set(sov, out);
> }
>
> static void print_type_size(Visitor *v, uint64_t *obj, const char *name,
> @@ -103,9 +248,61 @@ static void print_type_number(Visitor *v, double *obj,
> const char *name,
> string_output_set(sov, g_strdup_printf("%f", *obj));
> }
>
> +static void
> +start_list(Visitor *v, const char *name, Error **errp)
> +{
> + StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
> +
> + /* we can't traverse a list in a list */
> + assert(sov->list_mode == LM_NONE);
> + sov->list_mode = LM_STARTED;
> + sov->head = true;
> +}
> +
> +static GenericList *
> +next_list(Visitor *v, GenericList **list, Error **errp)
> +{
> + StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
> + GenericList *ret = NULL;
> + if (*list) {
> + if (sov->head) {
> + ret = *list;
> + } else {
> + ret = (*list)->next;
> + }
> +
> + if (sov->head) {
> + if (ret && ret->next == NULL) {
> + sov->list_mode = LM_NONE;
> + }
> + sov->head = false;
> + } else {
> + if (ret && ret->next == NULL) {
> + sov->list_mode = LM_END;
> + }
> + }
> + }
> +
> + return ret;
> +}
> +
> +static void
> +end_list(Visitor *v, Error **errp)
> +{
> + StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
> +
> + assert(sov->list_mode == LM_STARTED ||
> + sov->list_mode == LM_END ||
> + sov->list_mode == LM_NONE ||
> + sov->list_mode == LM_IN_PROGRESS);
> + sov->list_mode = LM_NONE;
> + sov->head = true;
> +
> +}
> +
> char *string_output_get_string(StringOutputVisitor *sov)
> {
> - char *string = sov->string;
> + char *string = g_string_free(sov->string, false);
> sov->string = NULL;
> return string;
> }
> @@ -117,7 +314,20 @@ Visitor *string_output_get_visitor(StringOutputVisitor
> *sov)
>
> void string_output_visitor_cleanup(StringOutputVisitor *sov)
> {
> - g_free(sov->string);
> + SignedRange *r, *next;
> +
> + if (sov->string) {
> + g_string_free(sov->string, true);
> + }
> +
> + if (sov->ranges) {
> + QTAILQ_FOREACH_SAFE(r, sov->ranges, entry, next) {
> + QTAILQ_REMOVE(sov->ranges, r, entry);
> + s_range_free(r);
> + }
> + g_free(sov->ranges);
> + }
> +
> g_free(sov);
> }
>
> @@ -127,6 +337,7 @@ StringOutputVisitor *string_output_visitor_new(bool human)
>
> v = g_malloc0(sizeof(*v));
>
> + v->string = g_string_new(NULL);
> v->human = human;
> v->visitor.type_enum = output_type_enum;
> v->visitor.type_int = print_type_int;
> @@ -134,6 +345,9 @@ StringOutputVisitor *string_output_visitor_new(bool human)
> v->visitor.type_bool = print_type_bool;
> v->visitor.type_str = print_type_str;
> v->visitor.type_number = print_type_number;
> + v->visitor.start_list = start_list;
> + v->visitor.next_list = next_list;
> + v->visitor.end_list = end_list;
>
> return v;
> }
> diff --git a/tests/test-string-output-visitor.c
> b/tests/test-string-output-visitor.c
> index 22363d1..a460377 100644
> --- a/tests/test-string-output-visitor.c
> +++ b/tests/test-string-output-visitor.c
> @@ -57,6 +57,39 @@ static void test_visitor_out_int(TestOutputVisitorData
> *data,
> g_free(str);
> }
>
> +static void test_visitor_out_intList(TestOutputVisitorData *data,
> + const void *unused)
> +{
> + int64_t value[] = {-10, -7, -2, -1, 0, 1, 9, 10, 16, 15, 14,
> + 3, 4, 5, 6, 11, 12, 13, 21, 22, INT64_MAX - 1, INT64_MAX};
> + intList *list = NULL, **tmp = &list;
> + int i;
> + Error *errp = NULL;
> + char *str;
> +
> + for (i = 0; i < sizeof(value) / sizeof(value[0]); i++) {
> + *tmp = g_malloc0(sizeof(**tmp));
> + (*tmp)->value = value[i];
> + tmp = &(*tmp)->next;
> + }
> +
> + visit_type_intList(data->ov, &list, NULL, &errp);
> + g_assert(error_is_set(&errp) == 0);
> +
> + str = string_output_get_string(data->sov);
> + g_assert(str != NULL);
> + g_assert_cmpstr(str, ==,
> +
> "-10,-7,-2-1,3-6,9-16,21-22,9223372036854775806-9223372036854775807");
> + g_free(str);
> +#if 0
> + while (list) {
> + tmp2 = list->next;
> + g_free(list);
> + list = tmp2;
> + }
> +#endif
> +}
> +
> static void test_visitor_out_bool(TestOutputVisitorData *data,
> const void *unused)
> {
> @@ -182,6 +215,8 @@ int main(int argc, char **argv)
> &out_visitor_data, test_visitor_out_enum);
> output_visitor_test_add("/string-visitor/output/enum-errors",
> &out_visitor_data, test_visitor_out_enum_errors);
> + output_visitor_test_add("/string-visitor/output/intList",
> + &out_visitor_data, test_visitor_out_intList);
>
> g_test_run();
>
>
- [Qemu-devel] [PATCH v3 24/34] hostmem: allow preallocation of any memory region, (continued)
- [Qemu-devel] [PATCH v3 24/34] hostmem: allow preallocation of any memory region, address@hidden, 2014/03/26
- [Qemu-devel] [PATCH v3 27/34] hostmem: add properties for NUMA memory policy, address@hidden, 2014/03/26
- [Qemu-devel] [PATCH v3 11/34] qmp: improve error reporting for -object and object-add, address@hidden, 2014/03/26
- [Qemu-devel] [PATCH v3 32/34] qom: introduce object_property_get_enum and object_property_get_uint16List, address@hidden, 2014/03/26
- [Qemu-devel] [PATCH v3 20/34] memory: move RAM_PREALLOC_MASK to exec.c, rename, address@hidden, 2014/03/26
- [Qemu-devel] [PATCH v3 25/34] hostmem: add property to map memory with MAP_SHARED, address@hidden, 2014/03/26
- [Qemu-devel] [PATCH v3 23/34] hostmem: add merge and dump properties, address@hidden, 2014/03/26
- [Qemu-devel] [PATCH v3 34/34] hmp: add info memdev, address@hidden, 2014/03/26
- [Qemu-devel] [PATCH v3 28/34] hw: switch all boards to use memory_region_allocate_system_memory, address@hidden, 2014/03/26
- [Qemu-devel] [PATCH v3 31/34] qapi: make string output visitor parse int list, address@hidden, 2014/03/26
- Re: [Qemu-devel] [PATCH v3 31/34] qapi: make string output visitor parse int list,
Paolo Bonzini <=
- [Qemu-devel] [PATCH v3 33/34] qmp: add query-memdev, address@hidden, 2014/03/26
- [Qemu-devel] [PATCH v3 06/34] man: improve -numa doc, address@hidden, 2014/03/26
- [Qemu-devel] [PATCH v3 19/34] memory: move preallocation code out of exec.c, address@hidden, 2014/03/26
- [Qemu-devel] [PATCH v3 29/34] Introduce signed range., address@hidden, 2014/03/26
- [Qemu-devel] [PATCH v3 13/34] numa: introduce memory_region_allocate_system_memory, address@hidden, 2014/03/26
- [Qemu-devel] [PATCH v3 04/34] NUMA: convert -numa option to use OptsVisitor, address@hidden, 2014/03/26
- [Qemu-devel] [PATCH v3 30/34] qapi: make string input visitor parse int list, address@hidden, 2014/03/26
- [Qemu-devel] [PATCH v3 07/34] qemu-option: introduce qemu_find_opts_singleton, address@hidden, 2014/03/26
- [Qemu-devel] [PATCH v3 17/34] memory: move mem_path handling to memory_region_allocate_system_memory, address@hidden, 2014/03/26