Re: [RFC PATCH 5/5] hw/ppc/pnv_bmc: Simplify pnv_bmc_find()

[Cédric Le Goater]

On 2/16/23 13:25, Philippe Mathieu-Daudé wrote:
> ForeachArgs::name is only used once as TYPE_IPMI_BMC.
> Since the penultimate commit, object_child_foreach_recursive()'s
> handler takes an Error* argument and return a boolean.
> We can directly pass ForeachArgs::obj as context, removing the
> ForeachArgs structure.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> ---
> RFC: please double-check...
>
>
>   hw/ppc/pnv_bmc.c | 25 +++++++++----------------
>   1 file changed, 9 insertions(+), 16 deletions(-)
>
> diff --git a/hw/ppc/pnv_bmc.c b/hw/ppc/pnv_bmc.c
> index 05acc88a55..566284469f 100644
> --- a/hw/ppc/pnv_bmc.c
> +++ b/hw/ppc/pnv_bmc.c
> @@ -278,36 +278,29 @@ IPMIBmc *pnv_bmc_create(PnvPnor *pnor)
>       return IPMI_BMC(obj);
>   }
>   
> -typedef struct ForeachArgs {
> -    const char *name;
> -    Object *obj;
> -} ForeachArgs;
> -
>   static bool bmc_find(Object *child, void *opaque, Error **errp)
>   {
> -    ForeachArgs *args = opaque;
> +    Object **obj = opaque;
>   
> -    if (object_dynamic_cast(child, args->name)) {
> -        if (args->obj) {
> -            return false;

The purpose of this test was to catch multiple bmc devices and it's removed
now.
 
> +    if (object_dynamic_cast(child, TYPE_IPMI_BMC)) {
> +        if (*obj) {
> +            return true;
>           }
> -        args->obj = child;
> +        *obj = child;
>       }
>       return true;
>   }
>   
>   IPMIBmc *pnv_bmc_find(Error **errp)
>   {
> -    ForeachArgs args = { TYPE_IPMI_BMC, NULL };
> -    int ret;
> +    Object *obj = NULL;
>   
> -    ret = object_child_foreach_recursive(object_get_root(), bmc_find,
> -                                         &args, NULL);
> -    if (ret) {
> +    if (!object_child_foreach_recursive(object_get_root(), bmc_find, &obj,
> +                                        NULL)) {
>           error_setg(errp, "machine should have only one BMC device. "
>                      "Use '-nodefaults'");>           return NULL;
>       }

We don't test obj against NULL any more. This could break the QOM cast below.

Thanks,


C.


>   
> -    return args.obj ? IPMI_BMC(args.obj) : NULL;
> +    return IPMI_BMC(obj);
>   }