shell32: FOF_MULTIDESTFILES must be set when copying files into directory

Michael Karcher wine at mkarcher.dialup.fu-berlin.de
Sat Oct 18 18:17:23 CDT 2008


Am Samstag, den 18.10.2008, 22:18 +0000 schrieb Louis. Lenders:


> Spent loads of time for more than 3 years trying to help triaging bugs
> and helping out in appdb, only to find out that all my patches are
> ignored silently

Sorry, what do you mean by *all* of your patches?

This is from the wine git log:

2008-09-25	Louis Lenders	msi: Add stub for MsiSetExternalUIRecord.
2008-09-25	Louis Lenders	shdocvw: Create default App Paths key for iexplore... 
2008-09-04	Louis Lenders	wine.inf: Add default Directx registry key for InstalledVersion.
2008-08-28	Louis Lenders	shobjidl.idl: Add Taskbarlist interface definitions.
2008-08-28	Louis Lenders	shlwapi: Fix UrlUnEscape to expand URLs in-place even... 
2008-08-28	Louis Lenders	shlwapi: Add test showing UrlUnEscape should convert... 
2008-06-23	Louis Lenders	d3dx9_*: Add version resources.
2008-06-21	Louis Lenders	advapi32: Add stub for GetAuditedPermissionsFromAcl... 
2008-06-20	Louis Lenders	kernel32: Fix typo in SetProcessAffinityMask.
2008-06-10	Louis Lenders	mscoree: Add stub for CorBindToCurrentRuntime.
2008-05-29	Louis Lenders	wine.inf: Add fake glu32.
2008-04-18	Louis Lenders	wininet: Improve stub for FindNextUrlCacheEntryW a... 
2008-04-17	Louis Lenders	urlmon: Add stub for CoInternetSetFeatureEnabled.
2008-03-12	Louis Lenders	oleacc: Add GetOleaccVersionInfo.
2008-03-07	Louis Lenders	shdocvw: Return something more useful for WebBrowser_get_Rea... 
2008-02-29	Louis Lenders	programs: Add a stubbed out secedit.exe.
2008-02-21	Louis Lenders	shdocvw: Pretend success in WebBrowser_get_RegisterAsDropTarget.
2008-01-11	Louis Lenders	shdocvw: Change return value for PersistMemory_Load.
2008-01-10	Louis Lenders	user32: Add stub for GetLayeredWindowAttributes.
[further patches back to 2005-12-06]


> If people spent quite an amount of time trying to get a patch written,
> it shouldn't be too much trouble to just drop a note on wine-devel,
> what the reason is the patch got rejected.

In the case of the FOF_MULTIDESTFILES test patch that Vitaly didn't get
in, I don't see an obvious problem in the last version, although that
patch could be extended.

I suspect (but I am definitely not an expert in this area), that the
patch Vitaly Perov sent:

| +    /* move many files into directory with FOF_MULTIDESTFILES */
| +    set_curr_dir_path(from, "test?.txt\0");
| +    set_curr_dir_path(to, "testdir2\0");
| +    retval = SHFileOperationA(&shfo2);
| +    todo_wine
| +    {
| +        ok(retval == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", retval);
| +        ok(file_exists("testdir2\\test2.txt"), "Expected the file 'test2.txt' to exist\n");
| +        ok(file_exists("testdir2\\test4.txt"), "Expected the directory 'test4.txt' to exist\n");
| +    }

works because FOF_MULTIDESTFILES makes SHFileOperation pair from entries
with to entries. As in this case, the from entry only has one component,
namely "test?.txt" (even if it expands to multiple files), only one
component of the destination is used regardless of FOF_MULTIDESTFILES.

It would interesting to see what happens in the case that from contains
two wildcard patterns, like "test?.txt\0test_?.txt\0" and dest contains
two destinations like "testdir2\0testdir2\\nested\0". If my intuition is
right, without FOF_MULTIDESTFILES everything is moved into testdir2, and
with FOF_MULTIDESTFILES, test1.txt, test2.txt, test3.txt and the
directory test4.txt is moved to testdir2, whereas test_5.txt is moved to
testdir2\nested.

But in the issue that started the thread I have to agree with James.
FOF_MULTIDESTFILES is not to be set just because the destination is a
directory. I quote Vitaly quoting 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.

In this sentence "rather than" is the same as "instead of". So
FOF_MULTIDESTFILES indicates that the pTo member does *not* specify one
destination directory, but a list of destination files instead. As the
test that did not get committed seems to show (if it was tested in
Windows, stating that helps getting a patch committed; sending a patch
that does not work on any Windows version is risky, as it might put you
on the infamous s**t list temporarily), the "multiple destination files"
MSDN talks about might also be multiple destination directories.

> >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.
Even if you submit a stub, you usually do that because an application
needs that function. In this case, it might be helpfull for people
trying to implement that function to find an example use case in the
testcase, so adding a todo_wine test is a nice thing, even if you just
add a stub (it also shows in the test statistics that something needs to
be done).

The tests do not only serve the purpose to test that Wine does what we
expect wine to do (in fact, that is only a minor point of the tests),
but more importantly, they should show that Wine and Windows do the
*same* thing, so writing a testcase that passes after your patch has
been applied to wine is a no-brainer unless you tested that this test
also passes on Windows.

If a patch that contains only a test did not get committed and you
resubmit it, please add a note on which Windows version you tested it,
if you didn't include it in your first try. General hint: If you can't
test on windows for some reason (like you propose a direct3d test that
needs a graphics card you don't own), do not send the testcase to
wine-patches, but to wine-devel, and explain briefly why you can't test,
and ask other developers to test it for you. This is usually not a
problem.

Regards,
  Michael Karcher





More information about the wine-devel mailing list