>
> From: Victor Toso <me@xxxxxxxxxxxxxx>
>
> SpiceChannel provides some internal utility functions that this patch
> takes advantage of:
>
> * spice_channel_get_channel_type()
> * spice_channel_get_channel_id()
> * spice_channel_get_state()
>
> Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
The question is also why all these files have access to "private" fields?
How much NOT private they are? Looks like is more an "exported" than a private.
> ---
> src/channel-inputs.c | 65 +++++++++++++++++++++++++++++---------------
> src/channel-main.c | 28 +++++++++----------
> src/spice-audio.c | 6 ++--
> src/spice-session.c | 3 +-
> 4 files changed, 62 insertions(+), 40 deletions(-)
>
> diff --git a/src/channel-inputs.c b/src/channel-inputs.c
> index a8cdd23..73cbb0f 100644
> --- a/src/channel-inputs.c
> +++ b/src/channel-inputs.c
> @@ -304,14 +304,20 @@ void spice_inputs_channel_motion(SpiceInputsChannel
> *channel, gint dx, gint dy,
> gint button_state)
> {
> SpiceInputsChannelPrivate *c;
> + gint channel_state;
Why not enum spice_channel_state ?
Here and in different hunks
>
> g_return_if_fail(channel != NULL);
> - g_return_if_fail(SPICE_CHANNEL(channel)->priv->state !=
> SPICE_CHANNEL_STATE_UNCONNECTED);
> - if (SPICE_CHANNEL(channel)->priv->state != SPICE_CHANNEL_STATE_READY)
> +
> + channel_state = spice_channel_get_state(SPICE_CHANNEL(channel));
> + g_return_if_fail(channel_state != SPICE_CHANNEL_STATE_UNCONNECTED);
> +
> + if (channel_state != SPICE_CHANNEL_STATE_READY) {
> return;
> + }
>
> - if (dx == 0 && dy == 0)
> + if (dx == 0 && dy == 0) {
> return;
> + }
>
> c = channel->priv;
> c->bs = button_state;
> @@ -360,8 +366,9 @@ void spice_inputs_channel_position(SpiceInputsChannel
> *channel, gint x, gint y,
>
> g_return_if_fail(channel != NULL);
>
> - if (SPICE_CHANNEL(channel)->priv->state != SPICE_CHANNEL_STATE_READY)
> + if (spice_channel_get_state(SPICE_CHANNEL(channel)) !=
> SPICE_CHANNEL_STATE_READY) {
> return;
> + }
>
> c = channel->priv;
> c->bs = button_state;
> @@ -411,10 +418,12 @@ void
> spice_inputs_channel_button_press(SpiceInputsChannel *channel, gint button,
>
> g_return_if_fail(channel != NULL);
>
> - if (SPICE_CHANNEL(channel)->priv->state != SPICE_CHANNEL_STATE_READY)
> + if (spice_channel_get_state(SPICE_CHANNEL(channel)) !=
> SPICE_CHANNEL_STATE_READY) {
> return;
> - if (spice_channel_get_read_only(SPICE_CHANNEL(channel)))
> + }
> + if (spice_channel_get_read_only(SPICE_CHANNEL(channel))) {
> return;
> + }
>
> c = channel->priv;
> switch (button) {
> @@ -476,10 +485,12 @@ void
> spice_inputs_channel_button_release(SpiceInputsChannel *channel, gint butto
>
> g_return_if_fail(channel != NULL);
>
> - if (SPICE_CHANNEL(channel)->priv->state != SPICE_CHANNEL_STATE_READY)
> + if (spice_channel_get_state(SPICE_CHANNEL(channel)) !=
> SPICE_CHANNEL_STATE_READY) {
> return;
> - if (spice_channel_get_read_only(SPICE_CHANNEL(channel)))
> + }
> + if (spice_channel_get_read_only(SPICE_CHANNEL(channel))) {
> return;
> + }
>
> c = channel->priv;
> switch (button) {
> @@ -535,13 +546,18 @@ void spice_inputs_channel_key_press(SpiceInputsChannel
> *channel, guint scancode)
> {
> SpiceMsgcKeyDown down;
> SpiceMsgOut *msg;
> + gint channel_state;
>
> g_return_if_fail(channel != NULL);
> - g_return_if_fail(SPICE_CHANNEL(channel)->priv->state !=
> SPICE_CHANNEL_STATE_UNCONNECTED);
> - if (SPICE_CHANNEL(channel)->priv->state != SPICE_CHANNEL_STATE_READY)
> +
> + channel_state = spice_channel_get_state(SPICE_CHANNEL(channel));
> + g_return_if_fail(channel_state != SPICE_CHANNEL_STATE_UNCONNECTED);
> + if (channel_state != SPICE_CHANNEL_STATE_READY) {
> return;
> - if (spice_channel_get_read_only(SPICE_CHANNEL(channel)))
> + }
> + if (spice_channel_get_read_only(SPICE_CHANNEL(channel))) {
> return;
> + }
>
> down.code = spice_make_scancode(scancode, FALSE);
> msg = spice_msg_out_new(SPICE_CHANNEL(channel),
> SPICE_MSGC_INPUTS_KEY_DOWN);
> @@ -578,13 +594,18 @@ void
> spice_inputs_channel_key_release(SpiceInputsChannel *channel, guint scancod
> {
> SpiceMsgcKeyUp up;
> SpiceMsgOut *msg;
> + gint channel_state;
>
> g_return_if_fail(channel != NULL);
> - g_return_if_fail(SPICE_CHANNEL(channel)->priv->state !=
> SPICE_CHANNEL_STATE_UNCONNECTED);
> - if (SPICE_CHANNEL(channel)->priv->state != SPICE_CHANNEL_STATE_READY)
> +
> + channel_state = spice_channel_get_state(SPICE_CHANNEL(channel));
> + g_return_if_fail(channel_state != SPICE_CHANNEL_STATE_UNCONNECTED);
> + if (channel_state != SPICE_CHANNEL_STATE_READY) {
> return;
> - if (spice_channel_get_read_only(SPICE_CHANNEL(channel)))
> + }
> + if (spice_channel_get_read_only(SPICE_CHANNEL(channel))) {
> return;
> + }
>
> up.code = spice_make_scancode(scancode, TRUE);
> msg = spice_msg_out_new(SPICE_CHANNEL(channel),
> SPICE_MSGC_INPUTS_KEY_UP);
> @@ -624,13 +645,14 @@ void
> spice_inputs_channel_key_press_and_release(SpiceInputsChannel *input_channe
> SpiceChannel *channel = SPICE_CHANNEL(input_channel);
>
> g_return_if_fail(channel != NULL);
> - g_return_if_fail(channel->priv->state !=
> SPICE_CHANNEL_STATE_UNCONNECTED);
> + g_return_if_fail(spice_channel_get_state(channel) !=
> SPICE_CHANNEL_STATE_UNCONNECTED);
>
> - if (channel->priv->state != SPICE_CHANNEL_STATE_READY)
> + if (spice_channel_get_state(channel) != SPICE_CHANNEL_STATE_READY) {
> return;
> - if (spice_channel_get_read_only(channel))
> + }
> + if (spice_channel_get_read_only(channel)) {
> return;
> -
> + }
> if (spice_channel_test_capability(channel,
> SPICE_INPUTS_CAP_KEY_SCANCODE)) {
> SpiceMsgOut *msg;
> guint16 code;
> @@ -664,16 +686,15 @@ static SpiceMsgOut* set_key_locks(SpiceInputsChannel
> *channel, guint locks)
> SpiceMsgcKeyModifiers modifiers;
> SpiceMsgOut *msg;
> SpiceInputsChannelPrivate *ic;
> - SpiceChannelPrivate *c;
>
> g_return_val_if_fail(SPICE_IS_INPUTS_CHANNEL(channel), NULL);
>
> ic = channel->priv;
> - c = SPICE_CHANNEL(channel)->priv;
> -
> ic->locks = locks;
> - if (c->state != SPICE_CHANNEL_STATE_READY)
> +
> + if (spice_channel_get_state(SPICE_CHANNEL(channel)) !=
> SPICE_CHANNEL_STATE_READY) {
> return NULL;
> + }
>
> msg = spice_msg_out_new(SPICE_CHANNEL(channel),
> SPICE_MSGC_INPUTS_KEY_MODIFIERS);
> diff --git a/src/channel-main.c b/src/channel-main.c
> index 334be7d..ac0d408 100644
> --- a/src/channel-main.c
> +++ b/src/channel-main.c
> @@ -2204,14 +2204,13 @@ static void migrate_channel_event_cb(SpiceChannel
> *channel, SpiceChannelEvent ev
> gpointer data)
> {
> spice_migrate *mig = data;
> - SpiceChannelPrivate *c = SPICE_CHANNEL(channel)->priv;
>
> g_return_if_fail(mig->nchannels > 0);
> g_signal_handlers_disconnect_by_func(channel, migrate_channel_event_cb,
> data);
>
> switch (event) {
> case SPICE_CHANNEL_OPENED:
> - if (c->channel_type == SPICE_CHANNEL_MAIN) {
> + if (spice_channel_get_channel_type(channel) == SPICE_CHANNEL_MAIN) {
> SpiceSession *session =
> spice_channel_get_session(mig->src_channel);
> if (mig->do_seamless) {
> SpiceMainChannelPrivate *main_priv =
> SPICE_MAIN_CHANNEL(channel)->priv;
> @@ -2227,11 +2226,15 @@ static void migrate_channel_event_cb(SpiceChannel
> *channel, SpiceChannelEvent ev
> GList *channels, *l;
> l = channels = spice_session_get_channels(session);
> while (l != NULL) {
> - SpiceChannelPrivate *curc = SPICE_CHANNEL(l->data)->priv;
> + SpiceChannel *it = SPICE_CHANNEL(l->data);
> +
> l = l->next;
> - if (curc->channel_type == SPICE_CHANNEL_MAIN)
> + if (spice_channel_get_channel_type(it) ==
> SPICE_CHANNEL_MAIN) {
> continue;
> - migrate_channel_connect(mig, curc->channel_type,
> curc->channel_id);
> + }
> + migrate_channel_connect(mig,
> + spice_channel_get_channel_type(it),
> + spice_channel_get_channel_id(it));
> }
> g_list_free(channels);
> } else {
> @@ -2254,10 +2257,10 @@ static void migrate_channel_event_cb(SpiceChannel
> *channel, SpiceChannelEvent ev
> static gboolean main_migrate_handshake_done(gpointer data)
> {
> spice_migrate *mig = data;
> - SpiceChannelPrivate *c = SPICE_CHANNEL(mig->dst_channel)->priv;
> + SpiceChannel *channel = SPICE_CHANNEL(mig->dst_channel);
>
> - g_return_val_if_fail(c->channel_type == SPICE_CHANNEL_MAIN, FALSE);
> - g_return_val_if_fail(c->state ==
> SPICE_CHANNEL_STATE_MIGRATION_HANDSHAKE, FALSE);
> + g_return_val_if_fail(spice_channel_get_channel_type(channel) ==
> SPICE_CHANNEL_MAIN, FALSE);
> + g_return_val_if_fail(spice_channel_get_state(channel) ==
> SPICE_CHANNEL_STATE_MIGRATION_HANDSHAKE, FALSE);
>
> c->state = SPICE_CHANNEL_STATE_MIGRATING;
> mig->nchannels--;
> @@ -2407,20 +2410,18 @@ static void
> main_handle_migrate_begin_seamless(SpiceChannel *channel, SpiceMsgIn
>
> static void main_handle_migrate_dst_seamless_ack(SpiceChannel *channel,
> SpiceMsgIn *in)
> {
> - SpiceChannelPrivate *c = SPICE_CHANNEL(channel)->priv;
> SpiceMainChannelPrivate *main_priv = SPICE_MAIN_CHANNEL(channel)->priv;
>
> - g_return_if_fail(c->state == SPICE_CHANNEL_STATE_MIGRATION_HANDSHAKE);
> + g_return_if_fail(spice_channel_get_state(channel) ==
> SPICE_CHANNEL_STATE_MIGRATION_HANDSHAKE);
> main_priv->migrate_data->do_seamless = true;
> g_idle_add(main_migrate_handshake_done, main_priv->migrate_data);
> }
>
> static void main_handle_migrate_dst_seamless_nack(SpiceChannel *channel,
> SpiceMsgIn *in)
> {
> - SpiceChannelPrivate *c = SPICE_CHANNEL(channel)->priv;
> SpiceMainChannelPrivate *main_priv = SPICE_MAIN_CHANNEL(channel)->priv;
>
> - g_return_if_fail(c->state == SPICE_CHANNEL_STATE_MIGRATION_HANDSHAKE);
> + g_return_if_fail(spice_channel_get_state(channel) ==
> SPICE_CHANNEL_STATE_MIGRATION_HANDSHAKE);
> main_priv->migrate_data->do_seamless = false;
> g_idle_add(main_migrate_handshake_done, main_priv->migrate_data);
> }
> @@ -2551,11 +2552,10 @@ static void spice_main_handle_msg(SpiceChannel
> *channel, SpiceMsgIn *msg)
> {
> int type = spice_msg_in_type(msg);
> SpiceChannelClass *parent_class;
> - SpiceChannelPrivate *c = SPICE_CHANNEL(channel)->priv;
>
> parent_class = SPICE_CHANNEL_CLASS(spice_main_channel_parent_class);
>
> - if (c->state == SPICE_CHANNEL_STATE_MIGRATION_HANDSHAKE) {
> + if (spice_channel_get_state(channel) ==
> SPICE_CHANNEL_STATE_MIGRATION_HANDSHAKE) {
> if (type != SPICE_MSG_MAIN_MIGRATE_DST_SEAMLESS_ACK &&
> type != SPICE_MSG_MAIN_MIGRATE_DST_SEAMLESS_NACK) {
> g_critical("unexpected msg (%d)."
> diff --git a/src/spice-audio.c b/src/spice-audio.c
> index daf62df..10b31d7 100644
> --- a/src/spice-audio.c
> +++ b/src/spice-audio.c
> @@ -145,11 +145,13 @@ static void spice_audio_init(SpiceAudio *self)
>
> static void connect_channel(SpiceAudio *self, SpiceChannel *channel)
> {
> - if (channel->priv->state != SPICE_CHANNEL_STATE_UNCONNECTED)
> + if (spice_channel_get_state(channel) != SPICE_CHANNEL_STATE_UNCONNECTED)
> {
> return;
> + }
>
> - if (SPICE_AUDIO_GET_CLASS(self)->connect_channel(self, channel))
> + if (SPICE_AUDIO_GET_CLASS(self)->connect_channel(self, channel)) {
> spice_channel_connect(channel);
> + }
> }
>
> static void update_audio_channels(SpiceAudio *self, SpiceSession *session)
> diff --git a/src/spice-session.c b/src/spice-session.c
> index d0d9e54..afeb724 100644
> --- a/src/spice-session.c
> +++ b/src/spice-session.c
> @@ -2203,7 +2203,6 @@ GSocketConnection*
> spice_session_channel_open_host(SpiceSession *session, SpiceC
> g_return_val_if_fail(SPICE_IS_SESSION(session), NULL);
>
> SpiceSessionPrivate *s = session->priv;
> - SpiceChannelPrivate *c = channel->priv;
> spice_open_host open_host = { 0, };
> gchar *port, *endptr;
>
> @@ -2212,7 +2211,7 @@ GSocketConnection*
> spice_session_channel_open_host(SpiceSession *session, SpiceC
> open_host.session = session;
> open_host.channel = channel;
>
> - const char *name = spice_channel_type_to_string(c->channel_type);
> + const char *name =
> spice_channel_type_to_string(spice_channel_get_channel_type(channel));
> if (spice_strv_contains(s->secure_channels, "all") ||
> spice_strv_contains(s->secure_channels, name))
> *use_tls = TRUE;
By the way, the patch does not compile.
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel