Re: shell32: FOF_MULTIDESTFILES must be set when copying files into directory
2008/10/17 Vitaly Perov <vitperov(a)etersoft.ru>:
From e2eec69ff86960bd6729a3105747310ce9c256fd Mon Sep 17 00:00:00 2001 From: Vitaly Perov <vitperov(a)etersoft.ru> Date: Fri, 17 Oct 2008 16:56:46 +0400 Subject: [PATCH] shell32: FOF_MULTIDESTFILES must be set when copying files into directory
--- dlls/shell32/shlfileop.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/dlls/shell32/shlfileop.c b/dlls/shell32/shlfileop.c index 49f61d3..e6565ea 100644 --- a/dlls/shell32/shlfileop.c +++ b/dlls/shell32/shlfileop.c @@ -1382,6 +1382,7 @@ static void move_dir_to_dir(LPSHFILEOPSTRUCTW lpFileOp, const FILE_ENTRY *feFrom fileOp = *lpFileOp; fileOp.pFrom = szFrom; fileOp.pTo = szTo; + fileOp.fFlags |= FOF_MULTIDESTFILES;
SHFileOperationW(&fileOp); RemoveDirectoryW(feFrom->szFullPath);
We've been over this before. This needs a test case. Either way, this doesn't look right. What are you trying to fix? -- James Hawkins
We've been over this before. This needs a test case. Either way, this doesn't look right. What are you trying to fix?
Some program call SHFileOperationW to move one folder to another without FOF_MULTIDESTFILES. The source folder contain subfolders, so SHFileOperationW is called recursively. To move subfolder function move_dir_to_dir is called. Now it pass parent lpFileOp to the new iteration of SHFileOperationW, so FOF_MULTIDESTFILES is no set (but it should be set, as we copy files to the folder). So in the function move_files the following condition is true: if (!(lpFileOp->fFlags & FOF_MULTIDESTFILES) && !flFrom->bAnyDirectories && flFrom->dwNumFiles > flTo->dwNumFiles) { return ERROR_CANCELLED; } and SHFileOperationW return error. Ok, I write a test, but it wouldn't be soon. -- Best wishes, Vitaly Perov Russia, Saint-Petersburg. www.etersoft.ru
On Sat, Oct 18, 2008 at 5:48 AM, Vitaly Perov <vitperov(a)etersoft.ru> wrote:
We've been over this before. This needs a test case. Either way, this doesn't look right. What are you trying to fix?
Some program call SHFileOperationW to move one folder to another without FOF_MULTIDESTFILES. The source folder contain subfolders, so SHFileOperationW is called recursively. To move subfolder function move_dir_to_dir is called. Now it pass parent lpFileOp to the new iteration of SHFileOperationW, so FOF_MULTIDESTFILES is no set (but it should be set, as we copy files to the folder). So in the function move_files the following condition is true:
if (!(lpFileOp->fFlags & FOF_MULTIDESTFILES) && !flFrom->bAnyDirectories && flFrom->dwNumFiles > flTo->dwNumFiles) { return ERROR_CANCELLED; }
and SHFileOperationW return error.
You never mentioned a specific program, so I still don't know if this is a real world bug you're running into. Either way, your logic is flawed, as fileOp.pTo is a single directory in this case, so using FOF_MULTIDESTFILES is wrong. -- James Hawkins
You never mentioned a specific program, so I still don't know if this is a real world bug you're running into. Either way, your logic is flawed, as fileOp.pTo is a single directory in this case, so using FOF_MULTIDESTFILES is wrong.
MSDN: FOF_MULTIDESTFILES The pTo member specifies multiple destination files (one for each source file in pFrom) rather than one directory where all source files are to be deposited. As I understand, FOF_MULTIDESTFILES must be set, when pTo is directory The program is Garant 7 F1 (Russian legal system database). I don't think you know this program. It is not a real world bug, but there are some significant bugs in SHFileOperationW. I don't think a lot of programs use SHFileOperationW to move a files, so these fixes maybe don't fix bugs in bugzilla, but is it criterion why pathes wouldnt't be applied? I solve these bugs for Etersoft. Now in Etersoft wine Garant installs perfectly. But I want to fix these bugs in wine. I don't think my patches are perfect. I can fix them if you say what's wrong. I used to write a tests, but even a tests are ignored. Without any explanation. I was resending them, but had no answer again. Sometimes I think it's just waste of time. I have a lot of patches which I could send, but I think they wouldn't be applied just because they don't fix any "real world bug". -- Best wishes, Vitaly Perov Russia, Saint-Petersburg. www.etersoft.ru
2008/10/18 Vitaly Perov <vitperov(a)etersoft.ru>:
The program is Garant 7 F1 (Russian legal system database). I don't think you know this program. It is not a real world bug, but there are some significant bugs in SHFileOperationW. I don't think a lot of programs use SHFileOperationW to move a files, so these fixes maybe don't fix bugs in bugzilla, but is it criterion why pathes wouldnt't be applied? I solve these bugs for Etersoft. Now in Etersoft wine Garant installs perfectly. But I want to fix these bugs in wine. I don't think my patches are perfect. I can fix them if you say what's wrong. I used to write a tests, but even a tests are ignored. Without any explanation. I was resending them, but had no answer again. Sometimes I think it's just waste of time.
Having tests (as James has mentioned in this thread) improves the chances of patches getting in if they relate to bugs in Wine's implementation of the Windows API. Having applications that are affected by these bugs can be helpful, but is not a requirement (I fixed some issues with Wine's implementation of SHCreateStreamOnFile that didn't have any known applications that affect them, but I did this by writing test cases for it to highlight where Wine's behaviour was wrong; I wrote the patches to fix the issues only after I had the tests in and working). Have you tried asking on the #winehackers IRC channel? Sometimes, that is better than the mailing lists to get a response. Especially asking Alexandre Julliard directly on there. But also, when you do get a response, listen to it. Tests are necessary, especially when the code changes some fundamental behaviour, or code that is (or appears to be) working properly. Also, when did you submit the other patches? Patchwatcher has been put in place to run the conformance tests to pick up patches with issues. If they fail there, they won't be applied.
I have a lot of patches which I could send, but I think they wouldn't be applied just because they don't fix any "real world bug".
All bugs are important, even those that don't have any known applications that are affected by them. The point is that you need to prove that Windows is behaving in one way and that Wine is behaving in another. It can sometimes be difficult to get patches accepted into Wine, and sometimes you need to rework a patch or series of patches several times before they are accepted. The patches you have will be useful to the community. If you don't want to continue work on the patches (but I urge you to be more persistent, and try again), I would suggest you send them anyway so that other people can work on them so they can be accepted as a joint effort. - Reece
Thank you for your answer. I agree, that before sending a patch I should write a test first. But the problem is that even a tests are ignored without any explanation. Maybe my tests aren't good, but why anybody just tell what's wrong. Maybe a'm not enough persistent?
Have you tried asking on the #winehackers IRC channel? No, I doesn't use IRC. It's very hard to me to write in english in real-time
Ok, I try to resend my old tests, and write tests to other patches. Also there are some functions and stubs implemented, wchich doesn't require (as I think) a tests. -- Best wishes, Vitaly Perov Russia, Saint-Petersburg. www.etersoft.ru
On Sat, Oct 18, 2008 at 7:47 AM, Vitaly Perov <vitperov(a)etersoft.ru> wrote:
You never mentioned a specific program, so I still don't know if this is a real world bug you're running into. Either way, your logic is flawed, as fileOp.pTo is a single directory in this case, so using FOF_MULTIDESTFILES is wrong.
MSDN: FOF_MULTIDESTFILES The pTo member specifies multiple destination files (one for each source file in pFrom) rather than one directory where all source files are to be deposited.
As I understand, FOF_MULTIDESTFILES must be set, when pTo is directory
The program is Garant 7 F1 (Russian legal system database). I don't think you know this program. It is not a real world bug, but there are some significant bugs in SHFileOperationW. I don't think a lot of programs use SHFileOperationW to move a files, so these fixes maybe don't fix bugs in bugzilla, but is it criterion why pathes wouldnt't be applied? I solve these bugs for Etersoft. Now in Etersoft wine Garant installs perfectly. But I want to fix these bugs in wine. I don't think my patches are perfect. I can fix them if you say what's wrong. I used to write a tests, but even a tests are ignored. Without any explanation. I was resending them, but had no answer again. Sometimes I think it's just waste of time. I have a lot of patches which I could send, but I think they wouldn't be applied just because they don't fix any "real world bug".
Just blindly rummaging through code you don't understand very well (and yes, it's a complex beast of a function) is a recipe for disaster, and by disaster I mean introducing new bugs. Your assumption about this 'potential' bug is wrong.
MSDN: FOF_MULTIDESTFILES The pTo member specifies multiple destination files (one for each source file in pFrom) rather than one directory where all source files are to be deposited.
As I understand, FOF_MULTIDESTFILES must be set, when pTo is directory
That is incorrect. Please read the definition more carefully. The only time FOF_MULTIDESTFILES should be provided is when there are *multiple files* in pTo, and the definition explicitly says "rather than one directory." -- James Hawkins
participants (3)
-
James Hawkins -
Reece Dunn -
Vitaly Perov