Philippe Mathieu-Daudé <[email protected]> writes:
> Following the Error API best practices documented in commit
> e3fe3988d7 ("error: Document Error API usage rules"), have the
> object_child_foreach[_recursive]() handler take a Error* argument
> and return a boolean indicating whether this error is set or not.
> Convert all handler implementations.
>
> Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
This patch does several things (no, I'm not going to ask to split it):
* Convert object_child_foreach() and object_child_foreach_recursive() to
the Error API: add parameter Error **errp, change return type from int
to bool.
Straightforward.
* Adjust the callers: pass the new argument.
Looks like you pass NULL, which makes sense for a conversion such as
this. Always NULL?
* Convert object_child_foreach()'s and
object_child_foreach_recursive()'s callback parameter to the Error
API: add parameter Error **errp, change return type from int to bool.
Straightforward.
* Adjust the actual callbacks: take the new parameter and use it
properly, return bool instead of int.
Either don't touch @errp and return true, or store an error to @errp
and return false.
You're not doing this at least in bmc_find(), see right below.
I don't have the time for checking all the callbacks today. Mind
doing that yourself?
[...]
> diff --git a/hw/ppc/pnv_bmc.c b/hw/ppc/pnv_bmc.c
> index 99f1e8d7f9..05acc88a55 100644
> --- a/hw/ppc/pnv_bmc.c
> +++ b/hw/ppc/pnv_bmc.c
> @@ -283,17 +283,17 @@ typedef struct ForeachArgs {
> Object *obj;
> } ForeachArgs;
>
> -static int bmc_find(Object *child, void *opaque)
> +static bool bmc_find(Object *child, void *opaque, Error **errp)
> {
> ForeachArgs *args = opaque;
>
> if (object_dynamic_cast(child, args->name)) {
> if (args->obj) {
> - return 1;
> + return false;
No error set here.
> }
> args->obj = child;
> }
> - return 0;
> + return true;
> }
>
> IPMIBmc *pnv_bmc_find(Error **errp)
[...]
