[virt-tools-list] [PATCH virt-viewer 1/4] App: keep hash table of displays

[Fabiano Fidêncio]

On Fri, 2014-08-22 at 09:59 -0500, Jonathon Jongsma wrote:
> This is part of a re-factoring that will de-couple the client window
> from the remote display id.
> ---
>  src/virt-viewer-app.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> index b60ce2d..a3934f2 100644
> --- a/src/virt-viewer-app.c
> +++ b/src/virt-viewer-app.c
> @@ -109,6 +109,7 @@ struct _VirtViewerAppPrivate {
>      VirtViewerWindow *main_window;
>      GtkWidget *main_notebook;
>      GHashTable *windows;
> +    GHashTable *displays;
>      GHashTable *initial_display_map;
>      gchar *clipboard;
>  
> @@ -934,8 +935,14 @@ virt_viewer_app_display_added(VirtViewerSession *session G_GNUC_UNUSED,
>      VirtViewerAppPrivate *priv = self->priv;
>      VirtViewerWindow *window;
>      gint nth;
> +    gint* key = g_malloc(sizeof(gint));

Coding style here, please, use: gint *key = ...
Anyway, do you really have to malloc it here? (...)
https://developer.gnome.org/glib/2.39/glib-Type-Conversion-Macros.html#GINT-TO-POINTER:CAPS

>  
>      g_object_get(display, "nth-display", &nth, NULL);
> +
> +    *key = nth;
> +    g_debug("Insert display %d %p", nth, display);
> +    g_hash_table_insert(self->priv->displays, key, g_object_ref(display));
> +

(...) I'd prefer do not malloc and use it as:
g_hash_table_insert(self->priv->displays, GINT_TO_POINTER(nth),
g_object_ref(display));


>      window = virt_viewer_app_get_nth_window(self, nth);
>      if (window == NULL) {
>          if (priv->kiosk) {
> @@ -965,6 +972,7 @@ virt_viewer_app_display_removed(VirtViewerSession *session G_GNUC_UNUSED,
>  
>      gtk_widget_hide(GTK_WIDGET(display));
>      g_object_get(display, "nth-display", &nth, NULL);
> +    g_hash_table_remove(self->priv->displays, &nth);

If you follow my idea, you have to change it here as well.

>      win = virt_viewer_app_get_nth_window(self, nth);
>      if (!win)
>          return;
> @@ -1636,6 +1644,15 @@ virt_viewer_app_dispose (GObject *object)
>          g_hash_table_unref(tmp);
>      }
>  
> +    if (priv->displays) {
> +        GHashTable *tmp = priv->displays;
> +        /* null-ify before unrefing, because we need
> +         * to prevent callbacks using priv->displays
> +         * while it is being disposed off. */
> +        priv->displays = NULL;
> +        g_hash_table_unref(tmp);
> +    }
> +
>      g_clear_object(&priv->session);
>      g_free(priv->title);
>      priv->title = NULL;
> @@ -1702,6 +1719,7 @@ virt_viewer_app_init (VirtViewerApp *self)
>      virt_viewer_app_set_debug(opt_debug);
>  
>      self->priv = GET_PRIVATE(self);
> +    self->priv->displays = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, g_object_unref);

And use g_direct_hash and g_direct_equal instead of g_int_hash and
g_int_equal. And don't use the g_free, as you wouldn't malloc the key.

>      self->priv->windows = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, g_object_unref);
>      self->priv->config = g_key_file_new();
>      self->priv->config_file = g_build_filename(g_get_user_config_dir(),