Re: [PATCH v2 48/67] ui/vnc: merge vnc_display_init() and vnc_display_open()

["Daniel P. Berrangé" <berrange@redhat.com>]

On Sun, Apr 12, 2026 at 10:57:51PM +0200, Philippe Mathieu-Daudé wrote:
> On 12/4/26 16:17, Marc-André Lureau wrote:
> > Hi
> > 
> > On Sun, Apr 12, 2026 at 2:31 AM Philippe Mathieu-Daudé
> > <philmd@linaro.org> wrote:
> > > 
> > > On 10/4/26 21:19, Marc-André Lureau wrote:
> > > > Combine the two-step vnc_display_init()/vnc_display_open() sequence
> > > > into a single vnc_display_new() function that returns VncDisplay*.
> > > > This simplifies the API by making vnc_display_open() an
> > > > internal detail and will allow further code simplification.
> > > > 
> > > > vnc_display_new() is moved to vnc.h, since it returns VncDisplay* now.
> > > > Add vnc_display_free() for consistency, and it will be later used.
> > > > 
> > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > ---
> > > >    include/ui/console.h |  2 --
> > > >    ui/vnc.h             |  3 +++
> > > >    ui/vnc.c             | 75 ++++++++++++++++++++++------------------------------
> > > >    3 files changed, 35 insertions(+), 45 deletions(-)
> > > 
> > > 
> > > > -bool vnc_display_init(const char *id, Error **errp)
> > > > +VncDisplay *vnc_display_new(const char *id, Error **errp)
> > > >    {
> > > >        VncDisplay *vd;
> > > > 
> > > >        if (vnc_display_find(id) != NULL) {
> > > > -        return true;
> > > > +        return NULL;
> > > 
> > > Can you explain why we don't return the VncDisplay found here?
> > 
> > `vnc_display_new()` is called from `vnc_init_func()`. If we have
> > duplicate IDs, the function returns NULL, which is treated as an
> > error.
> > 
> > Duplicate IDs are handled earlier by qemu_opts_create() though. So we
> > could assert() instead ?
> 
> That would make this code path clearer for sure.

Yeah, an assert(!vnc_display_find(id)) would make sense.


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 :|