On 05/28/2015 06:21 AM, Markus Armbruster wrote: > Retain the function value for now, to permit selective conversion of > its callers. > > Signed-off-by: Markus Armbruster <arm...@redhat.com> > --- > include/qemu/option.h | 7 +++++-- > net/vhost-user.c | 8 +++++--- > qdev-monitor.c | 5 +++-- > ui/spice-core.c | 5 +++-- > util/qemu-config.c | 5 +++-- > util/qemu-option.c | 8 +++++--- > vl.c | 9 +++++---- > 7 files changed, 29 insertions(+), 18 deletions(-) > > diff --git a/include/qemu/option.h b/include/qemu/option.h > index a3cf4c1..ac0e43b 100644 > --- a/include/qemu/option.h > +++ b/include/qemu/option.h > @@ -100,8 +100,11 @@ void qemu_opt_set_bool(QemuOpts *opts, const char *name, > bool val, > Error **errp); > void qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val, > Error **errp); > -typedef int (*qemu_opt_loopfunc)(const char *name, const char *value, void > *opaque); > -int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque); > +typedef int (*qemu_opt_loopfunc)(void *opaque, > + const char *name, const char *value, > + Error **errp);
Again, justification for reordering callback parameter ordering might be
nice to mention in the commit message, but the code itself is correct, so:
Reviewed-by: Eric Blake <ebl...@redhat.com>
> +++ b/util/qemu-option.c
> @@ -597,17 +597,19 @@ void qemu_opt_set_number(QemuOpts *opts, const char
> *name, int64_t val,
> }
>
> /**
> - * For each member of @opts, call @func(name, value, @opaque).
> + * For each member of @opts, call @func(@opaque, name, value, @errp).
> + * @func() may store an Error through @errp, but must return non-zero then.
> * When @func() returns non-zero, break the loop and return that value.
> * Return zero when the loop completes.
> */
> -int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque)
> +int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
> + Error **errp)
> {
> QemuOpt *opt;
> int rc;
>
> QTAILQ_FOREACH(opt, &opts->head, next) {
> - rc = func(opt->name, opt->str, opaque);
> + rc = func(opaque, opt->name, opt->str, errp);
> if (rc) {
> return rc;
> }
As in my earlier review, wouldn't it be better to add:
assert(!*errp)
at this point, to ensure we meet the contract?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
