Hello,
On 01/21/2014 08:06 PM, K. Y. Srinivasan wrote:
> Implement the file copy service for Linux guests on Hyper-V. This permits the
> host to copy a file (over VMBUS) into the guest. This facility is part of
> "guest integration services" supported on the Windows platform.
> Here is a link that provides additional details on this functionality:
>
> http://technet.microsoft.com/en-us/library/dn464282.aspx
>
> In V1 version of the patch I have addressed comments from
> Olaf Hering <o...@aepfle.de> and Dan Carpenter <dan.carpen...@oracle.com>
>
> In V2 version of this patch I did some minor cleanup (making some globals
> static). In this version of the patch I have addressed all of Olaf's
> most recent set of comments/concerns.
>
> Signed-off-by: K. Y. Srinivasan <k...@microsoft.com>
> ---
Just a few comments.
> + /*
> + * The strings sent from the host are encoded in
> + * in utf16; convert it to utf8 strings.
> + * The host assures us that the utf16 strings will not exceed
> + * the max lengths specified. We will however, reserve room
> + * for the string terminating character - in the utf16s_utf8s()
> + * function we limit the size of the buffer where the converted
> + * string is placed to W_MAX_PATH -1 to gaurantee
s/gaurantee/guarantee
> + * that the strings can be properly terminated!
> + */
> +
> + switch (operation) {
> + case START_FILE_COPY:
> + memset(smsg_out, 0, sizeof(struct hv_start_fcopy));
> + smsg_out->hdr.operation = operation;
> + smsg_in = (struct hv_start_fcopy *)fcopy_transaction.fcopy_msg;
> +
> + utf16s_to_utf8s((wchar_t *)smsg_in->file_name, W_MAX_PATH,
> + UTF16_LITTLE_ENDIAN,
> + (__u8 *)smsg_out->file_name, W_MAX_PATH - 1);
> +
> + utf16s_to_utf8s((wchar_t *)smsg_in->path_name, W_MAX_PATH,
> + UTF16_LITTLE_ENDIAN,
> + (__u8 *)smsg_out->path_name, W_MAX_PATH - 1);
> +
> + smsg_out->copy_flags = smsg_in->copy_flags;
> + smsg_out->file_size = smsg_in->file_size;
> + break;
> +
> + default:
> + break;
> + }
> + up(&fcopy_transaction.read_sema);
> + return;
> +}
> +
> [...]
> +static ssize_t fcopy_read(struct file *file, char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + int ret;
> + void *src;
> + size_t copy_size;
> + int operation;
> +
> + /*
> + * Wait until there is something to be read.
> + */
> + ret = down_interruptible(&fcopy_transaction.read_sema);
> + if (ret)
> + return -EINTR;
I don't know, but since you use 'ret' only once and you don't
even read it after, you could just simply remove it and check
the return code of down_interruptible() directly
> +
> + operation = fcopy_transaction.fcopy_msg->operation;
> +
> + if (operation == START_FILE_COPY) {
> + src = &fcopy_transaction.message;
> + copy_size = sizeof(struct hv_start_fcopy);
> + if (count < copy_size)
> + return 0;
> + } else {
> + src = fcopy_transaction.fcopy_msg;
> + copy_size = sizeof(struct hv_do_fcopy);
> + if (count < copy_size)
> + return 0;
> + }
> + if (copy_to_user(buf, src, copy_size))
> + return -EFAULT;
...like you do here.
--
Regards,
Levente Kurusa
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
