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

Question about ddraw implemtation

by Ralph Little

Hi, I have been looking into bug #9672 regarding a crash on startup of The Sims. Tracing through what is happening, the problem seems to be manifesting in the DirectDraw implementation. The Sims calls CreateSurface() with DDSD->dwFlags & DDSD_PIXELFORMAT set to state that a pixformat is specified in u4.ddpfPixelFormat, but no optional flags are set in the pixelformat u4.ddpfPixelFormat.dwFlags [DDPIXELFORMAT]. The function PixelFormat_DD2WineD3D uses this to determine the proper WINE3D pixformat implementation. It does not cope with what I presume is the default situation of no flags having been set, the value being 0. I'm thinking that if you provide 0 for u4.ddpfPixelFormat.dwFlags, you get some basic default, but I don't know what that would be. Certainly, The Sims seems to be providing zero for this parameter. Any ideas what is the proper behaviour in this circumstance? I presume the crash is the game's inability to handle the failure of the surface create gracefully. Here is the relevant trace fragment (with my bit of debug "pixelformat given, flags = 0" added): Cheers, Ralph ======================================= trace:ddraw:ddraw4_CreateSurface iface 0x139f0c, surface_desc 0x34f944, surface 0x34f9c0, outer_unknown (nil). trace:ddraw:CreateSurface ddraw 0x139f08, surface_desc 0x34f944, surface 0x34f9c0, outer_unknown (nil). trace:ddraw:CreateSurface (0x139f08) Requesting surface desc : trace:ddraw:DDRAW_dump_members - DDSD_CAPS : DDSCAPS_SYSTEMMEMORY DDSCAPS_ZBUFFER trace:ddraw:DDRAW_dump_members - DDSD_HEIGHT : 600 trace:ddraw:DDRAW_dump_members - DDSD_WIDTH : 800 trace:ddraw:DDRAW_dump_members - DDSD_PIXELFORMAT : ( ) warn:ddraw:CreateSurface pixelformat given, flags = 0 trace:ddraw:ddraw_create_surface ddraw 0x139f08, surface_desc 0x34f7ec, surface 0x34f8b4, level 0. trace:ddraw:ddraw_create_surface (0x139f08) Requesting surface desc : trace:ddraw:DDRAW_dump_members - DDSD_CAPS : DDSCAPS_SYSTEMMEMORY DDSCAPS_ZBUFFER trace:ddraw:DDRAW_dump_members - DDSD_HEIGHT : 600 trace:ddraw:DDRAW_dump_members - DDSD_WIDTH : 800 trace:ddraw:DDRAW_dump_members - DDSD_PIXELFORMAT : ( ) trace:ddraw:PixelFormat_DD2WineD3D Convert a DirectDraw Pixelformat to a WineD3D Pixelformat trace:ddraw:DDRAW_dump_pixelformat ( ) err:ddraw:PixelFormat_DD2WineD3D Unknown Pixelformat! (0) warn:ddraw:ddraw_surface_init Unsupported / unknown pixelformat. warn:ddraw:ddraw_create_surface Failed to initialize surface, hr 0x88760091. warn:ddraw:CreateSurface ddraw_create_surface failed, hr 0x88760091. err:seh:raise_exception Unhandled exception code c0000005 flags 0 addr 0xfffffffff

May 12, 2011

Re: Ubuntu likely to abandon HAL next cycle -- is this workable?

by Alexandre Julliard

Damjan Jovanovic <damjan.jov(a)gmail.com> writes: > Udisks has regressed from the portability of HAL to being Linux-only, > what are we going to do for BSDs/Solaris/others? Of course even if we add udisks we'll keep the HAL support around. It's necessary for backwards compatibility, which is something we take seriously, unlike Ubuntu. -- Alexandre Julliard julliard(a)winehq.org

May 12, 2011

Re: ddraw: Fix filtering of enumerated display modes (try 4)

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=10886 Your paranoid android. === W7PROX64 (64 bit ddrawmodes) === Timeout

May 12, 2011

Re: Ubuntu likely to abandon HAL next cycle -- is this workable?

by Damjan Jovanovic

On Thu, May 12, 2011 at 12:28 PM, Scott Ritchie <scott(a)open-vote.org> wrote: > Consensus here seems to be to try and remove HAL from the archive. > > Wine still uses it, so this means we'll either need to lobby for keeping > HAL or to migrate to udisks. Does udisks meet our needs? Will it work > for a future USB driver? Is someone working on the migration? > > Thanks, > Scott Ritchie Udisks has regressed from the portability of HAL to being Linux-only, what are we going to do for BSDs/Solaris/others? Damjan Jovanovic

May 12, 2011

Ubuntu likely to abandon HAL next cycle -- is this workable?

by Scott Ritchie

Consensus here seems to be to try and remove HAL from the archive. Wine still uses it, so this means we'll either need to lobby for keeping HAL or to migrate to udisks. Does udisks meet our needs? Will it work for a future USB driver? Is someone working on the migration? Thanks, Scott Ritchie

May 12, 2011

Re: ddraw: Fix filtering of enumerated display modes (try 3)

by Stefan Dösinger

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 This patch looks OK to me Am 11.05.2011 um 10:03 schrieb Ričardas Barkauskas: > try 3: Non standard bit mask test added. More surface description and > pixel format flags tested and DDSD_REFRESHRATE filtering and appearance > in callback surface description fixed. It affects filtering even when > DDEDM_REFRESHRATES is not set. > > try 2: Removed pixel format printing. Fixed email address > > Ričardas Barkauskas > REalm > <0001-ddraw-Fix-filtering-of-enumerated-display-modes.txt> -----BEGIN PGP SIGNATURE----- Version: GnuPG/MacGPG2 v2.0.17 (Darwin) iQIcBAEBAgAGBQJNy5voAAoJEN0/YqbEcdMwDK8P/iRh2fXmfGB+DV6zgHd1QY+6 JBTDZdLpSRIhdeM6KI+CYxbdClOkg5IEw7HUVldc8N8WnGoO28oeaf+XlaDVTwIR 66PPzC6NI/Q/301Gx34aO17VNQpnUs8Q6ITQU5kmEzlCP1cL0Tt0VCQQt95RETbv zxs2IW26QCwoHEv7g9cO8MNoU3UYOFVdln6EONOvpNRZDuAxU+NChF5OPMUJsCmJ xbkNTkCdrwf6kyHbm1MQewIVcgmZu/8b+enhL6mDpHtwiVOgD8Blw9u7rvdjlZse h7qb9X4zbsgNUUpvC4B6M1cK1LAi5M5LEI5dyAPzkWFUNV2CDwyhagnaAxv4VHkg KcYH9rzUN4EAe9ctnMH633Q7NMo870KboZOOyThQdRaXRXosRGsaFQ6tHqkqbNlp u47OdATqZrTKlDWsAC9CNBV8+NsLat6wK6sDhLVKgRTJuL2gTcePmHS+hzQ5aIxA 8A+2avxZRGlzaBW7tC7hKt/jYV7NHemWzEZNn6mduNL/PysmT+43881AthlSLDhD aegYN6e1XECPnDfTNDP/XaGaObiUZgLb8MbmRzOTJDMSKt06Hgf+PSjIW9LW4w1j bYz1QABAV1Wmso1iYEYTh1vMEeXq9TRm0m1+w+CV/PEVsRNwz2XLsNzfCjLYcgMq zZDO1Seyq/e3PiCmf9hU =KEpe -----END PGP SIGNATURE-----

May 12, 2011

Re: [1/2] mmsystem.dll16: fix MCI_STATUS mapping for digitalvideo

by Damjan Jovanovic

On Mon, May 9, 2011 at 6:35 PM, <Joerg-Cyril.Hoehle(a)t-systems.com> wrote: > Any of these steps means much more code than the simple^Histic > approach which has worked surprisingly well for the last 10 years: "if > flag 00020000 is set in OPEN, it must be MCI_*_OPEN_PARENT, as none of > the other 4 standard drivers (wave, MIDI, avi qtz, CDA) uses that bit." > Likewise MCI_DGV_STATUS_DISKSPACE Thank you for the detailed explanation. The problem then becomes that if a driver expects MCI_DGV_STATUS_PARMS, and we pass MCI_STATUS_PARMS because the request didn't looks like it's destined for a digitalvideo device, it might go and read/write a field that's outside the bounds of the allocated structure. But since it seems that the extra fields should generally only be used if special flags were also passed, and those flags would allow us to detect the device type and pass the right structure, I suppose the current heuristics are good enough. And now I can see why Microsoft abandoned further MCI development :-). > Regards, > Jörg Höhle > Regards Damjan Jovanovic

May 11, 2011

Re: dsound: return success in IDirectSoundBufferImpl_AcquireResources

by Chris Robinson

On Wednesday, May 11, 2011 6:40:42 AM Joerg-Cyril.Hoehle(a)t-systems.com wrote: > Several bugs were fixed by returning the success value that apps would get > on native, instead of returning the value that would be consistent with > the specification and implementation state in Wine, e.g. NOTSUPPORTED, > ERR_NOTIMPLEMENTED etc. That's not fixing bugs, that's hiding them. :) The bug is that the function is not implemented/supported, not that the function is being truthful with its return value, and simply changing the return value doesn't fix that. Hence the bug is not fixed. It actually kind of bothers me to see patches to "fix bugs" that only change the return value of a stub from error to success... returning success when the function doesn't work is itself a bug, and not any better than a hack, IMO. It can lead apps down a path that assume the function did what it was supposed to, when it really didn't, and trigger problems in unrelated code making it harder to track down the actual problem. Especially as Wine tends to implement stuff on an as-needed basis, this leaves many stubs empty. If returning a failure code causes a problem, that's a sign that the stub should be properly filled out enough to satisfy it.

May 11, 2011

winealsa.drv/midi.c: Adds the midi MTC Quarter Frame messages support.

by Joerg-Cyril.Hoehle＠t-systems.com

Hi, well, I know little about MIDI and have no MIDI HW, yet what I read about the F1 code is that it is indeed a quarter time "tick" and takes 2 bytes, so the 2nd part of Stéphane Bacri's patch from last month looks good to me. The first half + toSend = ((ev->data.control.value & 0x7f) << 8) | MIDI_CMD_COMMON_MTC_QUARTER; looks ok too (by analogy with the surrounding code). One would argue that the existing shifts in midi.c would benefit from named constants too, http://source.winehq.org/source/dlls/winealsa.drv/midi.c#L365 MIDI_CMD_COMMON_SONG_SELECT 0xf3 MIDI_CMD_COMMON_SONG_POS 0xf2 http://www.alsa-project.org/alsa-doc/alsa-lib/group___m_i_d_i___commands.ht… Regards, Jörg Höhle

May 11, 2011

dsound: return success in IDirectSoundBufferImpl_AcquireResources

by Joerg-Cyril.Hoehle＠t-systems.com

Austin English wrote: - FIXME("(%p,%08u,%u,%p): stub\n",This,dwFlags,dwEffectsCount,pdwResultCodes); + FIXME("(%p,%08u,%u,%p): stub, faking success!\n",This,dwFlags,dwEffectsCount,pdwResultCodes); This is superfluous :-) "stub" is generally synonym with faking success. One lesson about apps - I tend to forget it myself - is that often enough they don't check every result and choose an alternative control path when some call fails, so better return success even thought it's a lie. Several bugs were fixed by returning the success value that apps would get on native, instead of returning the value that would be consistent with the specification and implementation state in Wine, e.g. NOTSUPPORTED, ERR_NOTIMPLEMENTED etc. I don't like that, but it helps. Often enough, the stub must fill part of the output parameters to prevent a crash later on. This remembers me to look at bug #17893 (needs amstream stub) again. Regards, Jörg Höhle

May 11, 2011

configure looking for prelink

by Claudio Fontana

Hello all, I noticed that wine's configure script looks for prelink using AC_PATH_PROG(PRELINK, prelink, false, [/sbin /usr/sbin $PATH]) I'd suggest changing it to AC_PATH_PROG(PRELINK, prelink, false, [/sbin /usr/sbin /usr/local/sbin $PATH]) to cover the case where the prelink package is built from source and installed in the default prefix (/usr/local). Thanks, Claudio Fontana

May 11, 2011

Re: msvcrt: Don't pass NULL as __ms_va_list in printf implementation

by Eric Pouech

> > You could va_copy() it to a second va_list if you need to do it twice. > va_copy is C99. Are we ready to say that is now dependant to C89 ?. A+ -- Eric Pouech

May 11, 2011

Re: [PATCH] riched20: Fixed some lParams with keycodes (Coverity)

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=10861 Your paranoid android. === WINEBUILD (build) === Patch failed to apply

May 11, 2011

Re: msvcrt: Don't pass NULL as __ms_va_list in printf implementation

by Marcus Meissner

On Wed, May 11, 2011 at 12:20:51PM +0200, Piotr Caban wrote: > --- > dlls/msvcrt/printf.h | 2 +- > dlls/msvcrt/wcs.c | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) > > > diff --git a/dlls/msvcrt/printf.h b/dlls/msvcrt/printf.h > index 64d159e..53f35bc 100644 > --- a/dlls/msvcrt/printf.h > +++ b/dlls/msvcrt/printf.h > @@ -607,7 +607,7 @@ static int FUNC_NAME(create_positional_ctx)(void *args_ctx, const APICHAR *forma > int i, j; > > i = FUNC_NAME(pf_printf)(FUNC_NAME(puts_clbk_str), &puts_ctx, format, NULL, TRUE, FALSE, > - arg_clbk_type, args_ctx, NULL); > + arg_clbk_type, args_ctx, valist); You cannot use a valist twice. va_copy() before to a second one if you need the information twice. Ciao, Marcus

May 11, 2011

Re: msvcrt: Don't pass NULL as __ms_va_list in printf implementation

by Marcus Meissner

On Wed, May 11, 2011 at 12:20:51PM +0200, Piotr Caban wrote: > --- > dlls/msvcrt/printf.h | 2 +- > dlls/msvcrt/wcs.c | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) > > > diff --git a/dlls/msvcrt/printf.h b/dlls/msvcrt/printf.h > index 64d159e..53f35bc 100644 > --- a/dlls/msvcrt/printf.h > +++ b/dlls/msvcrt/printf.h > @@ -607,7 +607,7 @@ static int FUNC_NAME(create_positional_ctx)(void *args_ctx, const APICHAR *forma > int i, j; > > i = FUNC_NAME(pf_printf)(FUNC_NAME(puts_clbk_str), &puts_ctx, format, NULL, TRUE, FALSE, > - arg_clbk_type, args_ctx, NULL); > + arg_clbk_type, args_ctx, valist); You should not process a va_list twice. You could va_copy() it to a second va_list if you need to do it twice. Ciao, Marcus

May 11, 2011

Re: urlmon: Code clean up and make tests a bit stricker

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=10855 Your paranoid android. === WVISTAADM (32 bit protocol) === protocol.c:2855: Test failed: Abort failed: 800c0200 === W7PROX64 (32 bit protocol) === protocol.c:965: Test failed: unexpected call ReportResult protocol.c:970: Test failed: hrResult = 00000000, expected: 80004004 protocol.c:2683: Test failed: Read failed: 00000001 === W7PROX64 (64 bit protocol) === protocol.c:965: Test failed: unexpected call ReportResult protocol.c:970: Test failed: hrResult = 00000000, expected: 80004004 protocol.c:2683: Test failed: Read failed: 00000001

May 11, 2011

Re: [PATCH 1/3] gdi32/tests: Test for font selection by full name.

by Grazvydas Ignotas

On Wed, May 11, 2011 at 12:08 AM, Grazvydas Ignotas <notasas(a)gmail.com> wrote: > To ensure correct font is selected, look in font name table directly. > --- > dlls/gdi32/tests/font.c | 143 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 143 insertions(+), 0 deletions(-) I unintentionally sent these patches twice, sorry about that. Please disregard the second copy. Gražvydas

May 11, 2011

Re: ddraw: Fix filtering of enumerated display modes (try 3)

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=10851 Your paranoid android. === W7PROX64 (64 bit ddrawmodes) === Timeout

May 11, 2011

Re: __ms_va_list and NULL

by Alexandre Julliard

Piotr Caban <piotr(a)codeweavers.com> writes: > On 05/10/11 20:02, André Hentschel wrote: >> Hi Piotr and folks, >> Most ARM crosscompilers (can't test with a native ARM one right now) don't like using NULL for a va_list and i'm not the only one seeing this issue. >> Piotr, i think that's your code, so maybe you can tell if it needs to be tuned or if that is a bug in the compiler (as the normal x86(_64) gcc works here). >> I hacked around that issue and attached the patch for documentation purpose, at least it compiles fine, don't know if it runs. > I don't know if it's a compiler fault or the code is faulty. The code is faulty. You either need to pass a proper va_list, or make the function take a pointer instead. -- Alexandre Julliard julliard(a)winehq.org

May 10, 2011

Re: __ms_va_list and NULL

by Piotr Caban

On 05/10/11 20:02, André Hentschel wrote: > Hi Piotr and folks, > Most ARM crosscompilers (can't test with a native ARM one right now) don't like using NULL for a va_list and i'm not the only one seeing this issue. > Piotr, i think that's your code, so maybe you can tell if it needs to be tuned or if that is a bug in the compiler (as the normal x86(_64) gcc works here). > I hacked around that issue and attached the patch for documentation purpose, at least it compiles fine, don't know if it runs. I don't know if it's a compiler fault or the code is faulty. Adding empty __ms_va_lists looks suspicious. In this case there's always proper valist and you can pass (no need to create new). It will be ignored in the same way NULL is.

May 10, 2011

Re: [2/2] usp10/tests: test ScriptItemizeOpenType

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=10831 Your paranoid android. === WNT4WSSP6 (32 bit usp10) === usp10.c:1445: Test failed: ScriptStringXtoCP should return iTrailing = 1 not 0 for X = 73 usp10.c:1411: Test failed: Leading values should be after trialing for rtl chracters(6) usp10.c:1423: Test failed: ScriptStringXtoCP should return Ch = 6 not 5 for X = 87 usp10.c:1424: Test failed: ScriptStringXtoCP should return iTrailing = 0 not 1 for X = 71 usp10.c:1432: Test failed: ScriptStringXtoCP should return Ch = 6 not 7 for X = 87 usp10.c:1433: Test failed: ScriptStringXtoCP should return iTrailing = 1 not 0 for X = 88 usp10.c:1443: Test failed: ScriptStringXtoCP should return Ch = 7 not 6 for X = 87 usp10.c:1459: Test failed: ScriptStringXtoCP should return Ch = 5 not 6 for X = 87 usp10.c:1461: Test failed: ScriptStringXtoCP should return iTrailing = 0 not 1 for X = 86 === W2KPROSP4 (32 bit usp10) === usp10.c:1445: Test failed: ScriptStringXtoCP should return iTrailing = 1 not 0 for X = 67 usp10.c:1411: Test failed: Leading values should be after trialing for rtl chracters(6) usp10.c:1423: Test failed: ScriptStringXtoCP should return Ch = 6 not 5 for X = 81 usp10.c:1424: Test failed: ScriptStringXtoCP should return iTrailing = 0 not 1 for X = 65 usp10.c:1432: Test failed: ScriptStringXtoCP should return Ch = 6 not 7 for X = 81 usp10.c:1433: Test failed: ScriptStringXtoCP should return iTrailing = 1 not 0 for X = 82 usp10.c:1443: Test failed: ScriptStringXtoCP should return Ch = 7 not 6 for X = 81 usp10.c:1459: Test failed: ScriptStringXtoCP should return Ch = 5 not 6 for X = 81 usp10.c:1461: Test failed: ScriptStringXtoCP should return iTrailing = 0 not 1 for X = 80 === WXPPROSP3 (32 bit usp10) === usp10.c:1445: Test failed: ScriptStringXtoCP should return iTrailing = 1 not 0 for X = 67 usp10.c:1411: Test failed: Leading values should be after trialing for rtl chracters(6) usp10.c:1423: Test failed: ScriptStringXtoCP should return Ch = 6 not 5 for X = 81 usp10.c:1424: Test failed: ScriptStringXtoCP should return iTrailing = 0 not 1 for X = 65 usp10.c:1432: Test failed: ScriptStringXtoCP should return Ch = 6 not 7 for X = 81 usp10.c:1433: Test failed: ScriptStringXtoCP should return iTrailing = 1 not 0 for X = 82 usp10.c:1443: Test failed: ScriptStringXtoCP should return Ch = 7 not 6 for X = 81 usp10.c:1459: Test failed: ScriptStringXtoCP should return Ch = 5 not 6 for X = 81 usp10.c:1461: Test failed: ScriptStringXtoCP should return iTrailing = 0 not 1 for X = 80 === W2K3R2SESP2 (32 bit usp10) === usp10.c:1445: Test failed: ScriptStringXtoCP should return iTrailing = 1 not 0 for X = 67 usp10.c:1411: Test failed: Leading values should be after trialing for rtl chracters(6) usp10.c:1423: Test failed: ScriptStringXtoCP should return Ch = 6 not 5 for X = 81 usp10.c:1424: Test failed: ScriptStringXtoCP should return iTrailing = 0 not 1 for X = 65 usp10.c:1432: Test failed: ScriptStringXtoCP should return Ch = 6 not 7 for X = 81 usp10.c:1433: Test failed: ScriptStringXtoCP should return iTrailing = 1 not 0 for X = 82 usp10.c:1443: Test failed: ScriptStringXtoCP should return Ch = 7 not 6 for X = 81 usp10.c:1459: Test failed: ScriptStringXtoCP should return Ch = 5 not 6 for X = 81 usp10.c:1461: Test failed: ScriptStringXtoCP should return iTrailing = 0 not 1 for X = 80 === WVISTAADM (32 bit usp10) === usp10.c:1445: Test failed: ScriptStringXtoCP should return iTrailing = 1 not 0 for X = 67 usp10.c:1411: Test failed: Leading values should be after trialing for rtl chracters(6) usp10.c:1423: Test failed: ScriptStringXtoCP should return Ch = 6 not 5 for X = 81 usp10.c:1424: Test failed: ScriptStringXtoCP should return iTrailing = 0 not 1 for X = 65 usp10.c:1432: Test failed: ScriptStringXtoCP should return Ch = 6 not 7 for X = 81 usp10.c:1433: Test failed: ScriptStringXtoCP should return iTrailing = 1 not 0 for X = 82 usp10.c:1443: Test failed: ScriptStringXtoCP should return Ch = 7 not 6 for X = 81 usp10.c:1459: Test failed: ScriptStringXtoCP should return Ch = 5 not 6 for X = 81 usp10.c:1461: Test failed: ScriptStringXtoCP should return iTrailing = 0 not 1 for X = 80 === W2K8SE (32 bit usp10) === usp10.c:1445: Test failed: ScriptStringXtoCP should return iTrailing = 1 not 0 for X = 67 usp10.c:1411: Test failed: Leading values should be after trialing for rtl chracters(6) usp10.c:1423: Test failed: ScriptStringXtoCP should return Ch = 6 not 5 for X = 81 usp10.c:1424: Test failed: ScriptStringXtoCP should return iTrailing = 0 not 1 for X = 65 usp10.c:1432: Test failed: ScriptStringXtoCP should return Ch = 6 not 7 for X = 81 usp10.c:1433: Test failed: ScriptStringXtoCP should return iTrailing = 1 not 0 for X = 82 usp10.c:1443: Test failed: ScriptStringXtoCP should return Ch = 7 not 6 for X = 81 usp10.c:1459: Test failed: ScriptStringXtoCP should return Ch = 5 not 6 for X = 81 usp10.c:1461: Test failed: ScriptStringXtoCP should return iTrailing = 0 not 1 for X = 80 === W7PRO (32 bit usp10) === usp10.c:1445: Test failed: ScriptStringXtoCP should return iTrailing = 1 not 0 for X = 67 usp10.c:1411: Test failed: Leading values should be after trialing for rtl chracters(6) usp10.c:1423: Test failed: ScriptStringXtoCP should return Ch = 6 not 5 for X = 81 usp10.c:1424: Test failed: ScriptStringXtoCP should return iTrailing = 0 not 1 for X = 65 usp10.c:1432: Test failed: ScriptStringXtoCP should return Ch = 6 not 7 for X = 81 usp10.c:1433: Test failed: ScriptStringXtoCP should return iTrailing = 1 not 0 for X = 82 usp10.c:1443: Test failed: ScriptStringXtoCP should return Ch = 7 not 6 for X = 81 usp10.c:1459: Test failed: ScriptStringXtoCP should return Ch = 5 not 6 for X = 81 usp10.c:1461: Test failed: ScriptStringXtoCP should return iTrailing = 0 not 1 for X = 80 === W7PROX64 (32 bit usp10) === usp10.c:1445: Test failed: ScriptStringXtoCP should return iTrailing = 1 not 0 for X = 67 usp10.c:1411: Test failed: Leading values should be after trialing for rtl chracters(6) usp10.c:1423: Test failed: ScriptStringXtoCP should return Ch = 6 not 5 for X = 81 usp10.c:1424: Test failed: ScriptStringXtoCP should return iTrailing = 0 not 1 for X = 65 usp10.c:1432: Test failed: ScriptStringXtoCP should return Ch = 6 not 7 for X = 81 usp10.c:1433: Test failed: ScriptStringXtoCP should return iTrailing = 1 not 0 for X = 82 usp10.c:1443: Test failed: ScriptStringXtoCP should return Ch = 7 not 6 for X = 81 usp10.c:1459: Test failed: ScriptStringXtoCP should return Ch = 5 not 6 for X = 81 usp10.c:1461: Test failed: ScriptStringXtoCP should return iTrailing = 0 not 1 for X = 80

May 10, 2011

__ms_va_list and NULL

by André Hentschel

Hi Piotr and folks, Most ARM crosscompilers (can't test with a native ARM one right now) don't like using NULL for a va_list and i'm not the only one seeing this issue. Piotr, i think that's your code, so maybe you can tell if it needs to be tuned or if that is a bug in the compiler (as the normal x86(_64) gcc works here). I hacked around that issue and attached the patch for documentation purpose, at least it compiles fine, don't know if it runs. without that patch: arm-linux-gnueabi-gcc -c -I. -I. -I../../include -I../../include -D__WINESRC__ -D_MT -D_REENTRANT -fPIC -Wall -pipe -fno-strength-reduce -fno-strict-aliasing -Wdeclaration-after-statement -Wempty-body -Wstrict-prototypes -Wtype-limits -Wwrite-strings -Wpointer-arith -g -O2 -o wcs.o wcs.c In file included from wcs.c:38: printf.h: In function ‘create_positional_ctx_a’: printf.h:610: error: incompatible type for argument 9 of ‘pf_printf_a’ printf.h:338: note: expected ‘va_list’ but argument is of type ‘void *’ In file included from wcs.c:40: printf.h: In function ‘create_positional_ctx_w’: printf.h:610: error: incompatible type for argument 9 of ‘pf_printf_w’ printf.h:338: note: expected ‘va_list’ but argument is of type ‘void *’ wcs.c: In function ‘MSVCRT_vsprintf_p_l’: wcs.c:886: error: incompatible type for argument 9 of ‘pf_printf_a’ printf.h:338: note: expected ‘va_list’ but argument is of type ‘void *’ wcs.c: In function ‘MSVCRT_vswprintf_p_l’: wcs.c:940: error: incompatible type for argument 9 of ‘pf_printf_w’ printf.h:338: note: expected ‘va_list’ but argument is of type ‘void *’ make: *** [wcs.o] Fehler 1 -- diff --git a/dlls/msvcrt/printf.h b/dlls/msvcrt/printf.h index 64d159e..4267133 100644 --- a/dlls/msvcrt/printf.h +++ b/dlls/msvcrt/printf.h @@ -605,9 +605,10 @@ static int FUNC_NAME(create_positional_ctx)(void *args_ctx, const APICHAR *forma struct FUNC_NAME(_str_ctx) puts_ctx = {INT_MAX, NULL}; printf_arg *args = args_ctx; int i, j; + __ms_va_list empty = {0}; i = FUNC_NAME(pf_printf)(FUNC_NAME(puts_clbk_str), &puts_ctx, format, NULL, TRUE, FALSE, - arg_clbk_type, args_ctx, NULL); + arg_clbk_type, args_ctx, empty); if(i < 0) return i; diff --git a/dlls/msvcrt/wcs.c b/dlls/msvcrt/wcs.c index 2f2c140..f60cf75 100644 --- a/dlls/msvcrt/wcs.c +++ b/dlls/msvcrt/wcs.c @@ -870,6 +870,7 @@ int CDECL MSVCRT_vsprintf_p_l(char *buffer, MSVCRT_size_t length, const char *fo printf_arg args_ctx[MSVCRT__ARGMAX+1]; struct _str_ctx_a puts_ctx = {length, buffer}; int ret; + __ms_va_list empty = {0}; memset(args_ctx, 0, sizeof(args_ctx)); @@ -883,7 +884,7 @@ int CDECL MSVCRT_vsprintf_p_l(char *buffer, MSVCRT_size_t length, const char *fo arg_clbk_valist, NULL, args); else ret = pf_printf_a(puts_clbk_str_a, &puts_ctx, format, locale, TRUE, TRUE, - arg_clbk_positional, args_ctx, NULL); + arg_clbk_positional, args_ctx, empty); puts_clbk_str_a(&puts_ctx, 1, &nullbyte); return ret; @@ -924,6 +925,7 @@ int CDECL MSVCRT_vswprintf_p_l(MSVCRT_wchar_t *buffer, MSVCRT_size_t length, printf_arg args_ctx[MSVCRT__ARGMAX+1]; struct _str_ctx_w puts_ctx = {length, buffer}; int ret; + __ms_va_list empty = {0}; memset(args_ctx, 0, sizeof(args_ctx)); @@ -937,7 +939,7 @@ int CDECL MSVCRT_vswprintf_p_l(MSVCRT_wchar_t *buffer, MSVCRT_size_t length, arg_clbk_valist, NULL, args); else ret = pf_printf_w(puts_clbk_str_w, &puts_ctx, format, locale, TRUE, TRUE, - arg_clbk_positional, args_ctx, NULL); + arg_clbk_positional, args_ctx, empty); puts_clbk_str_w(&puts_ctx, 1, &nullbyte); return ret; -- Best Regards, André Hentschel

May 10, 2011

Re: [PATCH 1/2] mountmgr: Populate HKLM\HARDWARE\DEVICEMAP\Scsi here instead of in kernel32. (try 6)

by Alexandre Julliard

Charles Davis <cdavis(a)mymail.mines.edu> writes: >> Now it's not setting DeviceName at all here. > As far as I can tell, this is a limitation of the way mountmgr works. It > only assigns a device name when a disk is in the drive. I take it you > want me to fix that? Ultimately we'll want to create the devices even without a disk in the drive, but that can be a separate patch. For now setting the name only when it's mounted would be good enough. -- Alexandre Julliard julliard(a)winehq.org

May 10, 2011

Re: [3/5] d3dx9: Implement D3DXMESHOPT_ATTRSORT|D3DXMESHOPT_IGNOREVERTS. (try 2)

by Alexandre Julliard

Dylan Smith <dylan.ah.smith(a)gmail.com> writes: > static HRESULT WINAPI ID3DXMeshImpl_OptimizeInplace(ID3DXMesh *iface, DWORD flags, CONST DWORD *adjacency_in, DWORD *adjacency_out, > DWORD *face_remap_out, LPD3DXBUFFER *vertex_remap_out) > { That function clearly needs to be split into several helpers. -- Alexandre Julliard julliard(a)winehq.org

May 10, 2011

Re: [PATCH 1/2] mountmgr: Populate HKLM\HARDWARE\DEVICEMAP\Scsi here instead of in kernel32. (try 6)

by Charles Davis

On 5/10/11 11:11 AM, Alexandre Julliard wrote: > Charles Davis <cdavis(a)mymail.mines.edu> writes: > >> Try 6: Fix some problems noticed by Vitaliy Margolen. >> Try 5: Get the value of "DeviceName" from mountmgr instead of making one up. >> Try 4: Fix unused variable warning. >> Try 3: Add support for IDE drives. >> Try 2: Don't depend on the SCSI generic driver on Linux. >> >> --- >> dlls/kernel32/Makefile.in | 1 - >> dlls/kernel32/oldconfig.c | 404 ------------------------------------------ >> dlls/kernel32/process.c | 3 - >> dlls/mountmgr.sys/device.c | 192 ++++++++++++++++++++- >> dlls/mountmgr.sys/diskarb.c | 2 +- >> dlls/mountmgr.sys/hal.c | 207 +++++++++++++++++++++- >> dlls/mountmgr.sys/mountmgr.c | 6 +- >> dlls/mountmgr.sys/mountmgr.h | 37 ++++- >> 8 files changed, 439 insertions(+), 413 deletions(-) >> delete mode 100644 dlls/kernel32/oldconfig.c > > Now it's not setting DeviceName at all here. As far as I can tell, this is a limitation of the way mountmgr works. It only assigns a device name when a disk is in the drive. I take it you want me to fix that? Chip

May 10, 2011

Re: [PATCH 1/2] mountmgr: Populate HKLM\HARDWARE\DEVICEMAP\Scsi here instead of in kernel32. (try 6)

by Alexandre Julliard

Charles Davis <cdavis(a)mymail.mines.edu> writes: > Try 6: Fix some problems noticed by Vitaliy Margolen. > Try 5: Get the value of "DeviceName" from mountmgr instead of making one up. > Try 4: Fix unused variable warning. > Try 3: Add support for IDE drives. > Try 2: Don't depend on the SCSI generic driver on Linux. > > --- > dlls/kernel32/Makefile.in | 1 - > dlls/kernel32/oldconfig.c | 404 ------------------------------------------ > dlls/kernel32/process.c | 3 - > dlls/mountmgr.sys/device.c | 192 ++++++++++++++++++++- > dlls/mountmgr.sys/diskarb.c | 2 +- > dlls/mountmgr.sys/hal.c | 207 +++++++++++++++++++++- > dlls/mountmgr.sys/mountmgr.c | 6 +- > dlls/mountmgr.sys/mountmgr.h | 37 ++++- > 8 files changed, 439 insertions(+), 413 deletions(-) > delete mode 100644 dlls/kernel32/oldconfig.c Now it's not setting DeviceName at all here. -- Alexandre Julliard julliard(a)winehq.org

May 10, 2011

Re: gdi32: Handle pre-Unicode symbol TTFs with symbols at U+00XX.

by Alexandre Julliard

"Konstantin L. Metlov" <metlov(a)fti.dn.ua> writes: > Commit 834378183ad2dffa84fab63b080c31241ac33cba handles the case when glyph is in the U+F0XX range differently > from the original patch. The commit (as I've just tested) does not fix the problem with special symbols in > KOMPAS 3D CAD, which original patch fixed. With this patch I propose to revert the changes of commit > 834378183ad2dffa84fab63b080c31241ac33cba and apply the original patch instead. The original patch was ugly. There's no reason to test for all the conditions twice. If my version doesn't fix your issue please address that without reintroducing the previous ugliness. -- Alexandre Julliard julliard(a)winehq.org

May 10, 2011

Re: ddraw: Fix filtering of enumerated display modes by bits per pixel

by Stefan Dösinger

>> Please add an additional test that passes in non-null flags that >don't match >> standard R5G6B5. > >I'm not exactly sure what you mean with this. The other test has mask >of >R5G5B5. The one that's missing is R5G6B5, but I don't think it's >needed. E.g. a red mask of 0xf0f0, green 0x0f00 and blue 0x000f. The purpose is to find out if windows listens to non-zero masks. If it does you'll get 0 formats reported. If it doesn't you'll get the usual count. -- Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail gesendet.

May 10, 2011

Re: [PATCH try6] wininet: Added support for persistent HTTP connections

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=10825 Your paranoid android. === W7PROX64 (64 bit protocol) === protocol.c:972: Test failed: unexpected call ReportResult protocol.c:977: Test failed: hrResult = 00000000, expected: 80004004 protocol.c:2690: Test failed: Read failed: 00000001

May 10, 2011

Re: comctl32/listview: Make edit box handle available from LVN_ENDLABELEDIT notification

by Alexandre Julliard

Vitaliy Margolen <wine-devel(a)kievinfo.com> writes: > I'm guessing AJ doesn't care much about such details. So NVM. > > Anyone want's to remove any requirements to wine tests? As long as > they compile and pass on AJ's pc, is seems to be the only requirement. The tests are supposed to succeed. Writing fancy failure messages is a waste of time, since they are not supposed to be seen anyway. Printing the failing value is useful, but printing "foo was expected instead" is generally useless. And about window handles, IsWindow() is just fine, handles don't get reused the way you think. Learn how things work before flaming others. -- Alexandre Julliard julliard(a)winehq.org

May 10, 2011

Re: ddraw: Fix filtering of enumerated display modes by bits per pixel

by Ričardas Barkauskas

On 05/09/2011 10:11 PM, Stefan Dösinger wrote: > On Monday 09 May 2011 20:55:21 Ričardas Barkauskas wrote: >> + modes16bpp_cnt = 0; >> + U2(ddsd.ddpfPixelFormat).dwRBitMask = 0x0000; >> + U3(ddsd.ddpfPixelFormat).dwGBitMask = 0x0000; >> + U4(ddsd.ddpfPixelFormat).dwBBitMask = 0x0000; > Please add an additional test that passes in non-null flags that don't match > standard R5G6B5. I'm not exactly sure what you mean with this. The other test has mask of R5G5B5. The one that's missing is R5G6B5, but I don't think it's needed. >> + if ((DDSD->dwFlags & DDSD_PIXELFORMAT) && (DDSD- >> u4.ddpfPixelFormat.dwFlags & DDPF_RGB) ) >> + if(pixelformat.u1.dwRGBBitCount != DDSD- >> u4.ddpfPixelFormat.u1.dwRGBBitCount) continue; > For the fun of it you could see what happens when you pass a non-RGB > format(e.g. a DDPF_YUV or DDPF_BUMPDUDV one) to EnumPixelFormat, but that's > not really a requirement for me. Afaics the DDPF_RGB check is needed anyway to > enumerate 8 bit palettized formats properly. > > A YUV primary format may even make sense. A BUMPDUDV one not really. My bad. Didn't think of testing this before. Now it is to strict. Ricardas Barkauskas REalm

May 10, 2011

Re: kernel32: Implement CreateSymbolicLinkA/W by reusing a modified IoCreateSymbolicLink

by Vitaliy Margolen

On 05/09/2011 04:02 PM, André Hentschel wrote: > Also for http://bugs.winehq.org/show_bug.cgi?id=27108 I'm afraid that according to msdn kernel32's CreateSymbolicLink has absolutely nothing to do with ntdll's NtCreateSymbolicLinkObject. Other then similar name. The kernel32's creates file system symlinks (for files) and reparse points (for directories). Ntdll's NtCreateSymbolicLinkObject creates object manager's symlinks only. At leas in Wine they don't apply to file systems. Vitaliy.

May 9, 2011

Re: comctl32/listview: Make edit box handle available from LVN_ENDLABELEDIT notification

by Vitaliy Margolen

I'm guessing AJ doesn't care much about such details. So NVM. Anyone want's to remove any requirements to wine tests? As long as they compile and pass on AJ's pc, is seems to be the only requirement. Vitaliy. On 05/09/2011 08:42 AM, Nikolay Sivov wrote: > On 5/9/2011 17:39, Vitaliy Margolen wrote: >> On 05/09/2011 01:09 AM, Nikolay Sivov wrote: >>> Fix for http://bugs.winehq.org/show_bug.cgi?id=25908 >>> >> >> There are 3 separate changes in this patch: >>> - BOOL bSame; >>> + BOOL same; >> Variable name rename. > I don't think it's a big deal. >> >> The actual fix with new test. >>> + ok(g_editbox_disp_info.item.pszText != NULL, "got %p\n", >>> g_editbox_disp_info.item.pszText); >> Btw it's nice to print what did you expected back. > I don't expect NULL, that's all. I don't care about particular pointer > value. If you mean I should print that I expect NULL, I don't think so, it's > redundant. In case it fails you need to look at failed line anyway. >> >>> + ok(!IsWindow(hwndedit), "Expected Edit window to be freed\n"); >> I'm not so sure you can rely on system not allocating any more windows >> with the same handle. This is a race condition. > This duplicates previous test behaviour that's doing just fine for quite a > long time. Test is single threaded, and you should run test program in clean > environment, with no programs around best. So it's working fine in practice. >> >>> - /* WM_COMMAND with EN_KILLFOCUS isn't forwared to parent */ >>> + /* WM_COMMAND with EN_KILLFOCUS isn't forwarded to parent */ >> A typo fix. > And? Separate patch to fix a typo in test file comment? (same test function > I'm patching now even) >> >> Vitaliy. >> >> >> > > >

May 9, 2011

Re: kernel32: Implement CreateSymbolicLinkA/W by reusing a modified IoCreateSymbolicLink

by Juan Lang

Hi André, + NtCreateSymbolicLinkObject( &handle, STANDARD_RIGHTS_REQUIRED | 0x1 /* SYMBOLIC_LINK_ALL_ACCESS */, + &attr, &targetstr ); + RtlFreeUnicodeString(&linkstr); + RtlFreeUnicodeString(&targetstr); + /* FIXME: store handle somewhere */ + return handle != INVALID_HANDLE_VALUE; Your fixme already indicates you need to do something different: you're introducing a handle leak. Since the caller has no access to it, what's its use? Simply close it. Also, rather than test the handle value, NtCreateSymbolicLinkObject already has a more direct way to return success: its return value. I'd suggest you use that instead. Finally, any reason you don't call CreateSymbolicLinkW from CreateSymbolicLinkA? This is the usual way we implement things, and avoids having to fix two places if one turns out to be incorrect. Thanks, --Juan

May 9, 2011

Re: ddraw: Fix filtering of enumerated display modes by bits per pixel

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=10820 Your paranoid android. === W7PROX64 (64 bit ddrawmodes) === Timeout

May 9, 2011

Re: ddraw: Fix filtering of enumerated display modes by bits per pixel

by Stefan Dösinger

On Monday 09 May 2011 20:55:21 Ričardas Barkauskas wrote: > + modes16bpp_cnt = 0; > + U2(ddsd.ddpfPixelFormat).dwRBitMask = 0x0000; > + U3(ddsd.ddpfPixelFormat).dwGBitMask = 0x0000; > + U4(ddsd.ddpfPixelFormat).dwBBitMask = 0x0000; Please add an additional test that passes in non-null flags that don't match standard R5G6B5. > + if ((DDSD->dwFlags & DDSD_PIXELFORMAT) && (DDSD- >u4.ddpfPixelFormat.dwFlags & DDPF_RGB) ) > + if(pixelformat.u1.dwRGBBitCount != DDSD- >u4.ddpfPixelFormat.u1.dwRGBBitCount) continue; For the fun of it you could see what happens when you pass a non-RGB format(e.g. a DDPF_YUV or DDPF_BUMPDUDV one) to EnumPixelFormat, but that's not really a requirement for me. Afaics the DDPF_RGB check is needed anyway to enumerate 8 bit palettized formats properly. A YUV primary format may even make sense. A BUMPDUDV one not really.

May 9, 2011

[1/2] mmsystem.dll16: fix MCI_STATUS mapping for digitalvideo

by Joerg-Cyril.Hoehle＠t-systems.com

Hi, Damjan Jovanovic wrote: >What determines what subtype of MCI_STATUS_PARMS is passed to >mciSendCommand() for an MCI_STATUS message? I consider the string interface the defining property of MCI devices. This sets it apart from e.g. COM interfaces. Further I believe the string and command interfaces to be interchangeable. This has several consequences. A. The string interfaces only provides one return value. There's no need to provide other output variables since the string interface cannot return them. This answers >are you convinced no MCI driver ever modifies fields it shouldn't, and >no application depends on that exceptional behaviour? with yes. If you suspect a deviating behaviour, then by all means, write a test. For instance, I was surprised when my tests revealed that in some version of MS some of the MCI drivers modify dwRetSize (in INFO_PARMS). I read into VirtualAlloc() the other day to find out how to write-protect memory so as to detect whether some a priori read-only slot is written to. Of course, this does not prevent e.g. a root kit to hide as an MCI driver and write whatever into the PARMS structure. B. The MCI_*_PARMS and the MCI resource files must be congruent. The MCI parser does not know any PARMS structure. It uses the resource description to reconstruct the layout of the PARMS structure. C. Each device recognizes one structure per command. The structure is defined by the device's resource file (defaulting to winmm_res.rc). This answers >must [the driver] assume it got MCI_DGV_STATUS_PARMS only when it >deals with a DGV-specific status request like MCI_DGV_STATUS_DISKSPACE >and fall back to the generic MCI_STATUS_PARMS for all other cases? with no. Driver X can and must assume that it received a MCI_X_STATUS_PARMS. That's what it receives from the MCI parser. All MCI_*_STATUS_PARMS need not have their slots in the same place, but it would be unwise to deviate from the order of the core commands. Then you couldn't send an MCI_STATUS_MODE using the generic MCI_STATUS_PARMS to an unknown MCI device! Only "status x mode" would work. Wine's A/W and 16/32 MCI mappers depend on this common ordering. Note that the device type only selects the command set when defaulting to winmm_res.rc. A driver providing its own resource (like mciavi) can return any device type it sees fit -- I still wonder why mciavi returns type "digitalvideo" instead of "animation". In the end, the resource defines the layout and extra parameters that the driver purports to recognize. What I mean is that dispatching on devtype is no guarantee for correctness, rather than a good guess. These were the rules, now the exceptions. :-) - I believe MS goofed with mciavi/mciqtz32 because MCI_DGV_SETVIDEO_PARMS and mciavi_res.rc publish 4 slots - over, quality, algorithm, number - in non-matching order. Hence you cannot use the generic MCI parser to construct the layout in digitalv.h I never wrote a test to see whether there are some MS hacks to map one to the other, or whether the .h is incorrect. - Using the MCI string interface always involves an alias, whereas you can use the command interface without it. - The MCI_BREAK system command has an hwndBreak slot and associated MCI_BREAK_HWND flag that the string interface does not know about. - dwRetSize update mentioned above. After I discovered that, I had Wine implement the "straight" behaviour because the modification is not documented, not all drivers implement it across all versions, not all commands do it, native implements it bogusly for dwRetSize=0 and its value is unclear with W/A: so let's not write to it. I put a comment in the code to signal this irregularity. Does an application depend on the update? Who knows? Possible rewrites and future directions: - Several times I thought about passing a bUnicode flag within winmm's MCI parser to delay the mapping from A to W. Never written. - I thought about reusing the PARMS structure in-place, replacing string A with W pointers. This would eliminate the current hassle of not knowing the size of structure. All that one needs to know is the location of the string pointers. Never written. That's obviously not feasible with the 16/32 mapping due to differing offsets. Another, less fancy approach would be to parse the resource strings even for MCI commands so as to know the layout. It would slow down every command. But it would be correct. - One problem remains with OPEN. Basically, it looks like the MCI needs to preparse the "open ..." string to determine which device to open. Only when the device is known does winmm know which resource to use for parsing! E.g. mciwave accepts an extra buffer parameter. I don't believe native would do two passes. It may well be that native's parser is an implementation of the IBM patent I mentioned some month ago. Perhaps that helps solve the issue. - The OPEN issue affects the 16/32 mapper too. After all, MCI_ANIM_OPEN_PARMS contains a HWND beyond the common parameters... How to find out whether the OPEN will go to a device that uses extra slots? ... And how to fill these slots? The only solution path that I currently see is that mmsystem16 would duplicate some of winmm's job: send the 3 messages to load and open the driver to obtain the resource, then only call MCI_OPEN. Any of these steps means much more code than the simple^Histic approach which has worked surprisingly well for the last 10 years: "if flag 00020000 is set in OPEN, it must be MCI_*_OPEN_PARENT, as none of the other 4 standard drivers (wave, MIDI, avi qtz, CDA) uses that bit." Likewise MCI_DGV_STATUS_DISKSPACE Regards, Jörg Höhle

May 9, 2011

Re: [PATCH 1/2] mountmgr: Populate HKLM\HARDWARE\DEVICEMAP\Scsi here instead of in kernel32. (try 5)

by Charles Davis

On 5/9/11 8:36 AM, Vitaliy Margolen wrote: > On 05/09/2011 07:42 AM, Charles Davis wrote: >> Try 5: Get the value of "DeviceName" from mountmgr instead of making >> one up. >> Try 4: Fix unused variable warning. >> Try 3: Add support for IDE drives. >> Try 2: Don't depend on the SCSI generic driver on Linux. > >> - RtlCreateUnicodeStringFromAsciiz( &nameW, "FirstBusTimeScanInMs" ); >> + static const WCHAR bus_scan_timeW[] = >> {'F','i','r','s','t','B','u','s','S','c','a','n','T','i','m','e','I','n','M','s',0}; >> > These do not match. You're right, will fix. > >> >> + if (devtype) >> + { >> + if (!(type = p_libhal_device_get_property_string( ctx, udi, >> "storage.drive_type",&error ))) >> + *devtype = ~0; >> + else if (!strcmp( type, "disk" ) || !strcmp( type, "floppy" )) >> + *devtype = 0x00; >> + else if (!strcmp( type, "tape" )) >> + *devtype = 0x01; >> + else if (!strcmp( type, "cdrom" )) >> + *devtype = 0x05; >> + else if (!strcmp( type, "raid" )) >> + *devtype = 0x0C; >> + else >> + *devtype = ~0; >> + } > Please don't use magic numbers here. OK. > Also these numbers are wrong. From > winbase.h: > #define DRIVE_UNKNOWN 0 > #define DRIVE_NO_ROOT_DIR 1 > #define DRIVE_REMOVABLE 2 > #define DRIVE_FIXED 3 > #define DRIVE_REMOTE 4 > /* Win32 additions */ > #define DRIVE_CDROM 5 > #define DRIVE_RAMDISK 6 The numbers I used are actually the SCSI peripheral device type numbers. I use them in create_scsi_entry() to figure out which one of the "*Peripheral" strings to put in the registry. But yeah, I probably shouldn't use magic numbers like that. > > Missing "DeviceName"="Cdrom0" for cdroms. You only get a DeviceName if there's a disk in the drive (a limitation of the way mountmgr works). > > Also names of devices do not match. This can potentially break some > software: > -"Identifier"="ATA Hitachi HDS72202JKAO" > +"Identifier"="ATA Hitachi HDS72202" > -"Identifier"="ATA WDC WD7500AAKS-030.0" > +"Identifier"="ATA WDC WD7500AAKS-0" OK, I can fix these. > -"Identifier"="SAMSUNG DVDWBD SH-B083L SB01" > +"Identifier"="SAMSUNG DVDWBD SH-B083L" Can't fix that. HAL provides no way to query the SCSI revision of a SCSI device. Short of actually sending an INQUIRY SCSI command to the device, I don't know how to fix that. Thanks. Chip

May 9, 2011

Re: [PATCH try5] wininet: Added support for persistent HTTP connections

by Alexandre Julliard

Jacek Caban <jacek(a)codeweavers.com> writes: > + FreeLibrary(WININET_hModule); > + return ERROR_SUCCESS; It can still crash here. > +static void delayed_collect_connections(void) > +{ > + static const WCHAR wininet_dllW[] = {'w','i','n','i','n','e','t','.','d','l','l',0}; > + > + if(collector_running) > + return; > + > + if(!LoadLibraryW(wininet_dllW)) > + return; You probably don't want that inside the lock. Also GetModuleHandleEx is probably better. -- Alexandre Julliard julliard(a)winehq.org

May 9, 2011

Re: Support for D3DQUERY_TIMESTAMP* queries...

by Henri Verbeet

On 4 May 2011 20:38, Adam Martinson <amartinson(a)codeweavers.com> wrote: > OK, I think I addressed those issues, let me know how these look. > > +#define GL_TIME_ELAPSED_ARB 0x88BF > +#define GL_TIMESTAMP_ARB 0x8E28 Most of wined3d uses lowercase hex literals. > + type = wined3d_query_get_type(This->wineD3DQuery); > + if (type == D3DQUERYTYPE_TIMESTAMPDISJOINT) That's not a wined3d type. Also, this patch doesn't make sense on its own. > +static void test_timestamp_queries(IDirect3D9 *pD3d, HWND hwnd) Please don't use names like pD3d, pDevice, etc. There's also still a fair amount of inconsistencies in this patch. Patch 7/12 just adds dead code. It also seems to me the way of splitting the d3d9 and d3d10 disjoint queries is more complicated than needed. The difference can be handled completely in d3d10, since "ARB_TIMER_QUERY_FREQ" is just a constant that could easily just be part of the wined3d interface.

May 9, 2011

Re: comctl32/listview: Make edit box handle available from LVN_ENDLABELEDIT notification

by Nikolay Sivov

On 5/9/2011 17:39, Vitaliy Margolen wrote: > On 05/09/2011 01:09 AM, Nikolay Sivov wrote: >> Fix for http://bugs.winehq.org/show_bug.cgi?id=25908 >> > > There are 3 separate changes in this patch: >> - BOOL bSame; >> + BOOL same; > Variable name rename. I don't think it's a big deal. > > The actual fix with new test. >> + ok(g_editbox_disp_info.item.pszText != NULL, "got %p\n", >> g_editbox_disp_info.item.pszText); > Btw it's nice to print what did you expected back. I don't expect NULL, that's all. I don't care about particular pointer value. If you mean I should print that I expect NULL, I don't think so, it's redundant. In case it fails you need to look at failed line anyway. > >> + ok(!IsWindow(hwndedit), "Expected Edit window to be freed\n"); > I'm not so sure you can rely on system not allocating any more windows > with the same handle. This is a race condition. This duplicates previous test behaviour that's doing just fine for quite a long time. Test is single threaded, and you should run test program in clean environment, with no programs around best. So it's working fine in practice. > >> - /* WM_COMMAND with EN_KILLFOCUS isn't forwared to parent */ >> + /* WM_COMMAND with EN_KILLFOCUS isn't forwarded to parent */ > A typo fix. And? Separate patch to fix a typo in test file comment? (same test function I'm patching now even) > > Vitaliy. > > >

May 9, 2011

Re: [PATCH 1/2] mountmgr: Populate HKLM\HARDWARE\DEVICEMAP\Scsi here instead of in kernel32. (try 5)

by Vitaliy Margolen

On 05/09/2011 07:42 AM, Charles Davis wrote: > Try 5: Get the value of "DeviceName" from mountmgr instead of making one up. > Try 4: Fix unused variable warning. > Try 3: Add support for IDE drives. > Try 2: Don't depend on the SCSI generic driver on Linux. > - RtlCreateUnicodeStringFromAsciiz( &nameW, "FirstBusTimeScanInMs" ); > + static const WCHAR bus_scan_timeW[] = {'F','i','r','s','t','B','u','s','S','c','a','n','T','i','m','e','I','n','M','s',0}; These do not match. > > + if (devtype) > + { > + if (!(type = p_libhal_device_get_property_string( ctx, udi, "storage.drive_type",&error ))) > + *devtype = ~0; > + else if (!strcmp( type, "disk" ) || !strcmp( type, "floppy" )) > + *devtype = 0x00; > + else if (!strcmp( type, "tape" )) > + *devtype = 0x01; > + else if (!strcmp( type, "cdrom" )) > + *devtype = 0x05; > + else if (!strcmp( type, "raid" )) > + *devtype = 0x0C; > + else > + *devtype = ~0; > + } Please don't use magic numbers here. Also these numbers are wrong. From winbase.h: #define DRIVE_UNKNOWN 0 #define DRIVE_NO_ROOT_DIR 1 #define DRIVE_REMOVABLE 2 #define DRIVE_FIXED 3 #define DRIVE_REMOTE 4 /* Win32 additions */ #define DRIVE_CDROM 5 #define DRIVE_RAMDISK 6 Missing "DeviceName"="Cdrom0" for cdroms. Also names of devices do not match. This can potentially break some software: -"Identifier"="ATA Hitachi HDS72202JKAO" +"Identifier"="ATA Hitachi HDS72202" -"Identifier"="ATA WDC WD7500AAKS-030.0" +"Identifier"="ATA WDC WD7500AAKS-0" -"Identifier"="SAMSUNG DVDWBD SH-B083L SB01" +"Identifier"="SAMSUNG DVDWBD SH-B083L" Vitaliy.

May 9, 2011

Re: [PATCH try5] wininet: Added support for persistent HTTP connections

by Jacek Caban

On 05/09/11 13:13, Marvin wrote: > Hi, > > While running your changed tests on Windows, I think I found new failures. > Being a bot and all I'm not very good at pattern recognition, so I might be > wrong, but could you please double-check? > Full results can be found at > http://testbot.winehq.org/JobDetails.pl?Key=10813 > > Your paranoid android. > > > === W2K3R2SESP2 (32 bit protocol) === > protocol.c:972: Test failed: unexpected call ReportResult > protocol.c:977: Test failed: hrResult = 00000000, expected: 80004004 > protocol.c:2690: Test failed: Read failed: 00000001 It's unrelated to the patch. Jacek

May 9, 2011

Re: comctl32/listview: Make edit box handle available from LVN_ENDLABELEDIT notification

by Vitaliy Margolen

On 05/09/2011 01:09 AM, Nikolay Sivov wrote: > Fix for http://bugs.winehq.org/show_bug.cgi?id=25908 > There are 3 separate changes in this patch: > - BOOL bSame; > + BOOL same; Variable name rename. The actual fix with new test. > + ok(g_editbox_disp_info.item.pszText != NULL, "got %p\n", g_editbox_disp_info.item.pszText); Btw it's nice to print what did you expected back. > + ok(!IsWindow(hwndedit), "Expected Edit window to be freed\n"); I'm not so sure you can rely on system not allocating any more windows with the same handle. This is a race condition. > - /* WM_COMMAND with EN_KILLFOCUS isn't forwared to parent */ > + /* WM_COMMAND with EN_KILLFOCUS isn't forwarded to parent */ A typo fix. Vitaliy.

May 9, 2011

Re: [PATCH try5] wininet: Added support for persistent HTTP connections

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=10813 Your paranoid android. === W2K3R2SESP2 (32 bit protocol) === protocol.c:972: Test failed: unexpected call ReportResult protocol.c:977: Test failed: hrResult = 00000000, expected: 80004004 protocol.c:2690: Test failed: Read failed: 00000001

May 9, 2011

Re: wineoss.drv: Add mmdevapi driver.

by Andrew Eikum

On 05/07/2011 07:04 PM, Gerald Pfeifer wrote: > On Fri, 6 May 2011, Andrew Eikum wrote: >> If Wine is trying to build wineoss.drv, then you must have oss_sysinfo. >> If you have oss_sysinfo, then you have OSSv4. If you have OSSv4, then >> you should have all of the above symbols, but apparently you don't. So I >> suspect it's something amiss with your OSSv4 implementation, and that's >> where I would begin investigating. >> >> Does that help you diagnose the problem? > > Yes, thanks a lot! Based on yours and Farncois' input, I did some > research and found the following, among others > > http://manuals.opensound.com/developer/AFMT_S24_PACKED.html > > and consulting Google Code Search I find that also other applications > do something like the following here (mplayer, for example). > > Suggested patch below. Makes sense to me. Thanks. Do this patch and the float format patch together fix your build issues, Gerald? Andrew > > Gerald > > > ChangeLog: > Use AFMT_S24_LE instead of AFMT_S24_PACKED. > > diff --git a/dlls/wineoss.drv/mmdevdrv.c b/dlls/wineoss.drv/mmdevdrv.c > index ff21cd6..789cb1e 100644 > --- a/dlls/wineoss.drv/mmdevdrv.c > +++ b/dlls/wineoss.drv/mmdevdrv.c > @@ -500,7 +500,7 @@ static int get_oss_format(const WAVEFORMATEX *fmt) > case 16: > return AFMT_S16_LE; > case 24: > - return AFMT_S24_PACKED; > + return AFMT_S24_LE; > case 32: > return AFMT_S32_LE; > } > @@ -953,7 +953,7 @@ static HRESULT WINAPI AudioClient_GetMixFormat(IAudioClient *iface, > }else if(formats& AFMT_S32_LE){ > fmt->Format.wBitsPerSample = 32; > fmt->SubFormat = KSDATAFORMAT_SUBTYPE_PCM; > - }else if(formats& AFMT_S24_PACKED){ > + }else if(formats& AFMT_S24_LE){ > fmt->Format.wBitsPerSample = 24; > fmt->SubFormat = KSDATAFORMAT_SUBTYPE_PCM; > }else{

May 9, 2011

Re: wineoss.drv: Support platforms that do not feature AFMT_FLOAT.

by Andrew Eikum

On 05/07/2011 07:46 PM, Gerald Pfeifer wrote: > As we have seen from a related thread on wine-devel, FreeBSD's > implementation of OSS is slightly, though not a lot, different from > what current Linux distributions feature. > > This patch addresses one such case, AFMT_FLOAT which does not exist > on FreeBSD (yet?), and I'm sure other non-Linux systems. I guess we can add this workaround. I don't know how far down this road we should be willing to go, though. I would consider the FreeBSD implementation broken: it deviates from the spec as stated by the authors. So this seems to be their bug, not ours. How far should we bend over backwards to accommodate their bug? Anyways this is a tiny hassle, so I'm fine with it. Perhaps you should consider filing a FreeBSD bug? Thanks Gerald. Andrew > I created this diff with more context so that it becomes immediate that > this patch is harmless in that AFMT_FLOAT is handled as an error case > for system not providing it. > > Gerald > > --- > dlls/wineoss.drv/mmdevdrv.c | 4 ++++ > 1 files changed, 4 insertions(+), 0 deletions(-) > > diff --git a/dlls/wineoss.drv/mmdevdrv.c b/dlls/wineoss.drv/mmdevdrv.c > index 789cb1e..254a119 100644 > --- a/dlls/wineoss.drv/mmdevdrv.c > +++ b/dlls/wineoss.drv/mmdevdrv.c > @@ -504,25 +504,27 @@ static int get_oss_format(const WAVEFORMATEX *fmt) > case 32: > return AFMT_S32_LE; > } > return -1; > } > > if(fmt->wFormatTag == WAVE_FORMAT_IEEE_FLOAT || > (fmt->wFormatTag == WAVE_FORMAT_EXTENSIBLE&& > IsEqualGUID(&fmtex->SubFormat,&KSDATAFORMAT_SUBTYPE_IEEE_FLOAT))){ > if(fmt->wBitsPerSample != 32) > return -1; > > +#ifdef AFMT_FLOAT > return AFMT_FLOAT; > +#endif > } > > return -1; > } > > static WAVEFORMATEX *clone_format(const WAVEFORMATEX *fmt) > { > WAVEFORMATEX *ret; > size_t size; > > if(fmt->wFormatTag == WAVE_FORMAT_EXTENSIBLE) > size = sizeof(WAVEFORMATEXTENSIBLE); > @@ -935,27 +937,29 @@ static HRESULT WINAPI AudioClient_GetMixFormat(IAudioClient *iface, > > if(This->dataflow == eRender) > formats = This->ai.oformats; > else if(This->dataflow == eCapture) > formats = This->ai.iformats; > else > return E_UNEXPECTED; > > fmt->Format.wFormatTag = WAVE_FORMAT_EXTENSIBLE; > if(formats& AFMT_S16_LE){ > fmt->Format.wBitsPerSample = 16; > fmt->SubFormat = KSDATAFORMAT_SUBTYPE_PCM; > +#ifdef AFMT_FLOAT > }else if(formats& AFMT_FLOAT){ > fmt->Format.wBitsPerSample = 32; > fmt->SubFormat = KSDATAFORMAT_SUBTYPE_IEEE_FLOAT; > +#endif > }else if(formats& AFMT_U8){ > fmt->Format.wBitsPerSample = 8; > fmt->SubFormat = KSDATAFORMAT_SUBTYPE_PCM; > }else if(formats& AFMT_S32_LE){ > fmt->Format.wBitsPerSample = 32; > fmt->SubFormat = KSDATAFORMAT_SUBTYPE_PCM; > }else if(formats& AFMT_S24_LE){ > fmt->Format.wBitsPerSample = 24; > fmt->SubFormat = KSDATAFORMAT_SUBTYPE_PCM; > }else{ > ERR("Didn't recognize any available OSS formats: %x\n", formats); > return E_FAIL; > >

May 9, 2011

Re: audio: remove extraneous parentheses around equality comparisons

by Paul Chitescu

If you're at it can you move the variable at the right side of the comparison? if (((mr->msg_tosave + 1) % mr->ring_buffer_size) == mr->msg_toget)

May 9, 2011

Re: [PATCH] msi: Fixed check_flag_combo macro (Coverity)

by Hans Leidekker

On Mon, 2011-05-09 at 13:05 +0200, Marcus Meissner wrote: > > It doesn't work here: > > > > ../../../tools/runtest -q -P wine -M msi.dll -T ../../.. -p msi_test.exe.so action.c && touch action.ok > > action.c:5578: Test failed: Expected ERROR_SUCCESS, got 1627 > > action.c:5587: Test failed: Expected ERROR_FILE_NOT_FOUND, got 0 > > action.c:5599: Test failed: Expected ERROR_FILE_NOT_FOUND, got 0 > > action.c:5656: Test failed: key not created > > action.c:5660: Test failed: key not created > > action.c:5664: Test failed: key not created > > make: *** [action.ok] Error 6 > > I have the suspicion that my patch just uncovers another bug, either > in this specific check, or other code. :/ > > ... > trace:msi:ITERATE_RemoveEnvironmentString name L"=+-MSITESTVAR2" value L"1" > err:msi:env_parse_flags Invalid flags: 00000007 > ... > > Hans? You seem to have added it the remove environment checks in aaf5ff.. any idea? ITERATE_RemoveEnvironmentString already validates parsed flags, so I would move the check_flag_combo invocations from env_parse_flags to ITERATE_WriteEnvironmentString.

May 9, 2011

Re: [PATCH] msi: Fixed check_flag_combo macro (Coverity)

by Marcus Meissner

On Mon, May 09, 2011 at 12:07:39PM +0200, Alexandre Julliard wrote: > Marcus Meissner <meissner(a)suse.de> writes: > > > The macro was always FALSE, as > > x & y == ~y > > is always FALSE. x& y == y was clearly intended. > > CID 1448 > > It doesn't work here: > > ../../../tools/runtest -q -P wine -M msi.dll -T ../../.. -p msi_test.exe.so action.c && touch action.ok > action.c:5578: Test failed: Expected ERROR_SUCCESS, got 1627 > action.c:5587: Test failed: Expected ERROR_FILE_NOT_FOUND, got 0 > action.c:5599: Test failed: Expected ERROR_FILE_NOT_FOUND, got 0 > action.c:5656: Test failed: key not created > action.c:5660: Test failed: key not created > action.c:5664: Test failed: key not created > make: *** [action.ok] Error 6 I have the suspicion that my patch just uncovers another bug, either in this specific check, or other code. :/ ... trace:msi:ITERATE_RemoveEnvironmentString name L"=+-MSITESTVAR2" value L"1" err:msi:env_parse_flags Invalid flags: 00000007 ... Hans? You seem to have added it the remove environment checks in aaf5ff.. any idea? Ciao, Marcus

May 9, 2011