From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:53097) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R6js2-0006Bg-IC for qemu-devel@nongnu.org; Thu, 22 Sep 2011 10:03:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R6jrw-00052F-Jx for qemu-devel@nongnu.org; Thu, 22 Sep 2011 10:03:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:9905) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R6jrw-00051s-BW for qemu-devel@nongnu.org; Thu, 22 Sep 2011 10:03:16 -0400 Date: Thu, 22 Sep 2011 17:01:00 +0300 From: Alon Levy Message-ID: <20110922140100.GD2875@bow.tlv.redhat.com> References: <1316620283-8330-1-git-send-email-yhalperi@redhat.com> <1316620283-8330-9-git-send-email-yhalperi@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1316620283-8330-9-git-send-email-yhalperi@redhat.com> Subject: Re: [Qemu-devel] [PATCH spice-server 08/13] server: move the linking of channels to a separate routine List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Yonit Halperin Cc: qemu-devel@nongnu.org, spice-devel@freedesktop.org, kraxel@redhat.com On Wed, Sep 21, 2011 at 06:51:18PM +0300, Yonit Halperin wrote: > > Signed-off-by: Yonit Halperin > --- > server/reds.c | 68 +++++++++++++++++++++++++++++++++++--------------------- > 1 files changed, 42 insertions(+), 26 deletions(-) > > diff --git a/server/reds.c b/server/reds.c > index bea0eb0..e7388a0 100644 > --- a/server/reds.c > +++ b/server/reds.c > @@ -2612,12 +2612,48 @@ static void inputs_init() > reds_register_channel(channel); > } > > +static void reds_send_input_channel_insecure_warn() > +{ > + RedsOutItem *item; > + SpiceMsgNotify notify; > + char *mess = "keyboard channel is insecure"; > + const int mess_len = strlen(mess); > + > + item = new_out_item(SPICE_MSG_NOTIFY); > + > + notify.time_stamp = get_time_stamp(); > + notify.severity = SPICE_NOTIFY_SEVERITY_WARN; > + notify.visibilty = SPICE_NOTIFY_VISIBILITY_HIGH; > + notify.what = SPICE_WARN_GENERAL; > + notify.message_len = mess_len; > + > + spice_marshall_msg_notify(item->m, ¬ify); > + spice_marshaller_add(item->m, (uint8_t *)mess, mess_len + 1); > + > + reds_push_pipe_item(item); > +} > + > +static void reds_channel_do_link(Channel *channel, SpiceLinkMess *link_msg, RedsStream *stream) > +{ > + uint32_t *caps; > + > + ASSERT(channel); > + ASSERT(link_msg); > + ASSERT(stream); > + > + if (link_msg->channel_type == SPICE_CHANNEL_INPUTS && !stream->ssl) { > + reds_send_input_channel_insecure_warn(); > + } > + caps = (uint32_t *)((uint8_t *)link_msg + link_msg->caps_offset); > + channel->link(channel, stream, FALSE, link_msg->num_common_caps, > + link_msg->num_common_caps ? caps : NULL, link_msg->num_channel_caps, > + link_msg->num_channel_caps ? caps + link_msg->num_common_caps : NULL); > +} > + > static void reds_handle_other_links(RedLinkInfo *link) > { > Channel *channel; > - RedsStream *stream; > SpiceLinkMess *link_mess; > - uint32_t *caps; > > link_mess = link->link_mess; > > @@ -2635,36 +2671,16 @@ static void reds_handle_other_links(RedLinkInfo *link) > } > > reds_send_link_result(link, SPICE_LINK_ERR_OK); > + // TODO: if mig_target, where this call should be? here or only when the links are handled Both? add a prefix argument to reds_show_new_channel maybe. > reds_show_new_channel(link, reds->link_id); > - if (link_mess->channel_type == SPICE_CHANNEL_INPUTS && !link->stream->ssl) { > - RedsOutItem *item; > - SpiceMsgNotify notify; > - char *mess = "keyboard channel is insecure"; > - const int mess_len = strlen(mess); > - > - item = new_out_item(SPICE_MSG_NOTIFY); > - > - notify.time_stamp = get_time_stamp(); > - notify.severity = SPICE_NOTIFY_SEVERITY_WARN; > - notify.visibilty = SPICE_NOTIFY_VISIBILITY_HIGH; > - notify.what = SPICE_WARN_GENERAL; > - notify.message_len = mess_len; > + reds_stream_remove_watch(link->stream); > > - spice_marshall_msg_notify(item->m, ¬ify); > - spice_marshaller_add(item->m, (uint8_t *)mess, mess_len + 1); > + reds_channel_do_link(channel, link->link_mess, link->stream); Would be nice to consistently use link_mess or link->link_mess, not both. > + free(link_mess); > > - reds_push_pipe_item(item); > - } > - stream = link->stream; > - reds_stream_remove_watch(stream); > link->stream = NULL; > link->link_mess = NULL; > reds_link_free(link); > - caps = (uint32_t *)((uint8_t *)link_mess + link_mess->caps_offset); > - channel->link(channel, stream, reds->mig_target, link_mess->num_common_caps, > - link_mess->num_common_caps ? caps : NULL, link_mess->num_channel_caps, > - link_mess->num_channel_caps ? caps + link_mess->num_common_caps : NULL); > - free(link_mess); > } > > static void reds_handle_link(RedLinkInfo *link) > -- > 1.7.4.4 >