[PATCH] Path string from the host should not be treated as wchar_t

[PATCH] Path string from the host should not be treated as wchar_t

[yasuenag]

From: Yasumasa Suenaga <yasuenag@gmail.com>

hv_fcopy_uio_daemon handles file copy request from the host.
(e.g. Copy-VMFile commandlet)
The request has file path and its name, they would be stored as
__u16 arrays in struct hv_start_fcopy. They are casted to wchar_t*
in fcopyd to convert to UTF-8 string. wchar_t is 32bit in Linux
unlike Windows (16bit), so string conversion would be failed and
the user cannot copy file to Linux guest from Host via fcopyd.

fcopyd converts each characters to char if the value is less
than 0x80. Thus we can convert straightly without wcstombs() call,
it means we are no longer to convert to wchar_t.

Length of path depends on PATH_MAX (Linux) and W_MAX_PATH (Windows),
so this change also addes new check to snprintf() call to make
target path.
---
 tools/hv/hv_fcopy_uio_daemon.c | 38 ++++++++++++++--------------------
 1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/hv_fcopy_uio_daemon.c
index 0198321d1..049d4fd9c 100644
--- a/tools/hv/hv_fcopy_uio_daemon.c
+++ b/tools/hv/hv_fcopy_uio_daemon.c
@@ -58,12 +58,16 @@ static unsigned long long filesize;
 static int hv_fcopy_create_file(char *file_name, char *path_name, __u32 flags)
 {
 	int error = HV_E_FAIL;
+	int ret_snprintf;
 	char *q, *p;
 
 	filesize = 0;
 	p = path_name;
-	snprintf(target_fname, sizeof(target_fname), "%s/%s",
-		 path_name, file_name);
+	ret_snprintf = snprintf(target_fname, sizeof(target_fname), "%s/%s",
+	                        path_name, file_name);
+	if (ret_snprintf >= sizeof(target_fname))
+		/* target file name is too long */
+		goto done;
 
 	/*
 	 * Check to see if the path is already in place; if not,
@@ -273,6 +277,8 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
 	while (len < dest_size) {
 		if (src[len] < 0x80)
 			dest[len++] = (char)(*src++);
+		else if (src[len] == '0')
+			break;
 		else
 			dest[len++] = 'X';
 	}
@@ -282,27 +288,15 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
 
 static int hv_fcopy_start(struct hv_start_fcopy *smsg_in)
 {
-	setlocale(LC_ALL, "en_US.utf8");
-	size_t file_size, path_size;
-	char *file_name, *path_name;
-	char *in_file_name = (char *)smsg_in->file_name;
-	char *in_path_name = (char *)smsg_in->path_name;
-
-	file_size = wcstombs(NULL, (const wchar_t *restrict)in_file_name, 0) + 1;
-	path_size = wcstombs(NULL, (const wchar_t *restrict)in_path_name, 0) + 1;
-
-	file_name = (char *)malloc(file_size * sizeof(char));
-	path_name = (char *)malloc(path_size * sizeof(char));
-
-	if (!file_name || !path_name) {
-		free(file_name);
-		free(path_name);
-		syslog(LOG_ERR, "Can't allocate memory for file name and/or path name");
-		return HV_E_FAIL;
-	}
+	/*
+	 * file_name and path_name should have same length with appropriate
+	 * member of hv_start_fcopy.
+	*/
+	char file_name[W_MAX_PATH], path_name[W_MAX_PATH];
 
-	wcstoutf8(file_name, (__u16 *)in_file_name, file_size);
-	wcstoutf8(path_name, (__u16 *)in_path_name, path_size);
+	setlocale(LC_ALL, "en_US.utf8");
+	wcstoutf8(file_name, smsg_in->file_name, W_MAX_PATH - 1);
+	wcstoutf8(path_name, smsg_in->path_name, W_MAX_PATH - 1);
 
 	return hv_fcopy_create_file(file_name, path_name, smsg_in->copy_flags);
 }
-- 
2.49.0

Re: [PATCH] Path string from the host should not be treated as wchar_t

[Easwar Hariharan]

On 6/1/2025 6:45 AM, yasuenag@gmail.com wrote:
> From: Yasumasa Suenaga <yasuenag@gmail.com>
> 
> hv_fcopy_uio_daemon handles file copy request from the host.
> (e.g. Copy-VMFile commandlet)
> The request has file path and its name, they would be stored as
> __u16 arrays in struct hv_start_fcopy. They are casted to wchar_t*
> in fcopyd to convert to UTF-8 string. wchar_t is 32bit in Linux
> unlike Windows (16bit), so string conversion would be failed and
> the user cannot copy file to Linux guest from Host via fcopyd.
> 
> fcopyd converts each characters to char if the value is less
> than 0x80. Thus we can convert straightly without wcstombs() call,
> it means we are no longer to convert to wchar_t.
> 
> Length of path depends on PATH_MAX (Linux) and W_MAX_PATH (Windows),
> so this change also addes new check to snprintf() call to make
> target path.

Missing Signed-off-by for developer certificate of origin

> ---
>  tools/hv/hv_fcopy_uio_daemon.c | 38 ++++++++++++++--------------------
>  1 file changed, 16 insertions(+), 22 deletions(-)
> 

Thanks for the patch! Please look through https://www.kernel.org/doc/html/latest/process/submitting-patches.html
to see hints for the expected subject line, explanation body, and developer's certificate of origin

+CC: Saurabh for review, weird why get_maintainer.pl didn't get him 


> diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/hv_fcopy_uio_daemon.c
> index 0198321d1..049d4fd9c 100644
> --- a/tools/hv/hv_fcopy_uio_daemon.c
> +++ b/tools/hv/hv_fcopy_uio_daemon.c
> @@ -58,12 +58,16 @@ static unsigned long long filesize;
>  static int hv_fcopy_create_file(char *file_name, char *path_name, __u32 flags)
>  {
>  	int error = HV_E_FAIL;
> +	int ret_snprintf;
>  	char *q, *p;
>  
>  	filesize = 0;
>  	p = path_name;
> -	snprintf(target_fname, sizeof(target_fname), "%s/%s",
> -		 path_name, file_name);
> +	ret_snprintf = snprintf(target_fname, sizeof(target_fname), "%s/%s",
> +	                        path_name, file_name);
> +	if (ret_snprintf >= sizeof(target_fname))
> +		/* target file name is too long */
> +		goto done;

The error check can be inlined since we don't use ret_snprintf elsewhere, i.e.

if(snprintf(target_fname, sizeof(target_fname), "%s/%s", path_name, file_name) >= sizeof(target_fname)) {
	/* target file name is too long */
	goto done;
}

Note also the added braces, even if the extra line is a comment. https://www.kernel.org/doc/html/latest/process/coding-style.html

>  
>  	/*
>  	 * Check to see if the path is already in place; if not,
> @@ -273,6 +277,8 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
>  	while (len < dest_size) {
>  		if (src[len] < 0x80)
>  			dest[len++] = (char)(*src++);
> +		else if (src[len] == '0')
> +			break;
>  		else
>  			dest[len++] = 'X';
>  	}
> @@ -282,27 +288,15 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
>  
>  static int hv_fcopy_start(struct hv_start_fcopy *smsg_in)
>  {
> -	setlocale(LC_ALL, "en_US.utf8");
> -	size_t file_size, path_size;
> -	char *file_name, *path_name;
> -	char *in_file_name = (char *)smsg_in->file_name;
> -	char *in_path_name = (char *)smsg_in->path_name;
> -
> -	file_size = wcstombs(NULL, (const wchar_t *restrict)in_file_name, 0) + 1;
> -	path_size = wcstombs(NULL, (const wchar_t *restrict)in_path_name, 0) + 1;
> -
> -	file_name = (char *)malloc(file_size * sizeof(char));
> -	path_name = (char *)malloc(path_size * sizeof(char));
> -
> -	if (!file_name || !path_name) {
> -		free(file_name);
> -		free(path_name);
> -		syslog(LOG_ERR, "Can't allocate memory for file name and/or path name");
> -		return HV_E_FAIL;
> -	}
> +	/*
> +	 * file_name and path_name should have same length with appropriate
> +	 * member of hv_start_fcopy.
> +	*/
> +	char file_name[W_MAX_PATH], path_name[W_MAX_PATH];
>  
> -	wcstoutf8(file_name, (__u16 *)in_file_name, file_size);
> -	wcstoutf8(path_name, (__u16 *)in_path_name, path_size);
> +	setlocale(LC_ALL, "en_US.utf8");
> +	wcstoutf8(file_name, smsg_in->file_name, W_MAX_PATH - 1);
> +	wcstoutf8(path_name, smsg_in->path_name, W_MAX_PATH - 1);
>  
>  	return hv_fcopy_create_file(file_name, path_name, smsg_in->copy_flags);
>  }

[PATCH v2 0/1] Path string from the host should not be treated

[yasuenag]

From: Yasumasa Suenaga <yasuenag@gmail.com>

Hi,

Easwar, thanks a lot for your comment! I fixed where you pointed.
Let me know if something wrong in this patch - this is my first
contribution to Linux kernel...

Yasumasa Suenaga (1):
  Path string from the host should not be treated as wchar_t

 tools/hv/hv_fcopy_uio_daemon.c | 36 +++++++++++++---------------------
 1 file changed, 14 insertions(+), 22 deletions(-)

-- 
2.49.0

[PATCH v2 0/1] Path string from the host should not be treated

[yasuenag]

From: Yasumasa Suenaga <yasuenag@gmail.com>

Hi,

Easwar, thanks a lot for your comment! I fixed where you pointed.
Let me know if something wrong in this patch - this is my first
contribution to Linux kernel...

Yasumasa Suenaga (1):
  Path string from the host should not be treated as wchar_t

 tools/hv/hv_fcopy_uio_daemon.c | 36 +++++++++++++---------------------
 1 file changed, 14 insertions(+), 22 deletions(-)

-- 
2.49.0

[PATCH v2 1/1] Path string from the host should not be treated as wchar_t

[yasuenag]

From: Yasumasa Suenaga <yasuenag@gmail.com>

hv_fcopy_uio_daemon handles file copy request from the host.
(e.g. Copy-VMFile commandlet)
The request has file path and its name, they would be stored as
__u16 arrays in struct hv_start_fcopy. They are casted to wchar_t*
in fcopyd to convert to UTF-8 string. wchar_t is 32bit in Linux
unlike Windows (16bit), so string conversion would be failed and
the user cannot copy file to Linux guest from Host via fcopyd.

fcopyd converts each characters to char if the value is less
than 0x80. Thus we can convert straightly without wcstombs() call,
it means we are no longer to convert to wchar_t.

Length of path depends on PATH_MAX (Linux) and W_MAX_PATH (Windows),
so this change also addes new check to snprintf() call to make
target path.

Signed-off-by: Yasumasa Suenaga <yasuenag@gmail.com>
---
 tools/hv/hv_fcopy_uio_daemon.c | 36 +++++++++++++---------------------
 1 file changed, 14 insertions(+), 22 deletions(-)

diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/hv_fcopy_uio_daemon.c
index 0198321d1..85ec21696 100644
--- a/tools/hv/hv_fcopy_uio_daemon.c
+++ b/tools/hv/hv_fcopy_uio_daemon.c
@@ -62,8 +62,10 @@ static int hv_fcopy_create_file(char *file_name, char *path_name, __u32 flags)
 
 	filesize = 0;
 	p = path_name;
-	snprintf(target_fname, sizeof(target_fname), "%s/%s",
-		 path_name, file_name);
+	if (snprintf(target_fname, sizeof(target_fname), "%s/%s", path_name, file_name) >= sizeof(target_fname)) {
+		/* target file name is too long */
+		goto done;
+	}
 
 	/*
 	 * Check to see if the path is already in place; if not,
@@ -273,6 +275,8 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
 	while (len < dest_size) {
 		if (src[len] < 0x80)
 			dest[len++] = (char)(*src++);
+		else if (src[len] == '0')
+			break;
 		else
 			dest[len++] = 'X';
 	}
@@ -282,27 +286,15 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
 
 static int hv_fcopy_start(struct hv_start_fcopy *smsg_in)
 {
-	setlocale(LC_ALL, "en_US.utf8");
-	size_t file_size, path_size;
-	char *file_name, *path_name;
-	char *in_file_name = (char *)smsg_in->file_name;
-	char *in_path_name = (char *)smsg_in->path_name;
-
-	file_size = wcstombs(NULL, (const wchar_t *restrict)in_file_name, 0) + 1;
-	path_size = wcstombs(NULL, (const wchar_t *restrict)in_path_name, 0) + 1;
-
-	file_name = (char *)malloc(file_size * sizeof(char));
-	path_name = (char *)malloc(path_size * sizeof(char));
-
-	if (!file_name || !path_name) {
-		free(file_name);
-		free(path_name);
-		syslog(LOG_ERR, "Can't allocate memory for file name and/or path name");
-		return HV_E_FAIL;
-	}
+	/*
+	 * file_name and path_name should have same length with appropriate
+	 * member of hv_start_fcopy.
+	*/
+	char file_name[W_MAX_PATH], path_name[W_MAX_PATH];
 
-	wcstoutf8(file_name, (__u16 *)in_file_name, file_size);
-	wcstoutf8(path_name, (__u16 *)in_path_name, path_size);
+	setlocale(LC_ALL, "en_US.utf8");
+	wcstoutf8(file_name, smsg_in->file_name, W_MAX_PATH - 1);
+	wcstoutf8(path_name, smsg_in->path_name, W_MAX_PATH - 1);
 
 	return hv_fcopy_create_file(file_name, path_name, smsg_in->copy_flags);
 }
-- 
2.49.0

Re: [PATCH v2 0/1] Path string from the host should not be treated

[Easwar Hariharan]

On 6/2/2025 4:56 PM, yasuenag@gmail.com wrote:
> From: Yasumasa Suenaga <yasuenag@gmail.com>
> 
> Hi,
> 
> Easwar, thanks a lot for your comment! I fixed where you pointed.
> Let me know if something wrong in this patch - this is my first
> contribution to Linux kernel...
> 
> Yasumasa Suenaga (1):
>   Path string from the host should not be treated as wchar_t
> 
>  tools/hv/hv_fcopy_uio_daemon.c | 36 +++++++++++++---------------------
>  1 file changed, 14 insertions(+), 22 deletions(-)
> 

Please run scripts/checkpatch.pl --strict on the patch and fix the reported
issues.

As I mentioned previously, https://www.kernel.org/doc/html/latest/process/submitting-patches.html
is a good reference for the expected format of the patch. Specifically:

"Describe your changes in imperative mood, e.g. "make xyzzy do frotz" instead 
of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy to do frotz", 
as if you are giving orders to the codebase to change its behaviour."

The same also applies to the subject line.

Thanks,
Easwar (he/him)

P.S: It's good etiquette to explicitly CC folks who have provided comments
on the subsequent versions

[PATCH v3 0/1] hv_fcopy_uio_daemon: Fix file copy failure between Windows host and Linux guest

[yasuenag]

From: Yasumasa Suenaga <yasuenag@gmail.com>

Thanks a lot for your advice!
I fixed cosmetic problems and checked it with checkpatch.pl .
And also I fixed subject and description in the commit
as the document pointed.

I hope this version would go well...


Best regards,

Yasumasa


Yasumasa Suenaga (1):
  hv_fcopy_uio_daemon: Fix file copy failure between Windows host and
    Linux guest

 tools/hv/hv_fcopy_uio_daemon.c | 37 ++++++++++++++--------------------
 1 file changed, 15 insertions(+), 22 deletions(-)

-- 
2.49.0

[PATCH v3 1/1] hv_fcopy_uio_daemon: Fix file copy failure between Windows host and Linux guest

[yasuenag]

From: Yasumasa Suenaga <yasuenag@gmail.com>

Handle file copy request from the host (e.g. Copy-VMFile commandlet)
correctly.
Store file path and name as __u16 arrays in struct hv_start_fcopy.
Convert directly to UTF-8 string without casting to wchar_t* in fcopyd.

Fix string conversion failure caused by wchar_t size difference between
Linux (32bit) and Windows (16bit). Convert each character to char
if the value is less than 0x80 instead of using wcstombs() call.

Add new check to snprintf() call for target path creation to handle
length differences between PATH_MAX (Linux) and W_MAX_PATH (Windows).

Signed-off-by: Yasumasa Suenaga <yasuenag@gmail.com>
---
 tools/hv/hv_fcopy_uio_daemon.c | 37 ++++++++++++++--------------------
 1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/hv_fcopy_uio_daemon.c
index 0198321d1..86702f39e 100644
--- a/tools/hv/hv_fcopy_uio_daemon.c
+++ b/tools/hv/hv_fcopy_uio_daemon.c
@@ -62,8 +62,11 @@ static int hv_fcopy_create_file(char *file_name, char *path_name, __u32 flags)
 
 	filesize = 0;
 	p = path_name;
-	snprintf(target_fname, sizeof(target_fname), "%s/%s",
-		 path_name, file_name);
+	if (snprintf(target_fname, sizeof(target_fname), "%s/%s",
+		     path_name, file_name) >= sizeof(target_fname)) {
+		/* target file name is too long */
+		goto done;
+	}
 
 	/*
 	 * Check to see if the path is already in place; if not,
@@ -273,6 +276,8 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
 	while (len < dest_size) {
 		if (src[len] < 0x80)
 			dest[len++] = (char)(*src++);
+		else if (src[len] == '0')
+			break;
 		else
 			dest[len++] = 'X';
 	}
@@ -282,27 +287,15 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
 
 static int hv_fcopy_start(struct hv_start_fcopy *smsg_in)
 {
-	setlocale(LC_ALL, "en_US.utf8");
-	size_t file_size, path_size;
-	char *file_name, *path_name;
-	char *in_file_name = (char *)smsg_in->file_name;
-	char *in_path_name = (char *)smsg_in->path_name;
-
-	file_size = wcstombs(NULL, (const wchar_t *restrict)in_file_name, 0) + 1;
-	path_size = wcstombs(NULL, (const wchar_t *restrict)in_path_name, 0) + 1;
-
-	file_name = (char *)malloc(file_size * sizeof(char));
-	path_name = (char *)malloc(path_size * sizeof(char));
-
-	if (!file_name || !path_name) {
-		free(file_name);
-		free(path_name);
-		syslog(LOG_ERR, "Can't allocate memory for file name and/or path name");
-		return HV_E_FAIL;
-	}
+	/*
+	 * file_name and path_name should have same length with appropriate
+	 * member of hv_start_fcopy.
+	 */
+	char file_name[W_MAX_PATH], path_name[W_MAX_PATH];
 
-	wcstoutf8(file_name, (__u16 *)in_file_name, file_size);
-	wcstoutf8(path_name, (__u16 *)in_path_name, path_size);
+	setlocale(LC_ALL, "en_US.utf8");
+	wcstoutf8(file_name, smsg_in->file_name, W_MAX_PATH - 1);
+	wcstoutf8(path_name, smsg_in->path_name, W_MAX_PATH - 1);
 
 	return hv_fcopy_create_file(file_name, path_name, smsg_in->copy_flags);
 }
-- 
2.49.0

Re: [PATCH v3 1/1] hv_fcopy_uio_daemon: Fix file copy failure between Windows host and Linux guest

[Naman Jain]

On 6/4/2025 5:13 AM, yasuenag@gmail.com wrote:
> From: Yasumasa Suenaga <yasuenag@gmail.com>
> 
> Handle file copy request from the host (e.g. Copy-VMFile commandlet)
> correctly.
> Store file path and name as __u16 arrays in struct hv_start_fcopy.
> Convert directly to UTF-8 string without casting to wchar_t* in fcopyd.
> 
> Fix string conversion failure caused by wchar_t size difference between
> Linux (32bit) and Windows (16bit). Convert each character to char
> if the value is less than 0x80 instead of using wcstombs() call.
> 
> Add new check to snprintf() call for target path creation to handle
> length differences between PATH_MAX (Linux) and W_MAX_PATH (Windows).
> 
> Signed-off-by: Yasumasa Suenaga <yasuenag@gmail.com>
> ---
>   tools/hv/hv_fcopy_uio_daemon.c | 37 ++++++++++++++--------------------
>   1 file changed, 15 insertions(+), 22 deletions(-)
> 
> diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/hv_fcopy_uio_daemon.c
> index 0198321d1..86702f39e 100644
> --- a/tools/hv/hv_fcopy_uio_daemon.c
> +++ b/tools/hv/hv_fcopy_uio_daemon.c
> @@ -62,8 +62,11 @@ static int hv_fcopy_create_file(char *file_name, char *path_name, __u32 flags)
>   
>   	filesize = 0;
>   	p = path_name;
> -	snprintf(target_fname, sizeof(target_fname), "%s/%s",
> -		 path_name, file_name);
> +	if (snprintf(target_fname, sizeof(target_fname), "%s/%s",
> +		     path_name, file_name) >= sizeof(target_fname)) {
> +		/* target file name is too long */
> +		goto done;
> +	}
>   
>   	/*
>   	 * Check to see if the path is already in place; if not,
> @@ -273,6 +276,8 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
>   	while (len < dest_size) {
>   		if (src[len] < 0x80)
>   			dest[len++] = (char)(*src++);
> +		else if (src[len] == '0')
> +			break;
>   		else
>   			dest[len++] = 'X';
>   	}
> @@ -282,27 +287,15 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
>   
>   static int hv_fcopy_start(struct hv_start_fcopy *smsg_in)
>   {
> -	setlocale(LC_ALL, "en_US.utf8");
> -	size_t file_size, path_size;
> -	char *file_name, *path_name;
> -	char *in_file_name = (char *)smsg_in->file_name;
> -	char *in_path_name = (char *)smsg_in->path_name;
> -
> -	file_size = wcstombs(NULL, (const wchar_t *restrict)in_file_name, 0) + 1;
> -	path_size = wcstombs(NULL, (const wchar_t *restrict)in_path_name, 0) + 1;
> -
> -	file_name = (char *)malloc(file_size * sizeof(char));
> -	path_name = (char *)malloc(path_size * sizeof(char));
> -
> -	if (!file_name || !path_name) {
> -		free(file_name);
> -		free(path_name);
> -		syslog(LOG_ERR, "Can't allocate memory for file name and/or path name");
> -		return HV_E_FAIL;
> -	}
> +	/*
> +	 * file_name and path_name should have same length with appropriate
> +	 * member of hv_start_fcopy.
> +	 */
> +	char file_name[W_MAX_PATH], path_name[W_MAX_PATH];
>   
> -	wcstoutf8(file_name, (__u16 *)in_file_name, file_size);
> -	wcstoutf8(path_name, (__u16 *)in_path_name, path_size);
> +	setlocale(LC_ALL, "en_US.utf8");
> +	wcstoutf8(file_name, smsg_in->file_name, W_MAX_PATH - 1);
> +	wcstoutf8(path_name, smsg_in->path_name, W_MAX_PATH - 1);
>   
>   	return hv_fcopy_create_file(file_name, path_name, smsg_in->copy_flags);
>   }

Hi,
I understand this is your first patch for upstreaming. Here are a few
things you should consider:
1. Create a new patch file for every new version and then send it.
Currently it seems you are manually editing the same patch file in the
subject and sending it, so each patch version is showing up in the same
thread.
2. Read, re-read, absorb the information in the link that Easwar also
mentioned:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html
3. Keep a minimum of 1-2 weeks gap between successive patch versions to
give time to people to review your changes.
4. In the commit msg of v3, it is still not very clear what problem, you
are trying to fix here. Do you mean to say that fcopy does not work on
Linux? Or you are assuming it won't work and fixing some generic
problem?
Fcopy is supposed to work fine on Linux VM on HyperV with windows host. 
If there are some errors, please share in cover letter/comments
in the patch along with steps of execution.

5. If its a fix, we should have a proper Fixes tag with the commit you
are fixing.
6. Have a look at existing conversations at lore to get to know common
practices with single patch, multi patch, cover letters etc.
https://lore.kernel.org/linux-hyperv/?q=linux-hyperv

Regards,
Naman

Re: [PATCH v3 1/1] hv_fcopy_uio_daemon: Fix file copy failure between Windows host and Linux guest

[Yasumasa Suenaga]

Hi Naman,
Sorry for bothering you, but I have some questions:


> 1. Create a new patch file for every new version and then send it.
> Currently it seems you are manually editing the same patch file in the
> subject and sending it, so each patch version is showing up in the same
> thread.

I've committed changes with "git amend" and created formatted patch with "git format-patch",
then I sent all of *.patch files via "git send-email".
Do you mean I should commit to fix commits reviewers and send cover letter and incremental
patches only? I couldn't find about it from the guide.
I checked linux-hyperv list, all of patches have been sent in each version AFAICS.


> 3. Keep a minimum of 1-2 weeks gap between successive patch versions to
> give time to people to review your changes.

Does it include changes of commit message too?


> 4. In the commit msg of v3, it is still not very clear what problem, you
> are trying to fix here. Do you mean to say that fcopy does not work on
> Linux? Or you are assuming it won't work and fixing some generic
> problem?
> Fcopy is supposed to work fine on Linux VM on HyperV with windows host. If there are some errors, please share in cover letter/comments
> in the patch along with steps of execution.

I have a problem on my PC:

   Host: Windows 11 Pro (24H2 build 26100.4202)
   Guest: Fedora 42
     - kernel-6.14.4-300.fc42.x86_64
     - hypervfcopyd-6.10-1.fc42.x86_64

How to reproduce: run Copy-VMFile commandlet on Host:

Following log is in Japanese because my PC is set to Japanese, sorry.
But it says fcopy could not transfer file (test.ps1) to /tmp/ on Linux guest because it already exists.
I confirmed /tmp/test.ps1 does not exist of course.

```
> Copy-VMFile

cmdlet Copy-VMFile at command pipeline position 1
Supply values for the following parameters:
Name[0]: Fedora42
Name[1]:
SourcePath: test.ps1
DestinationPath: /tmp/
FileSource: Host
Copy-VMFile: ゲストへのファイルのコピーを開始できませんでした。

ソース ファイル 'C:\test\test.ps1' をゲストの宛先 '/tmp/' にコピーできませんでした。

'Fedora42' はゲスト: ファイルがあります。 (0x80070050) へのファイルのコピーを開始できませんでした。(仮想マシン ID 9BFDF23D-CCAA-4748-A770-6D654E09A133)

'Fedora42' は、コピー元ファイル 'C:\test\test.ps1' をゲスト上のコピー先 '/tmp/' にコピーできませんでした: ファイルがあります。 (0x80070050)。(仮想マシン ID 9BFDF23D-CCAA-4748-A770-6D654E09A133)
```

I got following fcopy log from journald - it is strange because "/tmp/test.ps1" should be shown here.
```
6月 04 17:48:24 fc42 HV_UIO_FCOPY[1080]: File: / exists
```

As I wrote in commit message, wchar_t is 32 bit in Linux. I confirmed it with "sizeof(wchar_t)".
However fcopyd handles it as 16 bit value (__u16), thus I think this is a bug in fcopy, and
I think it would also not work on other environments.

Actually it works fine with my patch to handle values as 16 bit.


Thanks,

Yasumasa


On 2025/06/04 15:19, Naman Jain wrote:
> 
> 
> On 6/4/2025 5:13 AM, yasuenag@gmail.com wrote:
>> From: Yasumasa Suenaga <yasuenag@gmail.com>
>>
>> Handle file copy request from the host (e.g. Copy-VMFile commandlet)
>> correctly.
>> Store file path and name as __u16 arrays in struct hv_start_fcopy.
>> Convert directly to UTF-8 string without casting to wchar_t* in fcopyd.
>>
>> Fix string conversion failure caused by wchar_t size difference between
>> Linux (32bit) and Windows (16bit). Convert each character to char
>> if the value is less than 0x80 instead of using wcstombs() call.
>>
>> Add new check to snprintf() call for target path creation to handle
>> length differences between PATH_MAX (Linux) and W_MAX_PATH (Windows).
>>
>> Signed-off-by: Yasumasa Suenaga <yasuenag@gmail.com>
>> ---
>>   tools/hv/hv_fcopy_uio_daemon.c | 37 ++++++++++++++--------------------
>>   1 file changed, 15 insertions(+), 22 deletions(-)
>>
>> diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/hv_fcopy_uio_daemon.c
>> index 0198321d1..86702f39e 100644
>> --- a/tools/hv/hv_fcopy_uio_daemon.c
>> +++ b/tools/hv/hv_fcopy_uio_daemon.c
>> @@ -62,8 +62,11 @@ static int hv_fcopy_create_file(char *file_name, char *path_name, __u32 flags)
>>       filesize = 0;
>>       p = path_name;
>> -    snprintf(target_fname, sizeof(target_fname), "%s/%s",
>> -         path_name, file_name);
>> +    if (snprintf(target_fname, sizeof(target_fname), "%s/%s",
>> +             path_name, file_name) >= sizeof(target_fname)) {
>> +        /* target file name is too long */
>> +        goto done;
>> +    }
>>       /*
>>        * Check to see if the path is already in place; if not,
>> @@ -273,6 +276,8 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
>>       while (len < dest_size) {
>>           if (src[len] < 0x80)
>>               dest[len++] = (char)(*src++);
>> +        else if (src[len] == '0')
>> +            break;
>>           else
>>               dest[len++] = 'X';
>>       }
>> @@ -282,27 +287,15 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
>>   static int hv_fcopy_start(struct hv_start_fcopy *smsg_in)
>>   {
>> -    setlocale(LC_ALL, "en_US.utf8");
>> -    size_t file_size, path_size;
>> -    char *file_name, *path_name;
>> -    char *in_file_name = (char *)smsg_in->file_name;
>> -    char *in_path_name = (char *)smsg_in->path_name;
>> -
>> -    file_size = wcstombs(NULL, (const wchar_t *restrict)in_file_name, 0) + 1;
>> -    path_size = wcstombs(NULL, (const wchar_t *restrict)in_path_name, 0) + 1;
>> -
>> -    file_name = (char *)malloc(file_size * sizeof(char));
>> -    path_name = (char *)malloc(path_size * sizeof(char));
>> -
>> -    if (!file_name || !path_name) {
>> -        free(file_name);
>> -        free(path_name);
>> -        syslog(LOG_ERR, "Can't allocate memory for file name and/or path name");
>> -        return HV_E_FAIL;
>> -    }
>> +    /*
>> +     * file_name and path_name should have same length with appropriate
>> +     * member of hv_start_fcopy.
>> +     */
>> +    char file_name[W_MAX_PATH], path_name[W_MAX_PATH];
>> -    wcstoutf8(file_name, (__u16 *)in_file_name, file_size);
>> -    wcstoutf8(path_name, (__u16 *)in_path_name, path_size);
>> +    setlocale(LC_ALL, "en_US.utf8");
>> +    wcstoutf8(file_name, smsg_in->file_name, W_MAX_PATH - 1);
>> +    wcstoutf8(path_name, smsg_in->path_name, W_MAX_PATH - 1);
>>       return hv_fcopy_create_file(file_name, path_name, smsg_in->copy_flags);
>>   }
> 
> Hi,
> I understand this is your first patch for upstreaming. Here are a few
> things you should consider:
> 1. Create a new patch file for every new version and then send it.
> Currently it seems you are manually editing the same patch file in the
> subject and sending it, so each patch version is showing up in the same
> thread.
> 2. Read, re-read, absorb the information in the link that Easwar also
> mentioned:
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
> 3. Keep a minimum of 1-2 weeks gap between successive patch versions to
> give time to people to review your changes.
> 4. In the commit msg of v3, it is still not very clear what problem, you
> are trying to fix here. Do you mean to say that fcopy does not work on
> Linux? Or you are assuming it won't work and fixing some generic
> problem?
> Fcopy is supposed to work fine on Linux VM on HyperV with windows host. If there are some errors, please share in cover letter/comments
> in the patch along with steps of execution.
> 
> 5. If its a fix, we should have a proper Fixes tag with the commit you
> are fixing.
> 6. Have a look at existing conversations at lore to get to know common
> practices with single patch, multi patch, cover letters etc.
> https://lore.kernel.org/linux-hyperv/?q=linux-hyperv
> 
> Regards,
> Naman
> 
> 

Re: [PATCH v3 1/1] hv_fcopy_uio_daemon: Fix file copy failure between Windows host and Linux guest

[Naman Jain]

On 6/4/2025 2:40 PM, Yasumasa Suenaga wrote:
> Hi Naman,
> Sorry for bothering you, but I have some questions:
> 
> 

Hi,
Happy to help. I have tested your patch on my setup, and it did not 
break anything for me. Please find response and additional questions inline.


>> 1. Create a new patch file for every new version and then send it.
>> Currently it seems you are manually editing the same patch file in the
>> subject and sending it, so each patch version is showing up in the same
>> thread.
> 
> I've committed changes with "git amend" and created formatted patch with 
> "git format-patch",
> then I sent all of *.patch files via "git send-email".
> Do you mean I should commit to fix commits reviewers and send cover 
> letter and incremental
> patches only? I couldn't find about it from the guide.
> I checked linux-hyperv list, all of patches have been sent in each 
> version AFAICS.
> 

* make changes
* git add, git commit, git format-patch, git send-email <path to v1 patch)
* make changes for v2
* git add, git commit --amend, git format-patch -v2, git send-email 
<path to v2 patch *only*>
and so on.

I follow above practice, and the patches go in new threads. I am not 
sure why they are landing in same thread for you. Anyhow, its not a big 
thing.

> 
>> 3. Keep a minimum of 1-2 weeks gap between successive patch versions to
>> give time to people to review your changes.
> 
> Does it include changes of commit message too?

Other reviewers may be reviewing older versions and it becomes confusing 
if new versions come too soon. To give everyone enough time to review 
your changes and to not have unnecessary patch versions, its better to 
wait for a few days, collect the feedback and address it together. We 
also need to mention what we changed since last time, to help reviewers 
know if their comments got addressed.

For single patch, we usually don't add a cover letter. You can refer
https://lore.kernel.org/all/20250102145243.2088-1-namjain@linux.microsoft.com/
The comments between the two "---" does not make it to git log.

> 
> 
>> 4. In the commit msg of v3, it is still not very clear what problem, you
>> are trying to fix here. Do you mean to say that fcopy does not work on
>> Linux? Or you are assuming it won't work and fixing some generic
>> problem?
>> Fcopy is supposed to work fine on Linux VM on HyperV with windows 
>> host. If there are some errors, please share in cover letter/comments
>> in the patch along with steps of execution.
> 
> I have a problem on my PC:
> 
>    Host: Windows 11 Pro (24H2 build 26100.4202)
>    Guest: Fedora 42
>      - kernel-6.14.4-300.fc42.x86_64
>      - hypervfcopyd-6.10-1.fc42.x86_64
> 
> How to reproduce: run Copy-VMFile commandlet on Host:
> 
> Following log is in Japanese because my PC is set to Japanese, sorry.
> But it says fcopy could not transfer file (test.ps1) to /tmp/ on Linux 
> guest because it already exists.
> I confirmed /tmp/test.ps1 does not exist of course.
> 
> ```
>> Copy-VMFile
> 
> cmdlet Copy-VMFile at command pipeline position 1
> Supply values for the following parameters:
> Name[0]: Fedora42
> Name[1]:
> SourcePath: test.ps1
> DestinationPath: /tmp/
> FileSource: Host
> Copy-VMFile: ゲストへのファイルのコピーを開始できませんでした。
> 
> ソース ファイル 'C:\test\test.ps1' をゲストの宛先 '/tmp/' にコピーできま 
> せんでした。
> 
> 'Fedora42' はゲスト: ファイルがあります。 (0x80070050) へのファイルのコ 
> ピーを開始できませんでした。(仮想マシン ID 9BFDF23D-CCAA-4748- 
> A770-6D654E09A133)
> 
> 'Fedora42' は、コピー元ファイル 'C:\test\test.ps1' をゲスト上のコピー先 
> '/tmp/' にコピーできませんでした: ファイルがあります。 (0x80070050)。(仮 
> 想マシン ID 9BFDF23D-CCAA-4748-A770-6D654E09A133)
> ```
> 
> I got following fcopy log from journald - it is strange because "/tmp/ 
> test.ps1" should be shown here.
> ```
> 6月 04 17:48:24 fc42 HV_UIO_FCOPY[1080]: File: / exists
> ```
> 
> As I wrote in commit message, wchar_t is 32 bit in Linux. I confirmed it 
> with "sizeof(wchar_t)".
> However fcopyd handles it as 16 bit value (__u16), thus I think this is 
> a bug in fcopy, and
> I think it would also not work on other environments.
> 
> Actually it works fine with my patch to handle values as 16 bit.
> 
> 
> Thanks,
> 
> Yasumasa
> 
> 
> On 2025/06/04 15:19, Naman Jain wrote:
>>
>>
>> On 6/4/2025 5:13 AM, yasuenag@gmail.com wrote:
>>> From: Yasumasa Suenaga <yasuenag@gmail.com>
>>>
>>> Handle file copy request from the host (e.g. Copy-VMFile commandlet)
>>> correctly.
>>> Store file path and name as __u16 arrays in struct hv_start_fcopy.
>>> Convert directly to UTF-8 string without casting to wchar_t* in fcopyd.
>>>
>>> Fix string conversion failure caused by wchar_t size difference between
>>> Linux (32bit) and Windows (16bit). Convert each character to char
>>> if the value is less than 0x80 instead of using wcstombs() call.
>>>
>>> Add new check to snprintf() call for target path creation to handle
>>> length differences between PATH_MAX (Linux) and W_MAX_PATH (Windows).
>>>
>>> Signed-off-by: Yasumasa Suenaga <yasuenag@gmail.com>
>>> ---
>>>   tools/hv/hv_fcopy_uio_daemon.c | 37 ++++++++++++++--------------------
>>>   1 file changed, 15 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/ 
>>> hv_fcopy_uio_daemon.c
>>> index 0198321d1..86702f39e 100644
>>> --- a/tools/hv/hv_fcopy_uio_daemon.c
>>> +++ b/tools/hv/hv_fcopy_uio_daemon.c
>>> @@ -62,8 +62,11 @@ static int hv_fcopy_create_file(char *file_name, 
>>> char *path_name, __u32 flags)
>>>       filesize = 0;
>>>       p = path_name;
>>> -    snprintf(target_fname, sizeof(target_fname), "%s/%s",
>>> -         path_name, file_name);
>>> +    if (snprintf(target_fname, sizeof(target_fname), "%s/%s",
>>> +             path_name, file_name) >= sizeof(target_fname)) {
>>> +        /* target file name is too long */

Please add a syslog for this. It will help in debugging issues.

>>> +        goto done;
>>> +    }
>>>       /*
>>>        * Check to see if the path is already in place; if not,
>>> @@ -273,6 +276,8 @@ static void wcstoutf8(char *dest, const __u16 
>>> *src, size_t dest_size)
>>>       while (len < dest_size) {
>>>           if (src[len] < 0x80)
>>>               dest[len++] = (char)(*src++);
>>> +        else if (src[len] == '0')
>>> +            break;

While this also works, I think you can add it to the while loop itself
while (len < dest_size && src[len])

Is this related to the FCopy issue that you see or this is just a fix 
for correct destination file name?


>>>           else
>>>               dest[len++] = 'X';
>>>       }
>>> @@ -282,27 +287,15 @@ static void wcstoutf8(char *dest, const __u16 
>>> *src, size_t dest_size)
>>>   static int hv_fcopy_start(struct hv_start_fcopy *smsg_in)
>>>   {
>>> -    setlocale(LC_ALL, "en_US.utf8");
>>> -    size_t file_size, path_size;
>>> -    char *file_name, *path_name;
>>> -    char *in_file_name = (char *)smsg_in->file_name;
>>> -    char *in_path_name = (char *)smsg_in->path_name;
>>> -
>>> -    file_size = wcstombs(NULL, (const wchar_t 
>>> *restrict)in_file_name, 0) + 1;
>>> -    path_size = wcstombs(NULL, (const wchar_t 
>>> *restrict)in_path_name, 0) + 1;
>>> -

Please help me understand if this is correct. The actual problem you are 
highlighting lies here when sizeof(wchat_t) is more than 16, i.e. 32?

>>> -    file_name = (char *)malloc(file_size * sizeof(char));
>>> -    path_name = (char *)malloc(path_size * sizeof(char));
>>> -
>>> -    if (!file_name || !path_name) {
>>> -        free(file_name);
>>> -        free(path_name);
>>> -        syslog(LOG_ERR, "Can't allocate memory for file name and/or 
>>> path name");
>>> -        return HV_E_FAIL;
>>> -    }
>>> +    /*
>>> +     * file_name and path_name should have same length with appropriate
>>> +     * member of hv_start_fcopy.
>>> +     */
>>> +    char file_name[W_MAX_PATH], path_name[W_MAX_PATH];
>>> -    wcstoutf8(file_name, (__u16 *)in_file_name, file_size);
>>> -    wcstoutf8(path_name, (__u16 *)in_path_name, path_size);
>>> +    setlocale(LC_ALL, "en_US.utf8");
>>> +    wcstoutf8(file_name, smsg_in->file_name, W_MAX_PATH - 1);
>>> +    wcstoutf8(path_name, smsg_in->path_name, W_MAX_PATH - 1);
>>>       return hv_fcopy_create_file(file_name, path_name, smsg_in- 
>>> >copy_flags);
>>>   }
>>
>> Hi,
>> I understand this is your first patch for upstreaming. Here are a few
>> things you should consider:
>> 1. Create a new patch file for every new version and then send it.
>> Currently it seems you are manually editing the same patch file in the
>> subject and sending it, so each patch version is showing up in the same
>> thread.
>> 2. Read, re-read, absorb the information in the link that Easwar also
>> mentioned:
>> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>> 3. Keep a minimum of 1-2 weeks gap between successive patch versions to
>> give time to people to review your changes.
>> 4. In the commit msg of v3, it is still not very clear what problem, you
>> are trying to fix here. Do you mean to say that fcopy does not work on
>> Linux? Or you are assuming it won't work and fixing some generic
>> problem?
>> Fcopy is supposed to work fine on Linux VM on HyperV with windows 
>> host. If there are some errors, please share in cover letter/comments
>> in the patch along with steps of execution.
>>
>> 5. If its a fix, we should have a proper Fixes tag with the commit you
>> are fixing.
>> 6. Have a look at existing conversations at lore to get to know common
>> practices with single patch, multi patch, cover letters etc.
>> https://lore.kernel.org/linux-hyperv/?q=linux-hyperv
>>
>> Regards,
>> Naman
>>


Regards,
Naman
>>

Re: [PATCH v3 1/1] hv_fcopy_uio_daemon: Fix file copy failure between Windows host and Linux guest

[Yasumasa Suenaga]

Hi Naman,

Thanks a lot for your help!

I will fix for your comment, and send as v4 patch next week :)

> Please help me understand if this is correct. The actual problem you are highlighting lies here when sizeof(wchat_t) is more than 16, i.e. 32?

Yes, so we should not cast array of __u16 to wchar_t*.
I confirmed it on Linux as following:
```
[yasuenag@fc42 sizeof]$ cat test.c
#include <stdio.h>
#include <stddef.h>

int main(){
   printf("%ld\n", sizeof(wchar_t));
   return 0;
}
[yasuenag@fc42 sizeof]$ rpm -q gcc
gcc-15.1.1-2.fc42.x86_64
[yasuenag@fc42 sizeof]$ gcc test.c
[yasuenag@fc42 sizeof]$ ./a.out
4
```

In Windows, it returns 2:
```
PS C:\test\sizeof> cl.exe .\test.c
Microsoft(R) C/C++ Optimizing Compiler Version 19.44.35208 for x86
Copyright (C) Microsoft Corporation.  All rights reserved.

test.c
Microsoft (R) Incremental Linker Version 14.44.35208.0
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:test.exe
test.obj
PS C:\test\sizeof> .\test.exe
2
```


Thanks,

Yasumasa


On 2025/06/04 21:36, Naman Jain wrote:
> 
> 
> On 6/4/2025 2:40 PM, Yasumasa Suenaga wrote:
>> Hi Naman,
>> Sorry for bothering you, but I have some questions:
>>
>>
> 
> Hi,
> Happy to help. I have tested your patch on my setup, and it did not break anything for me. Please find response and additional questions inline.
> 
> 
>>> 1. Create a new patch file for every new version and then send it.
>>> Currently it seems you are manually editing the same patch file in the
>>> subject and sending it, so each patch version is showing up in the same
>>> thread.
>>
>> I've committed changes with "git amend" and created formatted patch with "git format-patch",
>> then I sent all of *.patch files via "git send-email".
>> Do you mean I should commit to fix commits reviewers and send cover letter and incremental
>> patches only? I couldn't find about it from the guide.
>> I checked linux-hyperv list, all of patches have been sent in each version AFAICS.
>>
> 
> * make changes
> * git add, git commit, git format-patch, git send-email <path to v1 patch)
> * make changes for v2
> * git add, git commit --amend, git format-patch -v2, git send-email <path to v2 patch *only*>
> and so on.
> 
> I follow above practice, and the patches go in new threads. I am not sure why they are landing in same thread for you. Anyhow, its not a big thing.
> 
>>
>>> 3. Keep a minimum of 1-2 weeks gap between successive patch versions to
>>> give time to people to review your changes.
>>
>> Does it include changes of commit message too?
> 
> Other reviewers may be reviewing older versions and it becomes confusing if new versions come too soon. To give everyone enough time to review your changes and to not have unnecessary patch versions, its better to wait for a few days, collect the feedback and address it together. We also need to mention what we changed since last time, to help reviewers know if their comments got addressed.
> 
> For single patch, we usually don't add a cover letter. You can refer
> https://lore.kernel.org/all/20250102145243.2088-1-namjain@linux.microsoft.com/
> The comments between the two "---" does not make it to git log.
> 
>>
>>
>>> 4. In the commit msg of v3, it is still not very clear what problem, you
>>> are trying to fix here. Do you mean to say that fcopy does not work on
>>> Linux? Or you are assuming it won't work and fixing some generic
>>> problem?
>>> Fcopy is supposed to work fine on Linux VM on HyperV with windows host. If there are some errors, please share in cover letter/comments
>>> in the patch along with steps of execution.
>>
>> I have a problem on my PC:
>>
>>    Host: Windows 11 Pro (24H2 build 26100.4202)
>>    Guest: Fedora 42
>>      - kernel-6.14.4-300.fc42.x86_64
>>      - hypervfcopyd-6.10-1.fc42.x86_64
>>
>> How to reproduce: run Copy-VMFile commandlet on Host:
>>
>> Following log is in Japanese because my PC is set to Japanese, sorry.
>> But it says fcopy could not transfer file (test.ps1) to /tmp/ on Linux guest because it already exists.
>> I confirmed /tmp/test.ps1 does not exist of course.
>>
>> ```
>>> Copy-VMFile
>>
>> cmdlet Copy-VMFile at command pipeline position 1
>> Supply values for the following parameters:
>> Name[0]: Fedora42
>> Name[1]:
>> SourcePath: test.ps1
>> DestinationPath: /tmp/
>> FileSource: Host
>> Copy-VMFile: ゲストへのファイルのコピーを開始できませんでした。
>>
>> ソース ファイル 'C:\test\test.ps1' をゲストの宛先 '/tmp/' にコピーできま せんでした。
>>
>> 'Fedora42' はゲスト: ファイルがあります。 (0x80070050) へのファイルのコ ピーを開始できませんでした。(仮想マシン ID 9BFDF23D-CCAA-4748- A770-6D654E09A133)
>>
>> 'Fedora42' は、コピー元ファイル 'C:\test\test.ps1' をゲスト上のコピー先 '/tmp/' にコピーできませんでした: ファイルがあります。 (0x80070050)。(仮 想マシン ID 9BFDF23D-CCAA-4748-A770-6D654E09A133)
>> ```
>>
>> I got following fcopy log from journald - it is strange because "/tmp/ test.ps1" should be shown here.
>> ```
>> 6月 04 17:48:24 fc42 HV_UIO_FCOPY[1080]: File: / exists
>> ```
>>
>> As I wrote in commit message, wchar_t is 32 bit in Linux. I confirmed it with "sizeof(wchar_t)".
>> However fcopyd handles it as 16 bit value (__u16), thus I think this is a bug in fcopy, and
>> I think it would also not work on other environments.
>>
>> Actually it works fine with my patch to handle values as 16 bit.
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> On 2025/06/04 15:19, Naman Jain wrote:
>>>
>>>
>>> On 6/4/2025 5:13 AM, yasuenag@gmail.com wrote:
>>>> From: Yasumasa Suenaga <yasuenag@gmail.com>
>>>>
>>>> Handle file copy request from the host (e.g. Copy-VMFile commandlet)
>>>> correctly.
>>>> Store file path and name as __u16 arrays in struct hv_start_fcopy.
>>>> Convert directly to UTF-8 string without casting to wchar_t* in fcopyd.
>>>>
>>>> Fix string conversion failure caused by wchar_t size difference between
>>>> Linux (32bit) and Windows (16bit). Convert each character to char
>>>> if the value is less than 0x80 instead of using wcstombs() call.
>>>>
>>>> Add new check to snprintf() call for target path creation to handle
>>>> length differences between PATH_MAX (Linux) and W_MAX_PATH (Windows).
>>>>
>>>> Signed-off-by: Yasumasa Suenaga <yasuenag@gmail.com>
>>>> ---
>>>>   tools/hv/hv_fcopy_uio_daemon.c | 37 ++++++++++++++--------------------
>>>>   1 file changed, 15 insertions(+), 22 deletions(-)
>>>>
>>>> diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/ hv_fcopy_uio_daemon.c
>>>> index 0198321d1..86702f39e 100644
>>>> --- a/tools/hv/hv_fcopy_uio_daemon.c
>>>> +++ b/tools/hv/hv_fcopy_uio_daemon.c
>>>> @@ -62,8 +62,11 @@ static int hv_fcopy_create_file(char *file_name, char *path_name, __u32 flags)
>>>>       filesize = 0;
>>>>       p = path_name;
>>>> -    snprintf(target_fname, sizeof(target_fname), "%s/%s",
>>>> -         path_name, file_name);
>>>> +    if (snprintf(target_fname, sizeof(target_fname), "%s/%s",
>>>> +             path_name, file_name) >= sizeof(target_fname)) {
>>>> +        /* target file name is too long */
> 
> Please add a syslog for this. It will help in debugging issues.
> 
>>>> +        goto done;
>>>> +    }
>>>>       /*
>>>>        * Check to see if the path is already in place; if not,
>>>> @@ -273,6 +276,8 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
>>>>       while (len < dest_size) {
>>>>           if (src[len] < 0x80)
>>>>               dest[len++] = (char)(*src++);
>>>> +        else if (src[len] == '0')
>>>> +            break;
> 
> While this also works, I think you can add it to the while loop itself
> while (len < dest_size && src[len])
> 
> Is this related to the FCopy issue that you see or this is just a fix for correct destination file name?
> 
> 
>>>>           else
>>>>               dest[len++] = 'X';
>>>>       }
>>>> @@ -282,27 +287,15 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
>>>>   static int hv_fcopy_start(struct hv_start_fcopy *smsg_in)
>>>>   {
>>>> -    setlocale(LC_ALL, "en_US.utf8");
>>>> -    size_t file_size, path_size;
>>>> -    char *file_name, *path_name;
>>>> -    char *in_file_name = (char *)smsg_in->file_name;
>>>> -    char *in_path_name = (char *)smsg_in->path_name;
>>>> -
>>>> -    file_size = wcstombs(NULL, (const wchar_t *restrict)in_file_name, 0) + 1;
>>>> -    path_size = wcstombs(NULL, (const wchar_t *restrict)in_path_name, 0) + 1;
>>>> -
> 
> Please help me understand if this is correct. The actual problem you are highlighting lies here when sizeof(wchat_t) is more than 16, i.e. 32?
> 
>>>> -    file_name = (char *)malloc(file_size * sizeof(char));
>>>> -    path_name = (char *)malloc(path_size * sizeof(char));
>>>> -
>>>> -    if (!file_name || !path_name) {
>>>> -        free(file_name);
>>>> -        free(path_name);
>>>> -        syslog(LOG_ERR, "Can't allocate memory for file name and/or path name");
>>>> -        return HV_E_FAIL;
>>>> -    }
>>>> +    /*
>>>> +     * file_name and path_name should have same length with appropriate
>>>> +     * member of hv_start_fcopy.
>>>> +     */
>>>> +    char file_name[W_MAX_PATH], path_name[W_MAX_PATH];
>>>> -    wcstoutf8(file_name, (__u16 *)in_file_name, file_size);
>>>> -    wcstoutf8(path_name, (__u16 *)in_path_name, path_size);
>>>> +    setlocale(LC_ALL, "en_US.utf8");
>>>> +    wcstoutf8(file_name, smsg_in->file_name, W_MAX_PATH - 1);
>>>> +    wcstoutf8(path_name, smsg_in->path_name, W_MAX_PATH - 1);
>>>>       return hv_fcopy_create_file(file_name, path_name, smsg_in- >copy_flags);
>>>>   }
>>>
>>> Hi,
>>> I understand this is your first patch for upstreaming. Here are a few
>>> things you should consider:
>>> 1. Create a new patch file for every new version and then send it.
>>> Currently it seems you are manually editing the same patch file in the
>>> subject and sending it, so each patch version is showing up in the same
>>> thread.
>>> 2. Read, re-read, absorb the information in the link that Easwar also
>>> mentioned:
>>> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>>> 3. Keep a minimum of 1-2 weeks gap between successive patch versions to
>>> give time to people to review your changes.
>>> 4. In the commit msg of v3, it is still not very clear what problem, you
>>> are trying to fix here. Do you mean to say that fcopy does not work on
>>> Linux? Or you are assuming it won't work and fixing some generic
>>> problem?
>>> Fcopy is supposed to work fine on Linux VM on HyperV with windows host. If there are some errors, please share in cover letter/comments
>>> in the patch along with steps of execution.
>>>
>>> 5. If its a fix, we should have a proper Fixes tag with the commit you
>>> are fixing.
>>> 6. Have a look at existing conversations at lore to get to know common
>>> practices with single patch, multi patch, cover letters etc.
>>> https://lore.kernel.org/linux-hyperv/?q=linux-hyperv
>>>
>>> Regards,
>>> Naman
>>>
> 
> 
> Regards,
> Naman
>>>
>