Re: [PATCH 3/5] bulk: Have object_child_foreach() take Error* and return boolean

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@linaro.org> 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é <philmd@linaro.org>

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)

[...]