[Qemu-devel] Re: [PATCH 45/50] qemu-option: Functions to convert to/from QDict

[Luiz Capitulino]

On Thu, 04 Mar 2010 22:12:01 +0100
Markus Armbruster <address@hidden> wrote:

> Luiz Capitulino <address@hidden> writes:
> 
> > On Thu,  4 Mar 2010 16:57:06 +0100
> > Markus Armbruster <address@hidden> wrote:
> >
> >> The functions are somewhat restricted.  Good enough for the job at
> >> hand.  We'll extend them when we need more.
> >> 
> >> Signed-off-by: Markus Armbruster <address@hidden>
> >> ---
> >>  qemu-option.c |   79 
> >> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  qemu-option.h |    3 ++
> >>  2 files changed, 82 insertions(+), 0 deletions(-)
> >> 
> >> diff --git a/qemu-option.c b/qemu-option.c
> >> index ab488e4..24bb19b 100644
> >> --- a/qemu-option.c
> >> +++ b/qemu-option.c
> [...]
> >> +/*
> >> + * Convert from QemuOpts to QDict.
> >> + * The QDict values are of type QString.
> >> + * TODO We'll want to use types appropriate for opt->desc->type, but
> >> + * this is enough for now.
> >> + */
> >> +QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict)
> >> +{
> >> +    QemuOpt *opt;
> >> +    QObject *val;
> >> +
> >> +    if (!qdict) {
> >> +        qdict = qdict_new();
> >> +    }
> >> +    if (opts->id) {
> >> +        qdict_put(qdict, "id", qstring_from_str(opts->id));
> >> +    }
> >> +    QTAILQ_FOREACH(opt, &opts->head, next) {
> >> +        val = QOBJECT(qstring_from_str(opt->str));
> >> +        qdict_put_obj(qdict, opt->name, val);
> >> +    }
> >
> >  Why not just do:
> >
> > qdict_put(qdict, opt->name, qstring_from_str(opt->str));
> 
> I got a version without the TODO.  Looks like this:
> 
>     QTAILQ_FOREACH(opt, &opts->head, next) {
>         switch (opt->desc ? opt->desc->type : QEMU_OPT_STRING) {
>         case QEMU_OPT_BOOL:
>             val = QOBJECT(qbool_from_int(opt->value.boolean));
>             break;
>         case QEMU_OPT_NUMBER:
>         case QEMU_OPT_SIZE:
>             // FIXME opts only uint64_t, qint only int64_t
>             val = QOBJECT(qint_from_int(opt->value.uint));
>             break;
>         default:
>             val = QOBJECT(qstring_from_str(opt->str));
>             break;
>         }
>         qdict_put_obj(qdict, opt->name, val);
>     }
> 
> I dumbed it down to keep this series as simple as possible, and missed
> the simplification opportunity.  Okay to leave it as is?

 Yes, although it's worth changing in case you resend the series with
other modifications (assuming you'll cleanup soon, of course).