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

[Victor Toso]

Hi,

On Sat, Jun 25, 2016 at 12:10:30PM +0200, Victor Toso wrote:
> 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.

I mean:
main_handle_ -> namespace for channel-main msg handler
main_agent_handle_msg() -> helper for main_handle_agent_data()

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

main_agent_handle_xfer_status() -> helper for main_agent_handle_msg()

> 
> > 
> > 
> > >  {
> > > +    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