[Spice-devel] [PATCH 06/26] server/red_channel (all): add peer to config_socket sig

Alon Levy alevy at redhat.com
Sun Feb 20 00:44:28 PST 2011


On Tue, Feb 15, 2011 at 02:43:51AM +0100, Marc-André Lureau wrote:
> Motivation? (it's loss of encapsulation, imho)
> 

Because the peer belongs to a specific connection, which right now is
synonymous with a channel, but later becomes not so, actually it belongs
to a RedChannelClient (an entity I have not introduced yet but it's ).
So basically, I agree that it looks like I'm breaking encapsulation, but
actually I'm just trying to do small incremental changes, the end result
will be that peer belongs to RedChannelClient, and the callback is still
in RedChannel. Later perhaps it should move to RedChannelClient too, and
then the added parameter can get lost and return to being either accessed
directly or via an accessor function.

> On Fri, Feb 11, 2011 at 6:48 PM, Alon Levy <alevy at redhat.com> wrote:
> > ---
> >  server/inputs_channel.c    |    8 ++++----
> >  server/main_channel.c      |    2 +-
> >  server/red_channel.c       |    2 +-
> >  server/red_channel.h       |    2 +-
> >  server/red_tunnel_worker.c |    8 ++++----
> >  server/red_worker.c        |    3 +--
> >  server/smartcard.c         |    2 +-
> >  7 files changed, 13 insertions(+), 14 deletions(-)
> >
> > diff --git a/server/inputs_channel.c b/server/inputs_channel.c
> > index ee0e833..8346e47 100644
> > --- a/server/inputs_channel.c
> > +++ b/server/inputs_channel.c
> > @@ -485,19 +485,19 @@ static void inputs_pipe_add_init(InputsChannel *inputs_channel)
> >     red_channel_pipe_add_push(&inputs_channel->base, &item->base);
> >  }
> >
> > -static int inputs_channel_config_socket(RedChannel *channel)
> > +static int inputs_channel_config_socket(RedChannel *channel, RedsStreamContext *peer)
> >  {
> >     int flags;
> >     int delay_val = 1;
> >
> > -    if (setsockopt(channel->peer->socket, IPPROTO_TCP, TCP_NODELAY,
> > +    if (setsockopt(peer->socket, IPPROTO_TCP, TCP_NODELAY,
> >             &delay_val, sizeof(delay_val)) == -1) {
> >         red_printf("setsockopt failed, %s", strerror(errno));
> >         return FALSE;
> >     }
> >
> > -    if ((flags = fcntl(channel->peer->socket, F_GETFL)) == -1 ||
> > -                 fcntl(channel->peer->socket, F_SETFL, flags | O_ASYNC) == -1) {
> > +    if ((flags = fcntl(peer->socket, F_GETFL)) == -1 ||
> > +                 fcntl(peer->socket, F_SETFL, flags | O_ASYNC) == -1) {
> >         red_printf("fcntl failed, %s", strerror(errno));
> >         return FALSE;
> >     }
> > diff --git a/server/main_channel.c b/server/main_channel.c
> > index 9c5cc74..21e0156 100644
> > --- a/server/main_channel.c
> > +++ b/server/main_channel.c
> > @@ -776,7 +776,7 @@ static void main_channel_release_msg_rcv_buf(RedChannel *channel, SpiceDataHeade
> >  {
> >  }
> >
> > -static int main_channel_config_socket(RedChannel *channel)
> > +static int main_channel_config_socket(RedChannel *channel, RedsStreamContext *peer)
> >  {
> >     return TRUE;
> >  }
> > diff --git a/server/red_channel.c b/server/red_channel.c
> > index b729e9f..bd61685 100644
> > --- a/server/red_channel.c
> > +++ b/server/red_channel.c
> > @@ -369,7 +369,7 @@ RedChannel *red_channel_create(int size, RedsStreamContext *peer,
> >
> >     channel->shut = 0; // came here from inputs, perhaps can be removed? XXX
> >
> > -    if (!config_socket(channel)) {
> > +    if (!config_socket(channel, peer)) {
> >         goto error;
> >     }
> >
> > diff --git a/server/red_channel.h b/server/red_channel.h
> > index 83cbbde..04b8aba 100644
> > --- a/server/red_channel.h
> > +++ b/server/red_channel.h
> > @@ -115,7 +115,7 @@ typedef int (*channel_handle_message_proc)(RedChannel *channel,
> >  typedef void (*channel_release_msg_recv_buf_proc)(RedChannel *channel,
> >                                                   SpiceDataHeader *msg_header, uint8_t *msg);
> >  typedef void (*channel_disconnect_proc)(RedChannel *channel);
> > -typedef int (*channel_configure_socket_proc)(RedChannel *channel);
> > +typedef int (*channel_configure_socket_proc)(RedChannel *channel, RedsStreamContext *peer);
> >  typedef void (*channel_send_pipe_item_proc)(RedChannel *channel, SpiceMarshaller *m, PipeItem *item);
> >  typedef void (*channel_hold_pipe_item_proc)(RedChannel *channel, PipeItem *item);
> >  typedef void (*channel_release_pipe_item_proc)(RedChannel *channel,
> > diff --git a/server/red_tunnel_worker.c b/server/red_tunnel_worker.c
> > index 10948b4..8fa70d6 100644
> > --- a/server/red_tunnel_worker.c
> > +++ b/server/red_tunnel_worker.c
> > @@ -3331,24 +3331,24 @@ static void arm_timer(SlirpUsrNetworkInterface *usr_interface, UserTimer *timer,
> >  * channel interface and other related procedures
> >  ************************************************/
> >
> > -static int tunnel_channel_config_socket(RedChannel *channel)
> > +static int tunnel_channel_config_socket(RedChannel *channel, RedsStreamContext *peer)
> >  {
> >     int flags;
> >     int delay_val;
> >
> > -    if ((flags = fcntl(channel->peer->socket, F_GETFL)) == -1) {
> > +    if ((flags = fcntl(peer->socket, F_GETFL)) == -1) {
> >         red_printf("accept failed, %s", strerror(errno)); // can't we just use red_error?
> >         return FALSE;
> >     }
> >
> > -    if (fcntl(channel->peer->socket, F_SETFL, flags | O_NONBLOCK) == -1) {
> > +    if (fcntl(peer->socket, F_SETFL, flags | O_NONBLOCK) == -1) {
> >         red_printf("accept failed, %s", strerror(errno));
> >         return FALSE;
> >     }
> >
> >     delay_val = 1;
> >
> > -    if (setsockopt(channel->peer->socket, IPPROTO_TCP, TCP_NODELAY, &delay_val,
> > +    if (setsockopt(peer->socket, IPPROTO_TCP, TCP_NODELAY, &delay_val,
> >                    sizeof(delay_val)) == -1) {
> >         red_printf("setsockopt failed, %s", strerror(errno));
> >     }
> > diff --git a/server/red_worker.c b/server/red_worker.c
> > index b79c1b6..d7fab60 100644
> > --- a/server/red_worker.c
> > +++ b/server/red_worker.c
> > @@ -9022,11 +9022,10 @@ static int display_channel_handle_message(RedChannel *channel, uint32_t size, ui
> >     }
> >  }
> >
> > -int common_channel_config_socket(RedChannel *channel)
> > +int common_channel_config_socket(RedChannel *channel, RedsStreamContext *peer)
> >  {
> >     int flags;
> >     int delay_val;
> > -    RedsStreamContext *peer = channel->peer;
> >
> >     if ((flags = fcntl(peer->socket, F_GETFL)) == -1) {
> >         red_printf("accept failed, %s", strerror(errno));
> > diff --git a/server/smartcard.c b/server/smartcard.c
> > index 66f57a4..e66ed1c 100644
> > --- a/server/smartcard.c
> > +++ b/server/smartcard.c
> > @@ -239,7 +239,7 @@ static void smartcard_char_device_detach(
> >     smartcard_channel_write_to_reader(smartcard_channel, &vheader);
> >  }
> >
> > -static int smartcard_channel_config_socket(RedChannel *channel)
> > +static int smartcard_channel_config_socket(RedChannel *channel, RedsStreamContext *peer)
> >  {
> >     return TRUE;
> >  }
> > --
> > 1.7.4
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> >
> 
> 
> 
> -- 
> Marc-André Lureau


More information about the Spice-devel mailing list