Hi

On Tue, Sep 8, 2026 at 8:49 PM Kim, Dongwon <[email protected]> wrote:
>
> Hi Marc-André,
>
> > Subject: Re: [PATCH] ui/gtk: Handle empty notebook state in menu handlers
> >
> > Hi
> >
> > On Tue, Sep 8, 2026 at 12:18 AM <[email protected]> wrote:
> > >
> > > From: Dongwon Kim <[email protected]>
> > >
> > > When the GTK notebook has no active page (e.g. before pages are added
> > > or if all pages are detached/removed), gtk_notebook_get_current_page()
> > > returns -1. Previously, this value was passed unchecked to
> > > gd_vc_find_by_page(), which could result in unexpected lookups or NULL
> > > dereferences.
> > >
> > > Update gd_vc_find_current() to explicitly return NULL when page < 0.
> > > Additionally, add NULL checks for the current VirtualConsole across
> > > all relevant UI menu callbacks to avoid dereferencing an invalid or
> > > NULL pointer when no console tab is currently focused.
> >
> > This looks reasonable, but can it happen? if so, how? Rather than handling a
> > situation that should not happen, I would rather crash or abort if we broke 
> > an
> > invariant.
> >
> > thanks
>
> The mention of "NULL dereferences" in the commit message was inaccurate.
> It's not causing a crash, but rather logic bugs. This happens when all VC tabs
> are detached (untabified). In that state:
>
> gtk_notebook_get_current_page() returns -1.
>
> For any detached tab, gtk_notebook_page_num() also returns -1.
>
> Consequently, gd_vc_find_by_page(s, -1) matches the very first detached
> console in s->vcs. Menu actions triggered on the empty main window mistakenly
> route to that detached window instead of being ignored.
>

Please correct the commit message then
thanks

> Thanks!
>
> >
> > >
> > > Cc: Daniel P. Berrangé <[email protected]>
> > > Cc: Marc-André Lureau <[email protected]>
> > > Signed-off-by: Dongwon Kim <[email protected]>
> > > ---
> > >  ui/gtk.c | 56
> > > ++++++++++++++++++++++++++++++++++++++++++++++++++------
> > >  1 file changed, 50 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/ui/gtk.c b/ui/gtk.c
> > > index 7078d89d67..0b5139ef31 100644
> > > --- a/ui/gtk.c
> > > +++ b/ui/gtk.c
> > > @@ -184,6 +184,11 @@ static VirtualConsole
> > *gd_vc_find_current(GtkDisplayState *s)
> > >      gint page;
> > >
> > >      page = gtk_notebook_get_current_page(GTK_NOTEBOOK(s->notebook));
> > > +
> > > +    if (page < 0) {
> > > +        return NULL;
> > > +    }
> > > +
> > >      return gd_vc_find_by_page(s, page);  }
> > >
> > > @@ -1462,7 +1467,10 @@ static void gd_menu_show_tabs(GtkMenuItem
> > *item, void *opaque)
> > >      } else {
> > >          gtk_notebook_set_show_tabs(GTK_NOTEBOOK(s->notebook), FALSE);
> > >      }
> > > -    gd_update_windowsize(vc);
> > > +
> > > +    if (vc) {
> > > +        gd_update_windowsize(vc);
> > > +    }
> > >  }
> > >
> > >  static int gd_vc_notebook_pos(GtkDisplayState *s, VirtualConsole
> > > *target) @@ -1535,6 +1543,10 @@ static void
> > gd_menu_untabify(GtkMenuItem *item, void *opaque)
> > >      GtkDisplayState *s = opaque;
> > >      VirtualConsole *vc = gd_vc_find_current(s);
> > >
> > > +    if (!vc) {
> > > +        return;
> > > +    }
> > > +
> > >      if (vc->type == GD_VC_GFX &&
> > >          qemu_console_is_graphic(vc->gfx.dcl.con)) {
> > >
> > > gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
> > > @@ -1588,7 +1600,10 @@ static void
> > gd_menu_show_menubar(GtkMenuItem *item, void *opaque)
> > >      } else {
> > >          gtk_widget_hide(s->menu_bar);
> > >      }
> > > -    gd_update_windowsize(vc);
> > > +
> > > +    if (vc) {
> > > +        gd_update_windowsize(vc);
> > > +    }
> > >  }
> > >
> > >  static void gd_accel_show_menubar(void *opaque) @@ -1605,7 +1620,7
> > @@
> > > static void gd_menu_full_screen(GtkMenuItem *item, void *opaque)
> > >      if (!s->full_screen) {
> > >          gtk_notebook_set_show_tabs(GTK_NOTEBOOK(s->notebook), FALSE);
> > >          gtk_widget_hide(s->menu_bar);
> > > -        if (vc->type == GD_VC_GFX) {
> > > +        if (vc && vc->type == GD_VC_GFX) {
> > >              gtk_widget_set_size_request(vc->gfx.drawing_area, -1, -1);
> > >          }
> > >          gtk_window_fullscreen(GTK_WINDOW(s->window));
> > > @@ -1618,14 +1633,16 @@ static void gd_menu_full_screen(GtkMenuItem
> > *item, void *opaque)
> > >              gtk_widget_show(s->menu_bar);
> > >          }
> > >          s->full_screen = FALSE;
> > > -        if (vc->type == GD_VC_GFX) {
> > > +        if (vc && vc->type == GD_VC_GFX) {
> > >              vc->gfx.scale_x = vc->gfx.preferred_scale;
> > >              vc->gfx.scale_y = vc->gfx.preferred_scale;
> > >              gd_update_windowsize(vc);
> > >          }
> > >      }
> > >
> > > -    gd_update_cursor(vc);
> > > +    if (vc) {
> > > +        gd_update_cursor(vc);
> > > +    }
> > >  }
> > >
> > >  static void gd_accel_full_screen(void *opaque) @@ -1639,6 +1656,10 @@
> > > static void gd_menu_zoom_in(GtkMenuItem *item, void *opaque)
> > >      GtkDisplayState *s = opaque;
> > >      VirtualConsole *vc = gd_vc_find_current(s);
> > >
> > > +    if (!vc) {
> > > +        return;
> > > +    }
> > > +
> > >      gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s-
> > >zoom_fit_item),
> > >                                     FALSE);
> > >
> > > @@ -1659,6 +1680,10 @@ static void gd_menu_zoom_out(GtkMenuItem
> > *item, void *opaque)
> > >      GtkDisplayState *s = opaque;
> > >      VirtualConsole *vc = gd_vc_find_current(s);
> > >
> > > +    if (!vc) {
> > > +        return;
> > > +    }
> > > +
> > >      gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s-
> > >zoom_fit_item),
> > >                                     FALSE);
> > >
> > > @@ -1676,6 +1701,10 @@ static void gd_menu_zoom_fixed(GtkMenuItem
> > *item, void *opaque)
> > >      GtkDisplayState *s = opaque;
> > >      VirtualConsole *vc = gd_vc_find_current(s);
> > >
> > > +    if (!vc) {
> > > +        return;
> > > +    }
> > > +
> > >      vc->gfx.scale_x = vc->gfx.preferred_scale;
> > >      vc->gfx.scale_y = vc->gfx.preferred_scale;
> > >
> > > @@ -1687,6 +1716,10 @@ static void gd_menu_zoom_fit(GtkMenuItem
> > *item, void *opaque)
> > >      GtkDisplayState *s = opaque;
> > >      VirtualConsole *vc = gd_vc_find_current(s);
> > >
> > > +    if (!vc) {
> > > +        return;
> > > +    }
> > > +
> > >      if (gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(s-
> > >zoom_fit_item))) {
> > >          s->free_scale = TRUE;
> > >      } else {
> > > @@ -1800,6 +1833,11 @@ static void gd_menu_grab_input(GtkMenuItem
> > *item, void *opaque)
> > >      VirtualConsole *vc = gd_vc_find_current(s);
> > >
> > >      if (gd_is_grab_active(s)) {
> > > +        if (!vc) {
> > > +            gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s-
> > >grab_item),
> > > +                                           FALSE);
> > > +            return;
> > > +        }
> > >          gd_grab_keyboard(vc, "user-request-main-window");
> > >          gd_grab_pointer(vc, "user-request-main-window");
> > >      } else {
> > > @@ -1807,7 +1845,9 @@ static void gd_menu_grab_input(GtkMenuItem
> > *item, void *opaque)
> > >          gd_ungrab_pointer(s);
> > >      }
> > >
> > > -    gd_update_cursor(vc);
> > > +    if (vc) {
> > > +        gd_update_cursor(vc);
> > > +    }
> > >  }
> > >
> > >  static void gd_change_page(GtkNotebook *nb, gpointer arg1, guint
> > > arg2, @@ -1983,6 +2023,10 @@ static void gd_menu_copy(GtkMenuItem
> > *item, void *opaque)
> > >      GtkDisplayState *s = opaque;
> > >      VirtualConsole *vc = gd_vc_find_current(s);
> > >
> > > +    if (!vc) {
> > > +        return;
> > > +    }
> > > +
> > >  #if VTE_CHECK_VERSION(0, 50, 0)
> > >      vte_terminal_copy_clipboard_format(VTE_TERMINAL(vc->vte.terminal),
> > >                                         VTE_FORMAT_TEXT);
> > > --
> > > 2.43.0
> > >
> > >
> >
> >
> > --
> > Marc-André Lureau



-- 
Marc-André Lureau

Reply via email to