[Spice-devel] [spice-gtk v4 08/24] main: let channel-main handle file-xfer messages

[Victor Toso]

On Fri, Jun 24, 2016 at 02:23:08PM -0500, Jonathon Jongsma wrote:
> On Thu, 2016-06-23 at 19:37 +0200, Victor Toso wrote:
> > By separating SpiceFileTransferTask from channel-main, we could choose
> > where to put the handler for messages. With the approach based on
> > spice_file_transfer_task_read_async(), we cannot have it under
> > spice-file-transfer-task.c due the need of callback and userdata on
> > _read_async.
> > 
> > It is much easier to keep this in channel-main and do not move any
> > VDAgent.
> > 
> > This patch reverts 349a52ca2d6af4b31a4f51c38a3292c04460953c changes
> > but renaming variable task to xfer_task.
> > ---
> >  src/channel-main.c | 29 ++++++++++++-----------------
> >  1 file changed, 12 insertions(+), 17 deletions(-)
> > 
> > diff --git a/src/channel-main.c b/src/channel-main.c
> > index 4b728fe..be5a454 100644
> > --- a/src/channel-main.c
> > +++ b/src/channel-main.c
> > @@ -1983,17 +1983,22 @@ static void file_xfer_read_async_cb(GObject
> > *source_object,
> >  }
> >  
> >  /* coroutine context */
> > -static void spice_file_transfer_task_handle_status(SpiceFileTransferTask
> > *task,
> > -                                                   VDAgentFileXferStatusMessa
> > ge *msg)
> > +static void file_xfer_handle_status(SpiceMainChannel *channel,
> > +                                    VDAgentFileXferStatusMessage *msg)
> 
> since it's moving back to a channel function. perhaps a name like
> 
> main_channel_handle_xfer_status()??

Indeed, file_xfer_ namespace and handling VDAgent messages are not
exactly compatible.

main_handle_ is used for channel-main messages with
main_agent_handle_msg being a helper for a helper for
main_agent_handle_msg.

file_xfer_handle_status is a helper of main_agent_handle_msg so I would
go for main_agent_handle_xfer_status() ?

> 
> 
> >  {
> > +    SpiceFileTransferTask *xfer_task;
> > +    SpiceMainChannelPrivate *c;
> >      GError *error = NULL;
> > -    g_return_if_fail(task != NULL);
> >  
> > -    SPICE_DEBUG("task %u received response %u", msg->id, msg->result);
> > +    c = channel->priv;
> > +    xfer_task = g_hash_table_lookup(c->file_xfer_tasks, GUINT_TO_POINTER(msg-
> > >id));
> > +    g_return_if_fail(xfer_task != NULL);
> > +
> > +    SPICE_DEBUG("xfer-task %u received response %u", msg->id, msg->result);
> >  
> >      switch (msg->result) {
> >      case VD_AGENT_FILE_XFER_STATUS_CAN_SEND_DATA:
> > -        spice_file_transfer_task_read_async(task, file_xfer_read_async_cb,
> > NULL);
> > +        spice_file_transfer_task_read_async(xfer_task,
> > file_xfer_read_async_cb, NULL);
> >          return;
> >      case VD_AGENT_FILE_XFER_STATUS_CANCELLED:
> >          error = g_error_new(SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> > @@ -2012,7 +2017,7 @@ static void
> > spice_file_transfer_task_handle_status(SpiceFileTransferTask *task,
> >          break;
> >      }
> >  
> > -    spice_file_transfer_task_completed(task, error);
> > +    spice_file_transfer_task_completed(xfer_task, error);
> >  }
> >  
> >  /* any context: the message is not flushed immediately,
> > @@ -2156,18 +2161,8 @@ static void main_agent_handle_msg(SpiceChannel
> > *channel,
> >          break;
> >      }
> >      case VD_AGENT_FILE_XFER_STATUS:
> > -    {
> > -        SpiceFileTransferTask *task;
> > -        VDAgentFileXferStatusMessage *msg = payload;
> > -
> > -        task = g_hash_table_lookup(c->file_xfer_tasks, GUINT_TO_POINTER(msg-
> > >id));
> > -        if (task != NULL) {
> > -            spice_file_transfer_task_handle_status(task, msg);
> > -        } else {
> > -            SPICE_DEBUG("cannot find task %u", msg->id);
> > -        }
> > +        file_xfer_handle_status(self, payload);
> >          break;
> > -    }
> >      default:
> >          g_warning("unhandled agent message type: %u (%s), size %u",
> >                    msg->type, NAME(agent_msg_types, msg->type), msg->size);
> 
> 
> Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

Thanks,
  toso

> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel