RE: [PATCH V2 1/1] Drivers: hv: Implement the file copy service

From: KY Srinivasan
Date: Mon Jan 20 2014 - 17:13:40 EST


Olaf,

I am cleaning up the code based on your feedback. By the time I am done with my cleanup, I doubt if
this patch would apply. Do you mind if I were to include your changes here as part of my cleanup?

Thank you,

K. Y

> -----Original Message-----
> From: Olaf Hering [mailto:olaf@xxxxxxxxx]
> Sent: Thursday, January 16, 2014 2:49 AM
> To: KY Srinivasan
> Cc: gregkh@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> devel@xxxxxxxxxxxxxxxxxxxxxx; apw@xxxxxxxxxxxxx; jasowang@xxxxxxxxxx
> Subject: Re: [PATCH V2 1/1] Drivers: hv: Implement the file copy service
>
> On Tue, Jan 14, 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:
>
> The change below fixes some warnings in the daemon code.
> Compile tested only.
> I also think the newlines in some of the syslog calls should be removed.
>
> Olaf
>
>
> hv_fcopy_daemon.c: In function 'hv_start_fcopy':
> hv_fcopy_daemon.c:44:3: warning: format '%s' expects argument of type 'char
> *', but argument 3 has type '__u16 *' [-Wformat=]
> smsg->file_name);
> ^
> hv_fcopy_daemon.c:44:3: warning: format '%s' expects argument of type 'char
> *', but argument 5 has type '__u16 *' [-Wformat=]
> hv_fcopy_daemon.c:57:6: warning: format '%s' expects argument of type 'char
> *', but argument 3 has type '__u16 *' [-Wformat=]
> errno, strerror(errno));
> ^
> hv_fcopy_daemon.c:61:4: warning: format '%s' expects argument of type 'char
> *', but argument 3 has type '__u16 *' [-Wformat=]
> syslog(LOG_ERR, "Invalid path: %s\n", smsg->path_name);
> ^
> hv_fcopy_daemon.c: In function 'main':
> hv_fcopy_daemon.c:117:8: warning: ignoring return value of 'daemon', declared
> with attribute warn_unused_result [-Wunused-result]
> daemon(1, 0);
> ^
> hv_fcopy_daemon.c:132:7: warning: ignoring return value of 'write', declared
> with attribute warn_unused_result [-Wunused-result]
> write(fcopy_fd, &version, sizeof(int));
> ^
> hv_fcopy_daemon.c:171:9: warning: ignoring return value of 'pwrite', declared
> with attribute warn_unused_result [-Wunused-result]
> pwrite(fcopy_fd, &error, sizeof(int), 0);
> ^
>
> Signed-off-by: Olaf Hering <olaf@xxxxxxxxx>
>
> diff --git a/tools/hv/hv_fcopy_daemon.c b/tools/hv/hv_fcopy_daemon.c
> index c0e5c90..d1fadb7 100644
> --- a/tools/hv/hv_fcopy_daemon.c
> +++ b/tools/hv/hv_fcopy_daemon.c
> @@ -35,14 +35,14 @@
> #include <dirent.h>
>
> static int target_fd;
> -char target_fname[W_MAX_PATH];
> +static char target_fname[W_MAX_PATH];
>
> static int hv_start_fcopy(struct hv_start_fcopy *smsg)
> {
> int error = HV_E_FAIL;
>
> - sprintf(target_fname, "%s%s%s", smsg->path_name, "/",
> - smsg->file_name);
> + snprintf(target_fname, sizeof(target_fname), "%s/%s",
> + (char *)smsg->path_name, (char*)smsg->file_name);
>
> syslog(LOG_INFO, "Target file name: %s\n", target_fname);
> /*
> @@ -54,12 +54,12 @@ static int hv_start_fcopy(struct hv_start_fcopy *smsg)
> if (mkdir((char *)smsg->path_name, 0755)) {
> syslog(LOG_ERR,
> "Failed to create '%s'; error: %d %s\n",
> - smsg->path_name,
> + (char *)smsg->path_name,
> errno, strerror(errno));
> goto done;
> }
> } else {
> - syslog(LOG_ERR, "Invalid path: %s\n", smsg-
> >path_name);
> + syslog(LOG_ERR, "Invalid path: %s", (char *)smsg-
> >path_name);
> goto done;
> }
> }
> @@ -115,7 +115,8 @@ int main(void)
> char *buffer[4096 * 2];
> struct hv_fcopy_hdr *in_msg;
>
> - daemon(1, 0);
> + if (daemon(1, 0))
> + return 1;
> openlog("HV_FCOPY", 0, LOG_USER);
> syslog(LOG_INFO, "HV_FCOPY starting; pid is:%d", getpid());
>
> @@ -130,7 +131,10 @@ int main(void)
> /*
> * Register with the kernel.
> */
> - write(fcopy_fd, &version, sizeof(int));
> + if (write(fcopy_fd, &version, sizeof(int)) != sizeof(int)) {
> + syslog(LOG_ERR, "write failed: %s",strerror(errno));
> + exit(EXIT_FAILURE);
> + }
>
> while (1) {
> /*
> @@ -169,6 +173,9 @@ int main(void)
>
> }
>
> - pwrite(fcopy_fd, &error, sizeof(int), 0);
> + if (pwrite(fcopy_fd, &error, sizeof(int), 0) != sizeof(int)) {
> + syslog(LOG_ERR, "pwrite failed: %s",strerror(errno));
> + exit(EXIT_FAILURE);
> + }
> }
> }
N‹§²æìr¸›yúèšØb²X¬¶ÇvØ^–)Þ{.nÇ+‰·¥Š{±‘êçzX§¶›¡Ü}©ž²ÆzÚ&j:+v‰¨¾«‘êçzZ+€Ê+zf£¢·hšˆ§~†­†Ûiÿûàz¹®w¥¢¸?™¨è­Ú&¢)ßf”ù^jÇy§m…á@A«a¶Úÿ 0¶ìh®å’i