Re: [Qemu-devel] [PULL 10/13] qom: catch errors in object_property_add_child

[Markus Armbruster]

Andreas Färber <address@hidden> writes:

> Am 18.12.2013 18:00, schrieb Luiz Capitulino:
>> From: Paolo Bonzini <address@hidden>
>> 
>> Signed-off-by: Paolo Bonzini <address@hidden>
>> Reviewed-By: Igor Mammedov <address@hidden>
>> Signed-off-by: Luiz Capitulino <address@hidden>
>> ---
>>  qom/object.c | 9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>> 
>> diff --git a/qom/object.c b/qom/object.c
>> index fc19cf6..68fe07a 100644
>> --- a/qom/object.c
>> +++ b/qom/object.c
>> @@ -988,17 +988,22 @@ static void object_finalize_child_property(Object 
>> *obj, const char *name,
>>  void object_property_add_child(Object *obj, const char *name,
>>                                 Object *child, Error **errp)
>>  {
>> +    Error *local_err = NULL;
>>      gchar *type;
>>  
>>      type = g_strdup_printf("child<%s>", object_get_typename(OBJECT(child)));
>>  
>>      object_property_add(obj, name, type, object_get_child_property,
>> -                        NULL, object_finalize_child_property, child, errp);
>> -
>> +                        NULL, object_finalize_child_property, child, 
>> &local_err);
>> +    if (error_is_set(&local_err)) {
>
> I've been told we shouldn't error_is_set() that way but instead write:
> if (local_err) {

"if (local_err)" is immediately obvious.

"if (error_is_set(&local_err))" isn't.  But it's consistent with places
where you do "if (error_is_set(errp))", where errp may be null, for
whatever that's worth.

Obvious is preferable.  Anyway, hardly a NAK-worthy crime.

[...]