[PULL 14/14] ui: add precondition for dpy_get_ui_info()

marcandre.lureau@redhat.com posted 14 patches 2 years, 5 months ago
Maintainers: "Marc-André Lureau" <marcandre.lureau@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, Peter Maydell <peter.maydell@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Akihiko Odaki <akihiko.odaki@daynix.com>
There is a newer version of this series
[PULL 14/14] ui: add precondition for dpy_get_ui_info()
Posted by marcandre.lureau@redhat.com 2 years, 5 months ago
From: Marc-André Lureau <marcandre.lureau@redhat.com>

Ensure that it only get called when dpy_ui_info_supported(). The
function should always return a result. There should be a non-null
console or active_console.

Modify the argument to be const as well.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Albert Esteve <aesteve@redhat.com>
---
 include/ui/console.h | 2 +-
 ui/console.c         | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index 79e4702912..28882f15a5 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -329,7 +329,7 @@ void update_displaychangelistener(DisplayChangeListener *dcl,
                                   uint64_t interval);
 void unregister_displaychangelistener(DisplayChangeListener *dcl);
 
-bool dpy_ui_info_supported(QemuConsole *con);
+bool dpy_ui_info_supported(const QemuConsole *con);
 const QemuUIInfo *dpy_get_ui_info(const QemuConsole *con);
 int dpy_set_ui_info(QemuConsole *con, QemuUIInfo *info, bool delay);
 
diff --git a/ui/console.c b/ui/console.c
index aa1e09462c..4a4f19ed33 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -801,7 +801,7 @@ static void dpy_set_ui_info_timer(void *opaque)
     con->hw_ops->ui_info(con->hw, head, &con->ui_info);
 }
 
-bool dpy_ui_info_supported(QemuConsole *con)
+bool dpy_ui_info_supported(const QemuConsole *con)
 {
     if (con == NULL) {
         con = active_console;
@@ -815,6 +815,8 @@ bool dpy_ui_info_supported(QemuConsole *con)
 
 const QemuUIInfo *dpy_get_ui_info(const QemuConsole *con)
 {
+    assert(dpy_ui_info_supported(con));
+
     if (con == NULL) {
         con = active_console;
     }
-- 
2.41.0


Re: [PULL 14/14] ui: add precondition for dpy_get_ui_info()
Posted by Daniel P. Berrangé 2 years, 4 months ago
On Tue, Sep 12, 2023 at 02:46:48PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Ensure that it only get called when dpy_ui_info_supported(). The
> function should always return a result. There should be a non-null
> console or active_console.

Empirically that does not appear to be the case. After this patch,
a no-args QEMU launch immediately aborts:

$ ./build/qemu-system-x86_64 
qemu-system-x86_64: ../ui/console.c:818: dpy_get_ui_info: Assertion `dpy_ui_info_supported(con)' failed.
Aborted (core dumped)

This ought to be running the GTK UI for me. Manually ask for
SDL instead and it doesn't crash.

> 
> Modify the argument to be const as well.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Albert Esteve <aesteve@redhat.com>
> ---
>  include/ui/console.h | 2 +-
>  ui/console.c         | 4 +++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 79e4702912..28882f15a5 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -329,7 +329,7 @@ void update_displaychangelistener(DisplayChangeListener *dcl,
>                                    uint64_t interval);
>  void unregister_displaychangelistener(DisplayChangeListener *dcl);
>  
> -bool dpy_ui_info_supported(QemuConsole *con);
> +bool dpy_ui_info_supported(const QemuConsole *con);
>  const QemuUIInfo *dpy_get_ui_info(const QemuConsole *con);
>  int dpy_set_ui_info(QemuConsole *con, QemuUIInfo *info, bool delay);
>  
> diff --git a/ui/console.c b/ui/console.c
> index aa1e09462c..4a4f19ed33 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -801,7 +801,7 @@ static void dpy_set_ui_info_timer(void *opaque)
>      con->hw_ops->ui_info(con->hw, head, &con->ui_info);
>  }
>  
> -bool dpy_ui_info_supported(QemuConsole *con)
> +bool dpy_ui_info_supported(const QemuConsole *con)
>  {
>      if (con == NULL) {
>          con = active_console;
> @@ -815,6 +815,8 @@ bool dpy_ui_info_supported(QemuConsole *con)
>  
>  const QemuUIInfo *dpy_get_ui_info(const QemuConsole *con)
>  {
> +    assert(dpy_ui_info_supported(con));
> +
>      if (con == NULL) {
>          con = active_console;
>      }
> -- 
> 2.41.0
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|