>
> > + if (msg.type == SPICE_MSG_MAIN_AGENT_DATA)
> > + {
> > + var agent_data = new SpiceMsgcMainAgentData(0, msg.data);
>
> This should be a separate type; SpiceMsgMainAgentData, not Msgc.
>
> > +SpiceMainConn.prototype.file_xfer_start = function(file)
> > +{
> > + var task_id, xfer_start, task;
> > +
> > + if (typeof this.file_xfer_tasks === "undefined")
> > + this.file_xfer_tasks = {};
>
> Isn't that redundant to the initialization above?
>
Yes, it is.
> > +
> > + task_id = Object.keys(this.file_xfer_tasks).length;
>
> Aren't you running a risk of reusing task ids and running into
> possible
> collisions with this approach?
>
The task id is removed on the guest side as well when the task is completed, so there should be no problem. I can add the task counter.
>
> > + if (!this.file_xfer_tasks[file_xfer_status.id]) {
>
> This code base is mostly not K&R brace style; if you could follow
> that,
> I'd appreciate it.
>
>
> > + case VD_AGENT_FILE_XFER_STATUS_CANCELLED:
> > + xfer_error = "transfer is cancelled by spice agent";
> > + break;
>
> How common is this case? If it happens with any frequency, we may
> want
> to plan for some better clean up. (e.g. don't uselessly read +
> transmit
> a huge file after it's been canceled). You file onload callback
> could
> check for the existence of the task and abort if it vanishes.
>
It didn't happened during my tests, but the check should be there.
Thanks for the review, I will send v2.
Pavel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel