Re: [Qemu-block] [Qemu-devel] [PATCH 17/24] qapi: New qobject_input_visitor_new_str() for convenience

[Kevin Wolf]

Am 28.02.2017 um 19:48 hat Markus Armbruster geschrieben:
> Kevin Wolf <address@hidden> writes:
> 
> > Am 27.02.2017 um 12:20 hat Markus Armbruster geschrieben:
> >> Signed-off-by: Markus Armbruster <address@hidden>
> >> ---
> >>  include/qapi/qobject-input-visitor.h | 12 ++++++++++++
> >>  qapi/qobject-input-visitor.c         | 33 
> >> +++++++++++++++++++++++++++++++++
> >>  2 files changed, 45 insertions(+)
> >> 
> >> diff --git a/include/qapi/qobject-input-visitor.h 
> >> b/include/qapi/qobject-input-visitor.h
> >> index 282f9d2..b399285 100644
> >> --- a/include/qapi/qobject-input-visitor.h
> >> +++ b/include/qapi/qobject-input-visitor.h
> >> @@ -68,4 +68,16 @@ Visitor *qobject_input_visitor_new(QObject *obj);
> >>   */
> >>  Visitor *qobject_input_visitor_new_keyval(QObject *obj);
> >>  
> >> +/*
> >> + * Create a QObject input visitor for parsing @str.
> >> + *
> >> + * If @str looks like JSON, parse it as JSON, else as KEY=VALUE,...
> >> + * @implied_key applies to KEY=VALUE, and works as in keyval_parse().
> >> + * On failure, store an error through @errp and return NULL.
> >> + * On success, return a new QObject input visitor for the parse.
> >> + */
> >> +Visitor *qobject_input_visitor_new_str(const char *str,
> >> +                                       const char *implied_key,
> >> +                                       Error **errp);
> >> +
> >>  #endif
> >> diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
> >> index 3db5850..64a08d3 100644
> >> --- a/qapi/qobject-input-visitor.c
> >> +++ b/qapi/qobject-input-visitor.c
> >> @@ -18,9 +18,11 @@
> >>  #include "qapi/visitor-impl.h"
> >>  #include "qemu/queue.h"
> >>  #include "qemu-common.h"
> >> +#include "qapi/qmp/qjson.h"
> >>  #include "qapi/qmp/types.h"
> >>  #include "qapi/qmp/qerror.h"
> >>  #include "qemu/cutils.h"
> >> +#include "qemu/option.h"
> >>  
> >>  typedef struct StackObject {
> >>      const char *name;            /* Name of @obj in its parent, if any */
> >> @@ -655,3 +657,34 @@ Visitor *qobject_input_visitor_new_keyval(QObject 
> >> *obj)
> >>  
> >>      return &v->visitor;
> >>  }
> >> +
> >> +Visitor *qobject_input_visitor_new_str(const char *str,
> >> +                                       const char *implied_key,
> >> +                                       Error **errp)
> >> +{
> >> +    bool is_json = str[0] == '{';
> >
> > Should the keyval_parse() grammar just forbid { in key names then (and
> > ideally employ assertions to enforce this) to avoid any ambiguity here?
> > therwise the behaviour might depend on the order of options in a
> > key/value string, which could be somewhat confusing.
> 
> You're absolutely right.  Fortunately, PATCH 23 does.  I guess the
> easiest fix is to move it before this one.

Don't bother. I'm just reviewing the patches in order, so I didn't see
at this point that there is a fix in a later patch.

Kevin