[Spice-devel] [PATCH spice-gtk 6/8] RFC: Add read-only property on sessions

Hans de Goede hdegoede at redhat.com
Wed Nov 23 06:38:14 PST 2011


Hi,

See my comments below.

On 11/23/2011 01:23 PM, Marc-André Lureau wrote:
> It is useful to have a way to prevent sending commands in read-only
> sessions (think of multi-client)
>
> No clipboard sharing allowed in this case in gtk-session.
> ---
>   gtk/spice-gtk-session-priv.h |    1 +
>   gtk/spice-gtk-session.c      |   23 +++++++++++++++++++----
>   gtk/spice-session-priv.h     |    2 ++
>   gtk/spice-session.c          |   32 ++++++++++++++++++++++++++++++++
>   4 files changed, 54 insertions(+), 4 deletions(-)
>
> diff --git a/gtk/spice-gtk-session-priv.h b/gtk/spice-gtk-session-priv.h
> index 19692af..21a4251 100644
> --- a/gtk/spice-gtk-session-priv.h
> +++ b/gtk/spice-gtk-session-priv.h
> @@ -24,6 +24,7 @@ G_BEGIN_DECLS
>
>   void spice_gtk_session_update_keyboard_focus(SpiceGtkSession *self,
>                                                gboolean state);
> +gboolean spice_gtk_session_get_read_only(SpiceGtkSession *self);
>
>   G_END_DECLS
>
> diff --git a/gtk/spice-gtk-session.c b/gtk/spice-gtk-session.c
> index c181c7e..6f50762 100644
> --- a/gtk/spice-gtk-session.c
> +++ b/gtk/spice-gtk-session.c
> @@ -77,6 +77,7 @@ static void channel_new(SpiceSession *session, SpiceChannel *channel,
>                           gpointer user_data);
>   static void channel_destroy(SpiceSession *session, SpiceChannel *channel,
>                               gpointer user_data);
> +static gboolean read_only(SpiceGtkSession *self);
>
>   /* ------------------------------------------------------------------ */
>   /* gobject glue                                                       */

Why introduce a separate function for this, why not simply call
spice_session_get_read_only(priv->session) in the places where you now call
read_only(self) ?

> @@ -479,7 +480,7 @@ static void clipboard_owner_change(GtkClipboard        *clipboard,
>           }
>           s->clipboard_by_guest[selection] = FALSE;
>           s->clip_hasdata[selection] = TRUE;
> -        if (s->auto_clipboard_enable)
> +        if (s->auto_clipboard_enable&&  !read_only(self))
>               gtk_clipboard_request_targets(clipboard, clipboard_get_targets,
>                                             self);
>           break;
> @@ -622,7 +623,9 @@ static gboolean clipboard_grab(SpiceMainChannel *main, guint selection,
>       /* Receiving a grab implies we've released our own grab */
>       s->clip_grabbed[selection] = FALSE;
>
> -    if (!s->auto_clipboard_enable || s->nclip_targets[selection] == 0)
> +    if (read_only(self) ||
> +        !s->auto_clipboard_enable ||
> +        s->nclip_targets[selection] == 0)
>           goto skip_grab_clipboard;
>
>       if (!gtk_clipboard_set_with_data(cb, targets, i, clipboard_get,
> @@ -694,6 +697,9 @@ static gboolean clipboard_request(SpiceMainChannel *main, guint selection,
>       GtkClipboard* cb;
>       int m;
>
> +    if (read_only(self))
> +        return FALSE;
> +
>       cb = get_clipboard_from_selection(s, selection);
>       g_return_val_if_fail(cb != NULL, FALSE);
>
> @@ -773,6 +779,14 @@ static void channel_destroy(SpiceSession *session, SpiceChannel *channel,
>       }
>   }
>
> +static gboolean read_only(SpiceGtkSession *self) {
> +    gboolean read_only;
> +
> +    g_object_get(self->priv->session, "read-only",&read_only, NULL);
> +
> +    return read_only;
> +}
> +
>   /* ---------------------------------------------------------------- */
>   /* private functions (usbredir related)                             */
>   G_GNUC_INTERNAL
> @@ -791,9 +805,8 @@ void spice_gtk_session_update_keyboard_focus(SpiceGtkSession *self,
>           auto_connect = TRUE;
>
>       manager = spice_usb_device_manager_get(s->session, NULL, NULL);
> -    if (manager) {
> +    if (manager)
>           g_object_set(manager, "auto-connect", auto_connect, NULL);
> -    }
>   }
>
>   /* ------------------------------------------------------------------ */

This seems like an unrelated code cleanup change.

> @@ -843,6 +856,7 @@ SpiceGtkSession *spice_gtk_session_get(SpiceSession *session)
>   void spice_gtk_session_copy_to_guest(SpiceGtkSession *self)
>   {
>       g_return_if_fail(SPICE_IS_GTK_SESSION(self));
> +    g_return_if_fail(read_only(self) == FALSE);
>
>       SpiceGtkSessionPrivate *s = self->priv;
>       int selection = VD_AGENT_CLIPBOARD_SELECTION_CLIPBOARD;
> @@ -864,6 +878,7 @@ void spice_gtk_session_copy_to_guest(SpiceGtkSession *self)
>   void spice_gtk_session_paste_from_guest(SpiceGtkSession *self)
>   {
>       g_return_if_fail(SPICE_IS_GTK_SESSION(self));
> +    g_return_if_fail(read_only(self) == FALSE);
>
>       SpiceGtkSessionPrivate *s = self->priv;
>       int selection = VD_AGENT_CLIPBOARD_SELECTION_CLIPBOARD;
> diff --git a/gtk/spice-session-priv.h b/gtk/spice-session-priv.h
> index 8f10407..0aafd27 100644
> --- a/gtk/spice-session-priv.h
> +++ b/gtk/spice-session-priv.h
> @@ -37,6 +37,7 @@ struct _SpiceSessionPrivate {
>       GByteArray        *pubkey;
>       char              *cert_subject;
>       guint             verify;
> +    gboolean          read_only;
>
>       /* whether to enable audio */
>       gboolean          audio;
> @@ -118,6 +119,7 @@ void spice_session_get_caches(SpiceSession *session,
>                                 SpiceGlzDecoderWindow **glz_window);
>   void spice_session_palettes_clear(SpiceSession *session);
>   void spice_session_images_clear(SpiceSession *session);
> +gboolean spice_session_get_read_only(SpiceSession *session);
>
>   G_END_DECLS
>
> diff --git a/gtk/spice-session.c b/gtk/spice-session.c
> index 9e30c74..e637521 100644
> --- a/gtk/spice-session.c
> +++ b/gtk/spice-session.c
> @@ -98,6 +98,7 @@ enum {
>       PROP_USBREDIR,
>       PROP_DISABLE_EFFECTS,
>       PROP_COLOR_DEPTH,
> +    PROP_READ_ONLY,
>   };
>
>   /* signals */
> @@ -369,6 +370,9 @@ static void spice_session_get_property(GObject    *gobject,
>       case PROP_AUDIO:
>           g_value_set_boolean(value, s->audio);
>           break;
> +    case PROP_READ_ONLY:
> +        g_value_set_boolean(value, s->read_only);
> +	break;
>       default:
>   	G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec);
>   	break;
> @@ -462,6 +466,10 @@ static void spice_session_set_property(GObject      *gobject,
>       case PROP_AUDIO:
>           s->audio = g_value_get_boolean(value);
>           break;
> +    case PROP_READ_ONLY:
> +        s->read_only = g_value_get_boolean(value);
> +        g_object_notify(gobject, "read-only");
> +        break;
>       default:
>           G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec);
>           break;
> @@ -833,6 +841,22 @@ static void spice_session_class_init(SpiceSessionClass *klass)
>                        1,
>                        SPICE_TYPE_CHANNEL);
>
> +    /**
> +     * SpiceSession:read-only:
> +     *
> +     * Whether this connection is read-only mode.
> +     *
> +     * Since: 0.8
> +     **/
> +    g_object_class_install_property
> +        (gobject_class, PROP_READ_ONLY,
> +         g_param_spec_boolean("read-only", "Read-only",
> +                              "Whether this connection is read-only mode",
> +                              FALSE,
> +                              G_PARAM_READWRITE |
> +                              G_PARAM_CONSTRUCT |
> +                              G_PARAM_STATIC_STRINGS));
> +
>       g_type_class_add_private(klass, sizeof(SpiceSessionPrivate));
>   }
>
> @@ -1097,6 +1121,14 @@ void spice_session_channel_migrate(SpiceSession *session, SpiceChannel *channel)
>       }
>   }
>
> +G_GNUC_INTERNAL
> +gboolean spice_session_get_read_only(SpiceSession *self)
> +{
> +    g_return_val_if_fail(SPICE_IS_SESSION(self), FALSE);
> +
> +    return self->priv->read_only;
> +}
> +
>   /**
>    * spice_session_disconnect:
>    * @session:

Regards,

Hans


More information about the Spice-devel mailing list