Message ID | 1374155964-11278-2-git-send-email-lersek@redhat.com |
---|---|
State | New |
Headers | show |
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 <lersek@redhat.com> > --- > 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); > } > >
On 07/18/13 16:53, Paolo Bonzini wrote: > Il 18/07/2013 15:59, Laszlo Ersek ha scritto: >> + 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! Yeah I hesitated between these two. Will fix. Thanks, Laszlo
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); } *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); }
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 <lersek@redhat.com> --- qapi/opts-visitor.c | 43 ++++++++++++++++++++++++++++++++++--------- 1 files changed, 34 insertions(+), 9 deletions(-)