On Tue, Mar 17, 2026 at 12:50:57PM +0400, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau <[email protected]>
> ---
>  ui/vnc.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)

Reviewed-by: Daniel P. Berrangé <[email protected]>

> 
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 605e5117b7f..af5fb3c1551 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -4318,7 +4318,7 @@ void vnc_parse(const char *str)
>  
>  int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp)
>  {
> -    Error *local_err = NULL;
> +    ERRP_GUARD();
>      char *id = (char *)qemu_opts_id(opts);
>  
>      if (!id) {
> @@ -4326,14 +4326,12 @@ int vnc_init_func(void *opaque, QemuOpts *opts, Error 
> **errp)
>          id = vnc_auto_assign_id(opts);
>      }
>  
> -    vnc_display_init(id, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> +    vnc_display_init(id, errp);
> +    if (*errp) {
>          return -1;
>      }
> -    vnc_display_open(id, &local_err);
> -    if (local_err != NULL) {
> -        error_propagate(errp, local_err);
> +    vnc_display_open(id, errp);
> +    if (*errp) {
>          return -1;
>      }
>      return 0;

Unrelated to this patch, I'm wondering about possible bugs in these
two init functions
:

  void vnc_display_init(const char *id, Error **errp)
  {
    VncDisplay *vd;

    if (vnc_display_find(id) != NULL) {
        return;
    }

Why isn't a vnc_display_find failure a reportable error ? With this
"return", the caller thinks we've successfully initialized the
'vc' struct but we haven't.

And


  void vnc_display_open(const char *id, Error **errp)
  {
    VncDisplay *vd = vnc_display_find(id);
    QemuOpts *opts = qemu_opts_find(&qemu_vnc_opts, id);

    ...snip....

    if (!opts) {
        return;
    }


Shouldn't the failure to find 'opts' be an error, rather than an
successful return ?

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|


Reply via email to