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 <olaf@xxxxxxxxx> and Dan Carpenter <dan.carpenter@xxxxxxxxxx> > > 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 <kys@xxxxxxxxxxxxx> > --- 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 devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
