Re: [Xen-devel] [PATCH RFC V2 08/10] libxl IDL: generate code to parse libxl__json_object to libxl_FOO struct

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, Apr 22, 2014 at 05:01:43PM +0100, Ian Campbell wrote:
> On Tue, 2014-04-22 at 16:54 +0100, Wei Liu wrote:
> > > > 
> > > > +                (nparent,fexpr) = ty.member(v, f.type.keyvar, parent 
> > > > is None)
> > > > +                s += libxl_C_type_parse_json(f.type.keyvar.type, "x", 
> > > > fexpr, "    ", nparent)
> > > > +                s += "}\n"
> > > > +            s += "x = libxl__json_map_get(\"%s\", %s, %s);\n" % 
> > > > (f.name, w, f.type.json_parse_type)
> > > > +            s += "if (x) {\n"
> > > > +            (nparent,fexpr) = ty.member(v, f, parent is None)
> > > > +            s += libxl_C_type_parse_json(f.type, "x", fexpr, "    ", 
> > > > nparent)
> > > > +            s += "    x = x->parent;\n"
> > > 
> > > What is this x->parent for?
> > > 
> > 
> > To make sure when we finishes processing this member, X points back to
> > its parent again, so that next call to libxl__json_map_get(x) is using
> > the right "x".
> 
> Can we create temporary fixed scope "x"s for each field? It's ok if that
> shadows the existing "x" (for the parent) I think, or am I missing
> something?
> 

libxl has -Wshadow on.

> > > > +            s += "}\n"
> > > 
> > > > @@ -444,4 +533,17 @@ if __name__ == '__main__':
> > > >          f.write("}\n")
> > > >          f.write("\n")
> > > >  
> > > > +    for ty in [t for t in types if t.json_parse_fn is not None]:
> > > > +        f.write("int %s_parse_json(libxl_ctx *ctx, const 
> > > > libxl__json_object *%s, %s)\n" % (ty.typename,"o",ty.make_arg("p", 
> > > > passby=idl.PASS_BY_REFERENCE)))
> > > > +        f.write("{\n")
> > > 
> > > Are we assuming/requiring the caller to have called the init function
> > > for the type? Did you consider having this function do it?
> > > 
> > 
> > Yes, I'm assuming so.
> > 
> > But I can also make this function do it if you wish.
> 
> I think that would be best, this function can be thought of as "init
> from this json", so it should probably do the whole job, which includes
> setting all the appropriate defaults if the json omits fields etc.
> 

Ack.

Wei.

> Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel