August 2011 - Wine-Devel - list.winehq.org

Re: [PATCH 4/5] mountmgr.sys: Also unlink a:: drive when removing a dos device

by Alexandre Julliard

GOUJON Alexandre <ale.goujon(a)gmail.com> writes: > What happened : when you click on the apply button, > apply_drive_changes() is called, then mountmgr_ioctl(), then > define_unix_drive(), then remove_dos_device() which unlink() k: but > not k:: Like I said, that's on purpose. -- Alexandre Julliard julliard(a)winehq.org

Aug. 31, 2011

Re: [PATCH 1/5] kernel32: Add UDF support

by Alexandre Julliard

GOUJON Alexandre <ale.goujon(a)gmail.com> writes: > As I said earlier, it also fails without my patch. > I will investigate but my patch does not cause a regression (as all > new features). It certainly doesn't fail here without your patch. -- Alexandre Julliard julliard(a)winehq.org

Aug. 31, 2011

Re: [PATCH 4/5] mountmgr.sys: Also unlink a:: drive when removing a dos device

by GOUJON Alexandre

On 08/31/2011 01:16 PM, Alexandre Julliard wrote: > Alexandre Goujon<ale.goujon(a)gmail.com> writes: > >> @@ -818,6 +818,7 @@ NTSTATUS remove_dos_device( int letter, const char *udi ) >> if ((path = get_dosdevices_path(&p ))) >> { >> p[0] = 'a' + drive->drive; >> + unlink( path ); > That's not done on purpose, to make it more likely that we use the same > drive letter next time the device is mounted. > Sorry but your explanation doesn't convince me. Here is a simple test case: - open winecfg then the Drives tab - make sure your fstab contains a mount point for a block device like /dev/cdrom - click Autodetect then apply - $ls -l .wine/dosdevices/ you should see something like k: pointing to the mounted point like /media/cdrom et k:: pointing to /dev/cdrom with k the right letter - remove k then apply k: has been removed but not k:: What happened : when you click on the apply button, apply_drive_changes() is called, then mountmgr_ioctl(), then define_unix_drive(), then remove_dos_device() which unlink() k: but not k::

Aug. 31, 2011

Re: [PATCH 1/5] kernel32: Add UDF support

by GOUJON Alexandre

On 08/31/2011 01:21 PM, Alexandre Julliard wrote: > Alexandre Goujon<ale.goujon(a)gmail.com> writes: > >> Based on Steven Wallace work. >> Should fix the wine side of bug #26273 > It fails here: > > ../../../tools/runtest -q -P wine -M kernel32.dll -T ../../.. -p kernel32_test.exe.so volume.c&& touch volume.ok > volume.c:105: Tests skipped: can't test removing fake drive > volume.c:359: Test failed: GetVolumeInformationA root failed, last error 66 > volume.c:401: Test failed: GetVolumeInformationA with \ failed, last error 66 > volume.c:423: Test failed: GetVolumeInformationA failed, last error 66 > volume.c:434: Test failed: GetVolumeInformationA failed, last error 66 > volume.c:445: Test failed: GetVolumeInformationA failed, root=C:\, last error=66 > volume.c:451: Test failed: GetVolumeInformationA did fail, root=\\?\C:\, last error=66 > volume.c:458: Test failed: GetVolumeInformationA did fail, root=\\.\C:\, last error=66 > volume.c:486: Test failed: GetVolumeInformationA failed, root=\\?\Volume{00000000-0000-0000-0000-000000000043}\, last error=66 > make[1]: *** [volume.ok] Error 8 > As I said earlier, it also fails without my patch. I will investigate but my patch does not cause a regression (as all new features).

Aug. 31, 2011

Re: [PATCH 3/5] kernel32: Improve GetVolumeInformation filesystem flags

by GOUJON Alexandre

On 08/31/2011 01:15 PM, Alexandre Julliard wrote: > Alexandre Goujon<ale.goujon(a)gmail.com> writes: > >> if (fsname) lstrcpynW( fsname, ntfsW, fsname_len ); >> if (filename_len) *filename_len = 255; >> - if (flags) *flags = FILE_CASE_PRESERVED_NAMES; >> + if (flags) *flags = FILE_NAMED_STREAMS | FILE_SUPPORTS_ENCRYPTION | FILE_SUPPORTS_OBJECT_IDS >> + | FILE_SUPPORTS_REPARSE_POINTS | FILE_SUPPORTS_SPARSE_FILES | FILE_VOLUME_QUOTAS >> + | FILE_FILE_COMPRESSION | FILE_PERSISTENT_ACLS | FILE_UNICODE_ON_DISK >> + | FILE_CASE_PRESERVED_NAMES | FILE_CASE_SENSITIVE_SEARCH; > I'm not sure it's a good idea to claim features we don't support. Do you > have an app that needs this? > No, I don't. When I worked on the patch, I made tests with several filesystems and noticed the values were wrong and variable across windows versions. I asked here [1] what to do but no one answered me. I also saw a FIXME comment in wine so I thought it was a good idea to fix these. But I think you're right, an app may rely on what we claim, so you can reject this one. --- [1] : http://www.winehq.org/pipermail/wine-devel/2011-August/091390.html

Aug. 31, 2011

Re: user32: fix potential crash when loading unsupported files (with tests) (try 2) (resend)

by Alexandre Julliard

Vitaly Perov <vitperov(a)etersoft.ru> writes: > @@ -1238,6 +1260,12 @@ static void test_LoadImage(void) > test_LoadImageFile("JPG", jpgimage, sizeof(jpgimage), "jpg", 0); > test_LoadImageFile("PNG", pngimage, sizeof(pngimage), "png", 0); > > + /* Corrupted images */ > + test_LoadCorruptedImageFile("corrupted_BMP", bmpimage, sizeof(bmpimage), "bmp", 1); > + test_LoadCorruptedImageFile("corrupted_GIF", gifimage, sizeof(gifimage), "gif", 0); > + test_LoadCorruptedImageFile("corrupted_JPG", jpgimage, sizeof(jpgimage), "jpg", 0); > + test_LoadCorruptedImageFile("corrupted_PNG", pngimage, sizeof(pngimage), "png", 0); > + That's an interesting test, but it has nothing to do with your fix. We shouldn't crash on loading corrupted files, but we shouldn't crash if idReserved is 0 either. Your proposed fix is just hiding the problem. Checking idReserved may or may not be a good idea, independently of fixing that crash, but that would need a different test. -- Alexandre Julliard julliard(a)winehq.org

Aug. 31, 2011

Re: [PATCH 1/5] kernel32: Add UDF support

by Alexandre Julliard

Alexandre Goujon <ale.goujon(a)gmail.com> writes: > Based on Steven Wallace work. > Should fix the wine side of bug #26273 It fails here: ../../../tools/runtest -q -P wine -M kernel32.dll -T ../../.. -p kernel32_test.exe.so volume.c && touch volume.ok volume.c:105: Tests skipped: can't test removing fake drive volume.c:359: Test failed: GetVolumeInformationA root failed, last error 66 volume.c:401: Test failed: GetVolumeInformationA with \ failed, last error 66 volume.c:423: Test failed: GetVolumeInformationA failed, last error 66 volume.c:434: Test failed: GetVolumeInformationA failed, last error 66 volume.c:445: Test failed: GetVolumeInformationA failed, root=C:\, last error=66 volume.c:451: Test failed: GetVolumeInformationA did fail, root=\\?\C:\, last error=66 volume.c:458: Test failed: GetVolumeInformationA did fail, root=\\.\C:\, last error=66 volume.c:486: Test failed: GetVolumeInformationA failed, root=\\?\Volume{00000000-0000-0000-0000-000000000043}\, last error=66 make[1]: *** [volume.ok] Error 8 -- Alexandre Julliard julliard(a)winehq.org

Aug. 31, 2011

Re: [PATCH 4/5] mountmgr.sys: Also unlink a:: drive when removing a dos device

by Alexandre Julliard

Alexandre Goujon <ale.goujon(a)gmail.com> writes: > @@ -818,6 +818,7 @@ NTSTATUS remove_dos_device( int letter, const char *udi ) > if ((path = get_dosdevices_path( &p ))) > { > p[0] = 'a' + drive->drive; > + unlink( path ); That's not done on purpose, to make it more likely that we use the same drive letter next time the device is mounted. -- Alexandre Julliard julliard(a)winehq.org

Aug. 31, 2011

Re: [PATCH 3/5] kernel32: Improve GetVolumeInformation filesystem flags

by Alexandre Julliard

Alexandre Goujon <ale.goujon(a)gmail.com> writes: > if (fsname) lstrcpynW( fsname, ntfsW, fsname_len ); > if (filename_len) *filename_len = 255; > - if (flags) *flags = FILE_CASE_PRESERVED_NAMES; > + if (flags) *flags = FILE_NAMED_STREAMS | FILE_SUPPORTS_ENCRYPTION | FILE_SUPPORTS_OBJECT_IDS > + | FILE_SUPPORTS_REPARSE_POINTS | FILE_SUPPORTS_SPARSE_FILES | FILE_VOLUME_QUOTAS > + | FILE_FILE_COMPRESSION | FILE_PERSISTENT_ACLS | FILE_UNICODE_ON_DISK > + | FILE_CASE_PRESERVED_NAMES | FILE_CASE_SENSITIVE_SEARCH; I'm not sure it's a good idea to claim features we don't support. Do you have an app that needs this? -- Alexandre Julliard julliard(a)winehq.org

Aug. 31, 2011

Re: cmd/tests: Test del /s recursion in subdirectories with colons.

by Octavian Voicu

2011/8/31 Frédéric Delanoy <frederic.delanoy(a)gmail.com>: > My point was simply to make sure that subsequent wine patches, maybe a > year or two from now, won't make it "work" again (think regression) You got that wrong -- it *does* work on Wine already :) I think we can be more permissive in this aspect, it's not like some weird app will fail because it succeeds in creating a "foo:" directory. > I never talked about marking it as broken (it obviously doesn't work > on any windows), just that running a plain "mkdir foo:" and not > checking the expected outcome is pointless IMHO (besides detecting > crash/overflows don't occur again in wine) > Maybe a simple "if exist foo: (echo wine is broken)" would be > sufficient. This way you would detect quickly it wine somehow makes it > "work" again. Mkdir / cd only fail if the 2nd char is a colon (so "a:", "a:foo", "1:" all fail), presumably because they parse that as a root drive. Mkdir "foo:" works on Wine. That was exactly the point of the patch, make sure the crash/overflow doesn't creep back in. I guess you had something else in mind when you suggested I write a test. Octavian

Aug. 31, 2011