Re: [Qemu-devel] [PATCH for 2.10 21/35] arm/sysbus-fdt: fix null pointer dereference

[Peter Maydell]

On 24 July 2017 at 19:27, Philippe Mathieu-Daudé <address@hidden> wrote:
> Use error_report() + exit() instead of error_setg(&error_fatal).
>
> hw/arm/sysbus-fdt.c:322:9: warning: Array access (from variable 'node_path') 
> results in a null pointer dereference
>     if (node_path[1]) {
>         ^~~~~~~~~~~~

I don't understand what this warning is trying to say.
We can't get to this point with a NULL node_path,
because of the previous conditional, which is using
error_setg(&error_fatal).

> Reported-by: Clang Static Analyzer
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
>  hw/arm/sysbus-fdt.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
> index d68e3dcdbd..ad0cc49b19 100644
> --- a/hw/arm/sysbus-fdt.c
> +++ b/hw/arm/sysbus-fdt.c
> @@ -315,15 +315,14 @@ static int add_amd_xgbe_fdt_node(SysBusDevice *sbdev, 
> void *opaque)
>      node_path = qemu_fdt_node_path(host_fdt, dt_name, vdev->compat,
>                                     &error_fatal);
>      if (!node_path || !node_path[0]) {
> -        error_setg(&error_fatal, "%s unable to retrieve node path for %s/%s",
> +        error_report("%s unable to retrieve node path for %s/%s",
>                     __func__, dt_name, vdev->compat);
> -    }
> -
> -    if (node_path[1]) {
> -        error_setg(&error_fatal, "%s more than one node matching %s/%s!",
> +        exit(1);
> +    } else if (node_path[1]) {
> +        error_report("%s more than one node matching %s/%s!",
>                     __func__, dt_name, vdev->compat);
> +        exit(1);
>      }
> -
>      g_free(dt_name);

This doesn't seem like an improvement. Now the
error handling in the function is an inconsistent
mix of error_report()+exit() and error_setg(&error_fatal).

thanks
-- PMM