[Spice-devel] [PATCH spice-html5 3/4] Implement methods for transfering files from client to guest

[Jeremy White]

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

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

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

> +    var FILE_XFER_CHUNK_SIZE = VD_AGENT_MAX_DATA_SIZE - 20 - 12;
> +                    //VD_AGENT_MAX_DATA_SIZE - SpiceMsgcMainAgentData - VDAgentFileXferDataMessage;

Couldn't you call the actual buffer_size() functions?

Cheers,

Jeremy