[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC 1/6] OptsVisitor: introduce basic list modes
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [RFC 1/6] OptsVisitor: introduce basic list modes |
Date: |
Thu, 18 Jul 2013 16:53:48 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 |
Il 18/07/2013 15:59, Laszlo Ersek ha scritto:
> We're going to need more state while processing a list of repeated
> options. This change eliminates "repeated_opts_first" and adds a new state
> variable:
>
> list_mode repeated_opts repeated_opts_first
> -------------- ------------- -------------------
> LM_NONE NULL false
> LM_STARTED non-NULL true
> LM_IN_PROGRESS non-NULL false
>
> Additionally, it is documented that lookup_scalar() and processed(), both
> called by opts_type_XXX(), are invalid in LM_STARTED -- generated qapi
> code calls opts_next_list() to allocate the very first link before trying
> to parse a scalar into it. List mode restrictions are expressed in
> positive / inclusive form.
>
> Signed-off-by: Laszlo Ersek <address@hidden>
> ---
> qapi/opts-visitor.c | 43 ++++++++++++++++++++++++++++++++++---------
> 1 files changed, 34 insertions(+), 9 deletions(-)
>
> diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
> index 174bd8b..ab9a602 100644
> --- a/qapi/opts-visitor.c
> +++ b/qapi/opts-visitor.c
> @@ -18,6 +18,15 @@
> #include "qapi/visitor-impl.h"
>
>
> +enum ListMode
> +{
> + LM_NONE, /* not traversing a list of repeated options */
> + LM_STARTED, /* opts_start_list() succeeded */
> + LM_IN_PROGRESS /* opts_next_list() has been called */
> +};
> +
> +typedef enum ListMode ListMode;
> +
> struct OptsVisitor
> {
> Visitor visitor;
> @@ -35,8 +44,8 @@ struct OptsVisitor
> /* The list currently being traversed with opts_start_list() /
> * opts_next_list(). The list must have a struct element type in the
> * schema, with a single mandatory scalar member. */
> + ListMode list_mode;
> GQueue *repeated_opts;
> - bool repeated_opts_first;
>
> /* If "opts_root->id" is set, reinstantiate it as a fake QemuOpt for
> * uniformity. Only its "name" and "str" fields are set. "fake_id_opt"
> does
> @@ -156,9 +165,11 @@ opts_start_list(Visitor *v, const char *name, Error
> **errp)
> OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
>
> /* we can't traverse a list in a list */
> - assert(ov->repeated_opts == NULL);
> + assert(ov->list_mode == LM_NONE);
> ov->repeated_opts = lookup_distinct(ov, name, errp);
> - ov->repeated_opts_first = (ov->repeated_opts != NULL);
> + if (ov->repeated_opts != NULL) {
> + ov->list_mode = LM_STARTED;
> + }
> }
>
>
> @@ -168,10 +179,13 @@ opts_next_list(Visitor *v, GenericList **list, Error
> **errp)
> OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
> GenericList **link;
>
> - if (ov->repeated_opts_first) {
> - ov->repeated_opts_first = false;
> + switch (ov->list_mode) {
> + case LM_STARTED:
> + ov->list_mode = LM_IN_PROGRESS;
> link = list;
> - } else {
> + break;
> +
> + case LM_IN_PROGRESS: {
> const QemuOpt *opt;
>
> opt = g_queue_pop_head(ov->repeated_opts);
> @@ -180,6 +194,11 @@ opts_next_list(Visitor *v, GenericList **list, Error
> **errp)
> return NULL;
> }
> link = &(*list)->next;
> + break;
> + }
> +
> + default:
> + assert(0);
"abort()" to avoid excessively punishing folks who use -DNDEBUG (and
also to ensure they do not get extra compiler warnings).
Otherwise looks good!
Paolo
> }
>
> *link = g_malloc0(sizeof **link);
> @@ -192,14 +211,16 @@ opts_end_list(Visitor *v, Error **errp)
> {
> OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
>
> + assert(ov->list_mode == LM_STARTED || ov->list_mode == LM_IN_PROGRESS);
> ov->repeated_opts = NULL;
> + ov->list_mode = LM_NONE;
> }
>
>
> static const QemuOpt *
> lookup_scalar(const OptsVisitor *ov, const char *name, Error **errp)
> {
> - if (ov->repeated_opts == NULL) {
> + if (ov->list_mode == LM_NONE) {
> GQueue *list;
>
> /* the last occurrence of any QemuOpt takes effect when queried by
> name
> @@ -207,6 +228,7 @@ lookup_scalar(const OptsVisitor *ov, const char *name,
> Error **errp)
> list = lookup_distinct(ov, name, errp);
> return list ? g_queue_peek_tail(list) : NULL;
> }
> + assert(ov->list_mode == LM_IN_PROGRESS);
> return g_queue_peek_head(ov->repeated_opts);
> }
>
> @@ -214,9 +236,12 @@ lookup_scalar(const OptsVisitor *ov, const char *name,
> Error **errp)
> static void
> processed(OptsVisitor *ov, const char *name)
> {
> - if (ov->repeated_opts == NULL) {
> + if (ov->list_mode == LM_NONE) {
> g_hash_table_remove(ov->unprocessed_opts, name);
> + return;
> }
> + assert(ov->list_mode == LM_IN_PROGRESS);
> + /* do nothing */
> }
>
>
> @@ -365,7 +390,7 @@ opts_start_optional(Visitor *v, bool *present, const char
> *name,
> OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
>
> /* we only support a single mandatory scalar field in a list node */
> - assert(ov->repeated_opts == NULL);
> + assert(ov->list_mode == LM_NONE);
> *present = (lookup_distinct(ov, name, NULL) != NULL);
> }
>
>
[Qemu-devel] [RFC 4/6] OptsVisitor: rebase opts_type_uint64() to parse_uint_full(), Laszlo Ersek, 2013/07/18
[Qemu-devel] [RFC 3/6] OptsVisitor: opts_type_int(): recognize intervals when LM_IN_PROGRESS, Laszlo Ersek, 2013/07/18
[Qemu-devel] [RFC 5/6] OptsVisitor: opts_type_uint64(): recognize intervals when LM_IN_PROGRESS, Laszlo Ersek, 2013/07/18
[Qemu-devel] [RFC 6/6] OptsVisitor: don't try to flatten overlong integer ranges, Laszlo Ersek, 2013/07/18