On Thu, 17 Sep 2020 13:18:51 +0100
Daniel P. Berrangé <berra...@redhat.com> wrote:
> On Thu, Sep 17, 2020 at 02:04:41PM +0200, Markus Armbruster wrote:
> > Greg Kurz <gr...@kaod.org> writes:
> >
> > > $ git grep object_property_get_uint -- :^{include,qom/object.c} | wc -l
> > > 60
> > >
> > > Manual inspecting the output of
> > >
> > > $ git grep -W object_property_get_uint -- :^{include,qom/object.c}
> > > ...
> > >
> > > seems to be showing that most users simply ignore errors (ie. pass NULL
> > > as 3rd argument). Then some users pass &error_abort and only a few
> > > pass a &err or errp.
> > >
> > > Assuming that users know what they're doing, I guess my proposal
> > > wouldn't bring much to the code base in the end... I'm not even
> > > sure now that it's worth changing the contract.
> >
> > We'd change
> >
> > val = object_property_get_uint(obj, name, &error_abort);
> >
> > to
> >
> > object_property_get_uint(obj, name, &val, &error_abort);
> >
> > which is not an improvement.
> >
> > Most of the ones passing NULL should probably pass &error_abort
> > instead. Doesn't change the argument.
>
> I wonder if we actually need to have an Error parameter at all for
> the getters. IIUC the only valid runtime error is probably the case
> where the property has not been set, and even then, I think properties
> usually have a default value that would be returned. All the other
> errors look like programmer errors.
>
> IOW, can we remove the Error parameter and have all the o_p_get*
> method impls use error_abort.
>
> In the rare case where a caller needs to handle a property not
> being set, they can use object_property_find() as a test before
> invoking the getter.
>
I tend to agree.
> Of course requires someone motivated to audit all the case that
> are not using NULL or error_abort and decide whether the attempt
> at passing a local errp was actually justified or not.
>
Since I've open the can of worms, I'm volunteering to do that if
we have a consensus on the fact that "the only valid runtime
error os the case the property has not been set".
Cheers,
--
Greg
> Regards,
> Daniel
