Re: [PATCH] ui/gtk: fix UI info precondition

[Michael Tokarev]

15.09.2023 14:36, marcandre.lureau@redhat.com:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> dpy_get_ui_info() shouldn't be called if the underlying GPU doesn't
> support it.
>
> Before the assert() was added and the regression introduced, GTK code
> used to get "zero" UI info, for ex with a simple VGA device. The assert
> was added to prevent from calling when there are no console too. The
> other display backend that calls dpy_get_ui_info() correctly checks that
> pre-condition.
>
> Calling dpy_set_ui_info() is "safe" in this case, it will simply return
> an error that can be generally ignored.
>
> Fixes: commit a92e7bb4c ("ui: add precondition for dpy_get_ui_info()")
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   ui/gtk.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/ui/gtk.c b/ui/gtk.c
> index e09f97a86b..7b542da0c0 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -726,6 +726,10 @@ static void gd_set_ui_refresh_rate(VirtualConsole *vc, int 
> refresh_rate)
>   {
>       QemuUIInfo info;
>   
> +    if (!dpy_ui_info_supported(vc->gfx.dcl.con)) {
> +        return;
> +    }
> +
>       info = *dpy_get_ui_info(vc->gfx.dcl.con);

Current dpy_ui_info_supported():

bool dpy_ui_info_supported(QemuConsole *con)
{
    if (con == NULL) {
        con = active_console;
    }
    if (con == NULL) {
        return false;
    }

    return con->hw_ops->ui_info != NULL;
}

This whole thing smells a bit wrong.  I'm not saying it *is* wrong, but
the feeling is here.

Where dpy_ui_info_supported() is called with a NULL con, so that an "active"
console needs to be used instead?

Maybe we should instead use something like

   if (!vc->gfx.dcl.con)

here in gd_set_ui_refresh_rate() ?

At the very least, the code is a bit, well, confusing.

/mjt