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

[KY Srinivasan]

> -----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);
> +		}
>  	}
>  }

Thanks Olaf; I will include these changes in the next version.

K. Y
èº{.nÇ+‰·Ÿ®‰­†+%ŠËlzwm…ébëæìr¸›zX§»®w¥Š{ayºÊÚë,j­¢f£¢·hš‹àz¹®w¥¢¸¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾«‘êçzZ+ƒùšŽŠÝj"ú!¶iO•æ¬z·švØ^¶m§ÿðÃnÆàþY&—