qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 1/3] qapi.py: Move common code to evaluate()


From: Kevin Wolf
Subject: Re: [Qemu-devel] [PATCH 1/3] qapi.py: Move common code to evaluate()
Date: Mon, 24 Jun 2013 17:17:35 +0200
User-agent: Mutt/1.5.21 (2010-09-15)

Am 21.06.2013 um 18:39 hat mdroth geschrieben:
> On Wed, Jun 19, 2013 at 06:28:05PM +0200, Kevin Wolf wrote:
> > Don't duplicate more code than is really necessary.
> > 
> > Signed-off-by: Kevin Wolf <address@hidden>
> > ---
> >  scripts/qapi.py | 24 ++++++++++--------------
> >  1 file changed, 10 insertions(+), 14 deletions(-)
> > 
> > diff --git a/scripts/qapi.py b/scripts/qapi.py
> > index 02ad668..3a64769 100644
> > --- a/scripts/qapi.py
> > +++ b/scripts/qapi.py
> > @@ -76,12 +76,18 @@ def parse(tokens):
> >          return tokens[0], tokens[1:]
> > 
> >  def evaluate(string):
> > -    return parse(map(lambda x: x, tokenize(string)))[0]
> > +    expr_eval = parse(map(lambda x: x, tokenize(string)))[0]
> > +
> > +    if expr_eval.has_key('enum'):
> > +        add_enum(expr_eval['enum'])
> > +    elif expr_eval.has_key('union'):
> > +        add_enum('%sKind' % expr_eval['union'])
> > +
> > +    return expr_eval
> 
> add_enum() adds stuff to a global table, so I don't really like the idea
> of pushing it further down the stack (however inconsequential it may be
> in this case...)

As if it made any difference if it's one level more or less... It's
already buried in the "library".

> I think the real reason we have repetition is the extra 'flush' we do
> after the for loop below to handle the last expression we read from a
> schema, which leads to a repeat of one of the clauses in the loop body.
> I've wanted to get rid of it a few times in the past so this is probably
> a good opportunity to do so.
> 
> Could you try adapting something like the following to keep the
> global stuff in parse_schema()?

I don't think there's any value in keeping the global stuff in
parse_schema() (or, to be precise, in functions directly called by
parse_schema()) , and I found it quite nice to have evaluate()
actually evaluate something instead of just parsing it.

But I agree that duplicating code, as small as it may be now, isn't
nice, so I can try to use the get_expr() thing. I just would prefer to
move the evaluation to evaluate() anyway.

Kevin



reply via email to

[Prev in Thread] Current Thread [Next in Thread]