Re: [PATCH 06/17] qom: Drop object_property_set_description() parameter @errp

[Eric Blake]

On 4/28/20 11:34 AM, Markus Armbruster wrote:
> object_property_set_description() and
> object_class_property_set_description() fail only when property @name
> is not found.
>
> There are 85 calls of object_property_set_description() and
> object_class_property_set_description().  None of them can fail:
>
> * 84 immediately follow the creation of the property.
>
> * The one in spapr_rng_instance_init() refers to a property created in
>    spapr_rng_class_init(), from spapr_rng_properties[].

I don't envy how you counted those.  But on sheer numbers, this patch 
has to be at least 85 changed lines in length...

> Every one of them still gets to decide what to pass for @errp.
>
> 51 calls pass &error_abort, 32 calls pass NULL, one receives the error
> and propagates it to &error_abort, and one propagates it to
> &error_fatal.  I'm actually surprised none of them violates the Error
> API.
>
> What are we gaining by letting callers handle the "property not found"
> error?  Use when the property is not known to exist is simpler: you
> don't have to guard the call with a check.  We haven't found such a
> use in 5+ years.  Until we do, let's make life a bit simpler and drop
> the @errp parameter.
>
> Signed-off-by: Markus Armbruster <address@hidden>
> ---

>   30 files changed, 91 insertions(+), 143 deletions(-)

...which it is. And the compiler would choke if you missed anything. 
The bulk of this is mechanical, with no way to split it smaller.

After hunting, I found the core of the patch:

> +++ b/qom/object.c
> @@ -2835,38 +2835,27 @@ object_property_add_alias(Object *obj, const char *name,
>       }
>   
>       object_property_set_description(obj, op->name,
> -                                    target_prop->description,
> -                                    &error_abort);
> +                                    target_prop->description);
>       return op;
>   }
>   
>   void object_property_set_description(Object *obj, const char *name,
> -                                     const char *description, Error **errp)
> +                                     const char *description)
>   {
>       ObjectProperty *op;
>   
> -    op = object_property_find(obj, name, errp);
> -    if (!op) {
> -        return;
> -    }
> -
> +    op = object_property_find(obj, name, &error_abort);
>       g_free(op->description);
>       op->description = g_strdup(description);
>   }
>   
>   void object_class_property_set_description(ObjectClass *klass,
>                                              const char *name,
> -                                           const char *description,
> -                                           Error **errp)
> +                                           const char *description)
>   {
>       ObjectProperty *op;
>   
>       op = g_hash_table_lookup(klass->properties, name);
> -    if (!op) {
> -        error_setg(errp, "Property '.%s' not found", name);
> -        return;
> -    }
> -

and it is sensible.

Reviewed-by: Eric Blake <address@hidden>

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org