From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53490) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uzpao-0003yf-30 for qemu-devel@nongnu.org; Thu, 18 Jul 2013 10:54:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Uzpam-0000pB-G4 for qemu-devel@nongnu.org; Thu, 18 Jul 2013 10:54:06 -0400 Received: from mx1.redhat.com ([209.132.183.28]:7031) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uzpam-0000p4-8F for qemu-devel@nongnu.org; Thu, 18 Jul 2013 10:54:04 -0400 Message-ID: <51E8017C.2000900@redhat.com> Date: Thu, 18 Jul 2013 16:53:48 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1374155964-11278-1-git-send-email-lersek@redhat.com> <1374155964-11278-2-git-send-email-lersek@redhat.com> In-Reply-To: <1374155964-11278-2-git-send-email-lersek@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC 1/6] OptsVisitor: introduce basic list modes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laszlo Ersek Cc: qemu-devel@nongnu.org, gaowanlong@cn.fujitsu.com 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 > --- > 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); > } > >