On 4/28/20 11:34 AM, Markus Armbruster wrote:
Same story as for object_property_add(): the only way
object_property_del() can fail is when the property with this name
does not exist. Since our property names are all hardcoded, failure
is a programming error, and the appropriate way to handle it is
passing &error_abort. Most callers do that, the commit before
previous fixed one that didn't (and got the error handling wrong), and
the two remaining exceptions ignore errors.
Drop the @errp parameter and assert the precondition instead.
Signed-off-by: Markus Armbruster <arm...@redhat.com>
---
I skipped review of 15/17 (it's less mechanical, and although the commit
message was good, verifying that the patch matches the commit message is
a bigger task). But assuming it is right, then this one indeed makes sense.
+++ b/qom/object.c
@@ -1280,15 +1280,10 @@ ObjectProperty *object_class_property_find(ObjectClass
*klass, const char *name,
return prop;
}
-void object_property_del(Object *obj, const char *name, Error **errp)
+void object_property_del(Object *obj, const char *name)
{
ObjectProperty *prop = g_hash_table_lookup(obj->properties, name);
- if (!prop) {
- error_setg(errp, "Property '.%s' not found", name);
- return;
- }
-
if (prop->release) {
prop->release(obj, name, prop->opaque);
}
However, the commit message says you assert the precondition, whereas
the code SEGVs rather than asserts if the precondition is not met. In
practice, both will flag the programmer error, so I don't care which you
do, but it's worth making the commit match the intent: Did you mean to
add an assert()?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
