Luiz Capitulino <lcapitul...@redhat.com> writes:
> On Thu, 7 Feb 2013 15:18:41 -0200
> Eduardo Habkost <ehabk...@redhat.com> wrote:
>
>> On Tue, Feb 05, 2013 at 09:39:19PM +0100, Laszlo Ersek wrote:
>> >
>> > Signed-off-by: Laszlo Ersek <ler...@redhat.com>
>> > ---
>> > hw/qdev-monitor.c | 21 ++++++++++++++++-----
>> > 1 files changed, 16 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
>> > index cf96046..545e66c 100644
>> > --- a/hw/qdev-monitor.c
>> > +++ b/hw/qdev-monitor.c
>> > @@ -100,9 +100,14 @@ static void qdev_print_devinfo(ObjectClass *klass,
>> > void *opaque)
>> > error_printf("\n");
>> > }
>> >
>> > +typedef struct {
>> > + DeviceState *dev;
>> > + Error **errp;
>> > +} PropertyContext;
>> > +
>> > static int set_property(const char *name, const char *value, void *opaque)
>> > {
>> > - DeviceState *dev = opaque;
>> > + PropertyContext *ctx = opaque;
>> > Error *err = NULL;
>> >
>> > if (strcmp(name, "driver") == 0)
>> > @@ -110,7 +115,7 @@ static int set_property(const char *name, const char
>> > *value, void *opaque)
>> > if (strcmp(name, "bus") == 0)
>> > return 0;
>> >
>> > - qdev_prop_parse(dev, name, value, &err);
>> > + qdev_prop_parse(ctx->dev, name, value, &err);
>> > if (error_is_set(&err)) {
>> > qerror_report_err(err);
>> > error_free(err);
>> > @@ -481,10 +486,16 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>> > if (id) {
>> > qdev->id = id;
>> > }
>> > - if (qemu_opt_foreach(opts, set_property, qdev, 1) != 0) {
>> > - qdev_free(qdev);
>> > - return NULL;
>> > +
>> > + {
>> > + PropertyContext ctx = { .dev = qdev, .errp = NULL };
>> > +
>> > + if (qemu_opt_foreach(opts, set_property, &ctx, 1) != 0) {
>>
>> What about redefining qemu_opt_loopfunc to accept an Error parameter?
>>
>> qemu_opt_foreach() is used in only 4 places in the whole QEMU code, and
>> it already has code to abort on error (but using the function return
>> value instead of an Error** paramater).
>
> He would have to convert the users too, which would be very welcome,
> but it's out of the scope of this series IMO (but would be fine as
> a first series to this work).
I'm fine with this patch as is.
I wouldn't add Error support to qemu_opt_foreach(), I'd replace it by a
more C-ish qemu_opt_next():
for (opt = qemu_opt_next(NULL); opt; opt = qemu_opt_next(opt)) {
set_property(qdev, qemu_opt_name(opt), qemu_opt_value(opt), &err);
if (err) {
...
break;
}
}
Higher-order functions are great in Lisp, but awkward in C.
>
>>
>>
>>
>> > + qdev_free(qdev);
>> > + return NULL;
>> > + }
>> > }
>> > +
>> > if (qdev->id) {
>> > object_property_add_child(qdev_get_peripheral(), qdev->id,
>> > OBJECT(qdev), NULL);
>> > --
>> > 1.7.1
>> >
>> >
>>
