> -----Original Message----- > From: Levente Kurusa [mailto:ilevex.linux@xxxxxxxxx] On Behalf Of Levente > Kurusa > Sent: Tuesday, January 21, 2014 11:01 AM > To: KY Srinivasan; gregkh@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > devel@xxxxxxxxxxxxxxxxxxxxxx; olaf@xxxxxxxxx; apw@xxxxxxxxxxxxx; > jasowang@xxxxxxxxxx > Subject: Re: [PATCH V3 1/1] Drivers: hv: Implement the file copy service > > 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 I will fix this. > > > + * 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 Good point; I will fix this. Thank you, K. Y _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
