Re: [PATCH v3 7/7] hw/xen: Fix errp handling in xen_console

[Anthony PERARD]

On Wed, Jan 15, 2025 at 04:27:25PM +0000, David Woodhouse wrote:
> diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
> index e61902461b..9e7f6da343 100644
> --- a/hw/char/xen_console.c
> +++ b/hw/char/xen_console.c
> @@ -581,19 +581,27 @@ static void 
> xen_console_device_create(XenBackendInstance *backend,
>                         output);
>              goto fail;
>          }
> -    } else if (number) {
> -        cd = serial_hd(number);
> -        if (!cd) {
> -            error_prepend(errp, "console: No serial device #%ld found: ",
> -                          number);
> -            goto fail;
> -        }
> +    } else if (errno != ENOENT) {
> +        error_prepend(errp, "console: No valid chardev found: ");
> +        goto fail;
>      } else {
> -        /* No 'output' node on primary console: use null. */
> -        cd = qemu_chr_new(label, "null", NULL);
> -        if (!cd) {
> -            error_setg(errp, "console: failed to create null device");
> -            goto fail;
> +        if (errp) {

I don't think you need this check, with ERRP_GUARD() macro `errp` is
never NULL.

> +            error_free(*errp);

After this, I think you still need
    *errp = NULL;

> +        }
> +        if (number) {
> +            cd = serial_hd(number);
> +            if (!cd) {
> +                error_setg(errp, "console: No serial device #%ld found: ",

That error message doesn't need the ": " at the end anymore.

With those fixed: Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>

Cheers,

-- 
Anthony PERARD