[Top][All Lists]

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

Re: [Qemu-devel] [PATCH RFC 0/2] QMP command qom-new

From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH RFC 0/2] QMP command qom-new
Date: Thu, 24 May 2012 15:52:22 +0100

On 24 May 2012 15:35, Anthony Liguori <address@hidden> wrote:
> Ah, okay.  So you're taking those comments a bit out of context.
> The right way to use object_new() is:
> Object *foo = object_new(TYPE_E1000);
> If you mistype TYPE_E1000, you should get a compile failure.  There's
> basically no way that this is going to fail unless you do:
> Object *foo = object_new("garbage");

But mostly for types we don't in fact have preprocessor defines for
them unless they're subclasses. The average device is created by
passing its name as a plain string. That's just as liable to typos
as property names are. Conversely, you could define preprocessor
macros for all of a device's property names if you thought it would

> There's no point in adding error handling here because if you make this much
> of a mistake, then you're likely not going to handle errors properly.
>  That's entirely different from:
> connect(foo, "baz", bar, "caz");
> It's extremely likely that you'll mistype one of the property names.  We
> have existence proofs in the three with qdev_prop_set*().  This can be deep
> within the bowels of device initialization and assert()'ing QEMU doing a
> device_add is a pretty nasty thing to do.

If a device is so untested that nobody has ever even attempted to
create it, then you're asking for trouble trying to do anything
with it in a production system. It could just as easily do an accidental
NULL dereference in its init function and kill QEMU that way.

I think this is exactly a case where we want "QOM infrastructure to
assert when it detects something bad" because it's not reasonable
for callers to do error handling for can't-happen situations.

> Contrast that with:
> foo->baz = &bar->caz;

...which isn't the way QOM has been set up to work, and in
any case doesn't really work with dynamically created properties.

-- PMM

reply via email to

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