[Spice-devel] [PATCH spice-server 5/6] red-channel-client: Change initialization order

[Jonathon Jongsma]

On Mon, 2016-10-31 at 06:16 -0400, Frediano Ziglio wrote:
> > 
> > 
> > On Fri, 2016-10-28 at 11:59 +0100, Frediano Ziglio wrote:
> > > 
> > > A bit more similar to previous
> > 
> > The patch looks fine to me, but it's not clear to me why it's an
> > improvement. Can you expand on that in the commit log at least?
> > 
> 
> Not that I found a problem (currently) with the change however
> the order change was not related to GObject itself.
> Reverting back ensures that this order is and was tested more
> than the actual one.
> 
> Frediano

Ah, I had misunderstood your commit message "a bit more similar to
previous". For some reason, I thought you meant that this patch was
similar to the previous patch in the series. Now I see that you mean
that this order is similar to how it used to be before refactoring. 

ACK with slightly improved commit log:

"Make the order of initialization closer to what it was before
conversion to GObject"

Or something like that.

Acked-by: Jonathon Jongsma <jjongsma at redhat.com>


> 
> > 
> > > 
> > > 
> > > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > > ---
> > >  server/red-channel-client.c | 28 +++++++++++++++-------------
> > >  1 file changed, 15 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/server/red-channel-client.c b/server/red-channel-
> > > client.c
> > > index 6c78237..42f1d2c 100644
> > > --- a/server/red-channel-client.c
> > > +++ b/server/red-channel-client.c
> > > @@ -896,7 +896,22 @@ static gboolean
> > > red_channel_client_initable_init(GInitable *initable,
> > >          goto cleanup;
> > >      }
> > >  
> > > +    if (!red_channel_config_socket(self->priv->channel, self)) {
> > > +        g_set_error_literal(&local_error,
> > > +                            SPICE_SERVER_ERROR,
> > > +                            SPICE_SERVER_ERROR_FAILED,
> > > +                            "Unable to configure socket");
> > > +        goto cleanup;
> > > +    }
> > > +
> > >      core = red_channel_get_core_interface(self->priv->channel);
> > > +    if (self->priv->stream)
> > > +        self->priv->stream->watch =
> > > +            core->watch_add(core, self->priv->stream->socket,
> > > +                            SPICE_WATCH_EVENT_READ,
> > > +                            red_channel_client_event,
> > > +                            self);
> > > +
> > >      if (self->priv->monitor_latency
> > >          && reds_stream_get_family(self->priv->stream) !=
> > > AF_UNIX) {
> > >          self->priv->latency_monitor.timer =
> > > @@ -909,23 +924,10 @@ static gboolean
> > > red_channel_client_initable_init(GInitable *initable,
> > >          self->priv->latency_monitor.roundtrip = -1;
> > >      }
> > >  
> > > -    if (self->priv->stream)
> > > -        self->priv->stream->watch =
> > > -            core->watch_add(core, self->priv->stream->socket,
> > > -                            SPICE_WATCH_EVENT_READ,
> > > -                            red_channel_client_event,
> > > -                            self);
> > >      self->priv->id = red_channel_get_n_clients(self->priv-
> > > >channel);
> > >      red_channel_add_client(self->priv->channel, self);
> > >      red_client_add_channel(self->priv->client, self);
> > >  
> > > -    if (!red_channel_config_socket(self->priv->channel, self)) {
> > > -        g_set_error_literal(&local_error,
> > > -                            SPICE_SERVER_ERROR,
> > > -                            SPICE_SERVER_ERROR_FAILED,
> > > -                            "Unable to configure socket");
> > > -    }
> > > -
> > >  cleanup:
> > >      pthread_mutex_unlock(&self->priv->client->lock);
> > >      if (local_error) {
> >