November 2009 - Wine-Devel - list.winehq.org

Re: [PATCH 02/16] comctl32: Add basic structure for IImageList interface

by Henri Verbeet

2009/11/10 Owen Rudge <orudge(a)codeweavers.com>: > +static HRESULT WINAPI ImageListImpl_Add(IImageList *iface, HBITMAP hbmImage, > + HBITMAP hbmMask, int *pi) > +{ > + FIXME("STUB: %p %p %p %p\n", iface, hbmImage, hbmMask, pi); > + return E_NOINTERFACE; > +} This is minor at this point, but E_NOTIMPL is usually a more appropriate return code for stubs.

Nov. 11, 2009

Re: What's the use of volatile in Wine code?

by Alexandre Julliard

<Joerg-Cyril.Hoehle(a)t-systems.com> writes: > So what's the use of volatile? When is it appropriate in Wine? In general it's never appropriate. There are some very specific cases where it can be used, but you have to be sure to know what you are doing. Using proper synchronization instead is strongly recommended. -- Alexandre Julliard julliard(a)winehq.org

Nov. 11, 2009

Re: [PATCH 01/16] comctl32: Add class factory for IImageList interface

by Alexandre Julliard

Owen Rudge <orudge(a)codeweavers.com> writes: > These patches finish the implementation of the IImageList v6 interface > in comctl32. This doesn't seem to match Windows, even independently from the structure layout issue. There's no DllGetClassObject or clsid registration on Windows, it's not handled as a standard COM dll. -- Alexandre Julliard julliard(a)winehq.org

Nov. 11, 2009

What's the use of volatile in Wine code?

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

Hi, AJ wrote in http://www.winehq.org/pipermail/wine-devel/2009-November/079575.html >If there is proper synchronization you don't need >volatile, and if there isn't volatile won't magically fix it. However, mcimidi has in its code since pre 1999: :/* it seems that in case of multi-threading, gcc is optimizing just a little bit : * too much. Tell gcc not to optimize status value using volatile. : */ :while (((volatile WINE_MCIMIDI*)wmm)->dwStatus == MCI_MODE_PAUSE); The comment is right. Any C compiler is allowed to optimize this into an endless loop without volatile. But I'd rather see the following: :volatile dwStatus = NOT_READY; /* one central declaration */ :... :while (wmm->dwStatus == PAUSE) ; /* what, busy wait!?! */ So what's the use of volatile? When is it appropriate in Wine? I found this article from MSDN about memory acquire and release semantics, volatile and multiple processors very interesting. MS-VC changed the code generator for 'volatile' between 2003 and 2005 to accomodate multi-processor systems: http://msdn.microsoft.com/en-us/library/ms686355(VS.85).aspx Regards, Jörg Höhle

Nov. 11, 2009

MCI and application-level thread-safety?

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

Hi, So far my my patches have dealt only with "internal thread consistency", i.e. to avoid bogus conditions that result by using threads *internal* to the MCI. I wonder whether the individual MCI devices must protect themselves from application-level threads, i.e. an application issuing commands to the same device from several threads? (Also needed for auto-close...) This for sure would require the addition of critical sections which currently only mciavi32 has. These have to be thought about very carefully to avoid dead-locks (give me Erlang or Occam, or queues like in the audio drivers instead of this lock-based error-prone stuff!). Regards, Jörg Höhle

Nov. 11, 2009

Where to receive MCI PostMessageW notifications?

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

Hi, the MCI relies on notification to implement auto-open correctly. When the command is done, it must send out a notification via PostMessageW. The receiver then ought to call mciClose. One possible implementation would be to have auto-open start a thread that does nothing else but wait for this message to arrive, then call mciClose. I wonder if there's something more lightweight than a thread (this is not Erlang) that could be used inside Wine. What could be used that reacts asynchronously to PostMessageW? Currently, the missing auto-open functionality causes - unclosed files (some seen as error messages when Wine exits) (here it is essential that the notification works 100%) - some application hangs, some of them in Bugzilla. Thanks, Jörg Höhle

Nov. 11, 2009

Re: [Fwd: 64bit wine]

by Michael Stefaniuc

Ben Klein wrote: > 2009/11/11 Michael Stefaniuc <mstefani(a)redhat.com>: >> For fedora it will not be possible to do a special 32bit compile on the x86_64 >> arch. > > Wait ... no gcc -m32 on Fedora x86_64? The compiler is not the problem but the missing i586 libraries+headers in the x86_64 build tree. >> Hence the only way is to have the default x86_32 version and a x86_64 >> version to be parallel installable. While it may be possible to use a different >> WINEPREFIX for the x86_64 version (which would have been my short term >> solution) it is not desirable for the users. > > Until WIne's WoW64 implementation works (which from what you said > before may be never), users will have to put up with separated 64bit Huh? Wine's WoW64 is well underway and is the feature that will make Wine reach the 1.2 milestone. > and 32bit Windows apps. The best solution is to use a separate > WINEPREFIX. The solution is for distributions to not ship Wine64 to the users yet. > Note that they would have the same issues if they were running Windows natively. We know that. I have forwarded the email basically as a follow-up to the WineConf 64bit session. We (as in Wine) needs to speed up the Wine64 documentation for our downstream packagers. I'll start it once i get a free round tuit. bye michael

Nov. 11, 2009

Re: Cpp Check Nov. 7th

by Alexandre Julliard

chris ahrendt <celticht32(a)yahoo.com> writes: > Alexandre Julliard wrote: >> chris ahrendt <celticht32(a)yahoo.com> writes: >> >>> Well its been a few since I ran CPP Check and so I ran it today and got the following: >>> >>> New CPP Check Errors for the Wine Code: >>> >>> [/home/cahrendt/wine-git/dlls/comctl32/imagelist.c:1908]: (error) Uninitialized variable: bmi_buf >>> [/home/cahrendt/wine-git/dlls/ntdll/server.c:802]: (error) Resource leak: fd >>> [/home/cahrendt/wine-git/dlls/ntdll/server.c:882]: (error) Resource leak: fd_cwd >>> [/home/cahrendt/wine-git/dlls/shell32/shellord.c:416]: (error) Uninitialized variable: szText >>> [/home/cahrendt/wine-git/dlls/shell32/shellord.c:416]: (error) Uninitialized variable: szTitle >>> [/home/cahrendt/wine-git/dlls/shell32/shellpath.c:1360]: (error) Uninitialized variable: InfoBuffer >>> [/home/cahrendt/wine-git/dlls/winex11.drv/xfont.c:2963]: (possible error) Resource leak: fd >>> [/home/cahrendt/wine-git/server/mapping.c:118]: (error) Uninitialized variable: page_size >> >> These are once again all false positives. I don't think there's any >> point in continuing to post such reports, at least not until cppcheck >> becomes more reliable. >> > Will report them to CPPcheck > > are you sure though all of these are false positives? Yes, there isn't a single legitimate bug in there. -- Alexandre Julliard julliard(a)winehq.org

Nov. 11, 2009

Re: [Fwd: 64bit wine]

by Marcus Meissner

On Wed, Nov 11, 2009 at 11:05:49AM +1100, Ben Klein wrote: > 2009/11/11 Michael Stefaniuc <mstefani(a)redhat.com>: > > For fedora it will not be possible to do a special 32bit compile on the x86_64 > > arch. > > Wait ... no gcc -m32 on Fedora x86_64? > > > Hence the only way is to have the default x86_32 version and a x86_64 > > version to be parallel installable. While it may be possible to use a different > > WINEPREFIX for the x86_64 version (which would have been my short term > > solution) it is not desirable for the users. > > Until WIne's WoW64 implementation works (which from what you said > before may be never), users will have to put up with separated 64bit > and 32bit Windows apps. The best solution is to use a separate > WINEPREFIX. > > Note that they would have the same issues if they were running Windows natively. On Wineconf we discussed how a mixed 64/32 wine will work. Largest part currently missing is the 64<->32 bit crosscall and doing 32/64 bit shared wineprefixes. So it is not yet ready to be setup and for regular users a 32bit only wine should be provided. Ciao, Marcus

Nov. 11, 2009

Re: [PATCH 02/16] comctl32: Add basic structure for IImageList interface

by Owen Rudge

> is wrong because it changes offsets... but it also breaks > those comments. (Can you put the vtable at the end?) In comctl32 v6, IImageList and HIMAGELIST are the same thing. See, for instance, http://msdn.microsoft.com/en-us/library/bb762185%28VS.85%29.aspx: "The IImageList pointer type, such as that returned in the ppv parameter, can be cast as an HIMAGELIST as needed; for example, for use in a list view. Conversely, an HIMAGELIST can be cast as a pointer to an IImageList." Obviously, the IImageList requires the vtable to be placed at the first offset. If any applications do rely on the (undocumented) imagelist structure, then these patches will indeed break them. Nikolay's solution would be a possibility, although it would complicate the code a bit. I would probably prefer to wait and see if we get reports of anything actually breaking, but I realise others may disagree there. Cheers, -- Owen Rudge http://www.owenrudge.net/

Nov. 11, 2009

re: [PATCH 02/16] comctl32: Add basic structure for IImageList interface

by Dan Kegel

As Nikolay pointed out, the change > /* the ones with offsets at the end are the same as in Windows */ > struct _IMAGELIST > { >+ const struct IImageListVtbl *lpVtbl;/* IImageList vtable */ >+ LONG ref; /* reference count */ >+ > DWORD magic; /* 00: 'SAMX' */ is wrong because it changes offsets... but it also breaks those comments. (Can you put the vtable at the end?)

Nov. 11, 2009

Re: [PATCH 02/16] comctl32: Add basic structure for IImageList interface

by Nikolay Sivov

On Wed, Nov 11, 2009 at 12:11 AM, Owen Rudge <orudge(a)codeweavers.com> wrote: > --- > dlls/comctl32/imagelist.c | 322 > ++++++++++++++++++++++++++++++++++++++++++++- > dlls/comctl32/imagelist.h | 3 + > 2 files changed, 319 insertions(+), 6 deletions(-) > After that: --- diff --git a/dlls/comctl32/imagelist.h b/dlls/comctl32/imagelist.h index 73ccd8a..51b8635 100644 --- a/dlls/comctl32/imagelist.h +++ b/dlls/comctl32/imagelist.h @@ -30,6 +30,9 @@ /* the ones with offsets at the end are the same as in Windows */ struct _IMAGELIST { + const struct IImageListVtbl *lpVtbl;/* IImageList vtable */ + LONG ref; /* reference count */ + DWORD magic; /* 00: 'SAMX' */ INT cCurImage; /* 04: ImageCount */ INT cMaxImage; /* 08: maximages */ --- let's pray that nobody depends on structure layout :). If it'll be a problem the possible solution could be to use two structure types - one for traditional API and one for com-based. Choosing one of it could be done possibly in ImageList_Create checking for manifest enabled in activation context. All traditional calls should check for that then and apply an offset to pointer to get HIMAGELIST without vtbl. We'll see...

Nov. 11, 2009

Re: Cpp Check Nov. 7th

by chris ahrendt

Alexandre Julliard wrote: > chris ahrendt <celticht32(a)yahoo.com> writes: > >> Well its been a few since I ran CPP Check and so I ran it today and got the following: >> >> New CPP Check Errors for the Wine Code: >> >> [/home/cahrendt/wine-git/dlls/comctl32/imagelist.c:1908]: (error) Uninitialized variable: bmi_buf >> [/home/cahrendt/wine-git/dlls/ntdll/server.c:802]: (error) Resource leak: fd >> [/home/cahrendt/wine-git/dlls/ntdll/server.c:882]: (error) Resource leak: fd_cwd >> [/home/cahrendt/wine-git/dlls/shell32/shellord.c:416]: (error) Uninitialized variable: szText >> [/home/cahrendt/wine-git/dlls/shell32/shellord.c:416]: (error) Uninitialized variable: szTitle >> [/home/cahrendt/wine-git/dlls/shell32/shellpath.c:1360]: (error) Uninitialized variable: InfoBuffer >> [/home/cahrendt/wine-git/dlls/winex11.drv/xfont.c:2963]: (possible error) Resource leak: fd >> [/home/cahrendt/wine-git/server/mapping.c:118]: (error) Uninitialized variable: page_size > > These are once again all false positives. I don't think there's any > point in continuing to post such reports, at least not until cppcheck > becomes more reliable. > Will report them to CPPcheck are you sure though all of these are false positives? chris

Nov. 11, 2009

Re: [Fwd: 64bit wine]

by Ben Klein

2009/11/11 Michael Stefaniuc <mstefani(a)redhat.com>: > For fedora it will not be possible to do a special 32bit compile on the x86_64 > arch. Wait ... no gcc -m32 on Fedora x86_64? > Hence the only way is to have the default x86_32 version and a x86_64 > version to be parallel installable. While it may be possible to use a different > WINEPREFIX for the x86_64 version (which would have been my short term > solution) it is not desirable for the users. Until WIne's WoW64 implementation works (which from what you said before may be never), users will have to put up with separated 64bit and 32bit Windows apps. The best solution is to use a separate WINEPREFIX. Note that they would have the same issues if they were running Windows natively.

Nov. 11, 2009

Re: Wine 64-bit in Fedora ...

by Ben Klein

2009/11/10 Michael Stefaniuc <mstefani(a)redhat.com>: > Uwe Bonnes wrote: >> Did nobody of the packages test what they did? > It is easy to miss as it requires: > - Fresh install of Wine (via PackageKit or yum install) and > - Old 32-bit WINEPREFIX. Except that it's obvious that the packagers are not people who USE wine, or they would notice their ia32 Windows apps suddenly not running.

Nov. 11, 2009

Re: avifile bugfixes

by Detlef Riekenberg

On Di, 2009-11-10 at 00:43 +0100, Julius Schwartzenberg wrote: > I will add tests to show this and submit the patches then. I created a test for avifil32 in June 2008, but it was not comitted. ( http://www.winehq.org/pipermail/wine-patches/2008-June/056310.html ) The bug 14085 was created to document the required feature: http://bugs.winehq.org/show_bug.cgi?id=14085 an importlib for multiple dlls is not supported (avicap32 + avifil32 + msvfw32 => vfw32) You can and should create tests (crosscompile them to check the Windows results and to verify the implementation changes) and send them as a seperate patch, but the patche can't be comitted before bug 14085 is fixed. -- By by ... Detlef

Nov. 10, 2009

Re: setupapi: add stub and some tests for SetupGetInfFileListW [resend]

by Henri Verbeet

2009/11/10 Stefan Leichter <Stefan.Leichter(a)camline.com>: > When reviewing the patch i found two line containing only spaces. But this are > to my understanding no "whitespace errors". Can you please tell me what else > you have seen ? > That counts as trailing spaces. Note that git apply warns about white space errors by default.

Nov. 10, 2009

Re: setupapi: add stub and some tests for SetupGetInfFileListW [resend]

by Stefan Leichter

Am Tuesday 10 November 2009 10:09:22 schrieb Henri Verbeet: > 2009/11/9 Stefan Leichter <Stefan.Leichter(a)camline.com>: > > my crystal ball is still not working, therefore resending unchanged > > Try http://source.winehq.org/patches/ > > And indeed, the patch doesn't apply: > > diff -urw wine-git/dlls/setupapi/install.c > wine-build/dlls/setupapi/install.c --- a/wine-git/dlls/setupapi/install.c > +++ b/wine-build/dlls/setupapi/install.c > > Note that you're also adding whitespace errors. When reviewing the patch i found two line containing only spaces. But this are to my understanding no "whitespace errors". Can you please tell me what else you have seen ? Thanks Stefan

Nov. 10, 2009

RE: [PATCH 16/16] comctl32/tests: Remove todo_wine from SHGetImageList tests

by Owen Rudge

> This patch suggest that one of the previous patches in this series makes > the tests succeed or actually fail as they are inside a todo_wine(). The previous patch is the one that makes the tests succeed. I split the todo_wine removal from the tests due to the modifications being in separate parts of the source tree, although I realise now that means the tests will give an error for the prior commit. I've submitted an updated patch that merges the two. Cheers, Owen

Nov. 10, 2009

Re: [PATCH 16/16] comctl32/tests: Remove todo_wine from SHGetImageList tests

by Paul Vriens

On 11/10/2009 07:12 PM, Owen Rudge wrote: > --- > dlls/comctl32/tests/imagelist.c | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > Hi Owen, This patch suggest that one of the previous patches in this series makes the tests succeed or actually fail as they are inside a todo_wine(). -- Cheers, Paul.

Nov. 10, 2009

Re: setupapi: add stub and some tests for SetupGetInfFileListW [resend]

by Alexandre Julliard

Stefan Leichter <Stefan.Leichter(a)camline.com> writes: > Am Tuesday 10 November 2009 10:09:22 schrieb Henri Verbeet: >> 2009/11/9 Stefan Leichter <Stefan.Leichter(a)camline.com>: >> > my crystal ball is still not working, therefore resending unchanged >> >> Try http://source.winehq.org/patches/ >> >> And indeed, the patch doesn't apply: >> >> diff -urw wine-git/dlls/setupapi/install.c >> wine-build/dlls/setupapi/install.c --- a/wine-git/dlls/setupapi/install.c >> +++ b/wine-build/dlls/setupapi/install.c >> > > i don't know where this stupid a/ and b/ was added. They are not in the > original patch see > http://www.winehq.org/pipermail/wine-patches/attachments/20091109/8912a56f/… They are added by the new patch filter, which tries to make everything appliable with git apply, and currently requires non-git patches to be based at the top of the tree. I'll look into supporting your case, but using git to format patches is strongly recommended. -- Alexandre Julliard julliard(a)winehq.org

Nov. 10, 2009

Re: rsaenh/tests(3/5): Add round-trip test of RSA

by Paul Vriens

On 11/05/2009 07:11 PM, Juan Lang wrote: > --Juan > > Hi Juan, This one introduces some test failures on Win9x and WinMe: http://test.winehq.org/data/tests/rsaenh:rsaenh.html Could you have a look? -- Cheers, Paul.

Nov. 10, 2009

Re: [8/9] widl: Add support for varargs functions.

by Paul Vriens

On 11/07/2009 03:55 PM, Rob Shearman wrote: > --- > dlls/rpcrt4/tests/server.c | 3 ++ > dlls/rpcrt4/tests/server.idl | 2 + > tools/widl/header.c | 68 ++++++++++++++++++++++------------------- > tools/widl/parser.l | 1 + > tools/widl/parser.y | 12 +++++-- > 5 files changed, 51 insertions(+), 35 deletions(-) > hi Rob, Either this one or "widl: Implement [range] attribute." introduced quite some test failures: http://test.winehq.org/data/tests/rpcrt4:server.html Could you please take a look? -- Cheers, Paul.

Nov. 10, 2009

Re: [2/3] oledb32: Add test for marshalling IDBProperties.

by Paul Vriens

On 11/07/2009 01:29 PM, Rob Shearman wrote: > --- > dlls/oledb32/tests/Makefile.in | 5 +- > dlls/oledb32/tests/marshal.c | 272 ++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 275 insertions(+), 2 deletions(-) > create mode 100644 dlls/oledb32/tests/marshal.c > Hi Rob, The test results for these new tests don't look too great: http://test.winehq.org/data/tests/oledb32:marshal.html Could you have a look? -- Cheers, Paul.

Nov. 10, 2009

Re: setupapi: add stub and some tests for SetupGetInfFileListW [resend]

by Stefan Leichter

Am Tuesday 10 November 2009 10:09:22 schrieb Henri Verbeet: > 2009/11/9 Stefan Leichter <Stefan.Leichter(a)camline.com>: > > my crystal ball is still not working, therefore resending unchanged > > Try http://source.winehq.org/patches/ > > And indeed, the patch doesn't apply: > > diff -urw wine-git/dlls/setupapi/install.c > wine-build/dlls/setupapi/install.c --- a/wine-git/dlls/setupapi/install.c > +++ b/wine-build/dlls/setupapi/install.c > i don't know where this stupid a/ and b/ was added. They are not in the original patch see http://www.winehq.org/pipermail/wine-patches/attachments/20091109/8912a56f/… > Note that you're also adding whitespace errors. I will check for whitespace problems

Nov. 10, 2009

Re: Infinite loop with translation DLL

by Marcus Meissner

On Mon, Nov 09, 2009 at 04:53:31PM +0100, Uwe Bonnes wrote: > >>>>> "Henri" == Henri Verbeet <hverbeet(a)gmail.com> writes: > > Henri> 2009/11/9 Uwe Bonnes <bon(a)elektron.ikp.physik.tu-darmstadt.de>: > >> Can anybody help with the flaw in my implementation? Or did anything > >> change in wine? > >> > Henri> I'm not sure if anything changed there recently, or if that's > Henri> even supposed to work, but I suppose that one way to solve the > Henri> problem would be to load the linux library dynamically with > Henri> wine_dlopen() and explicitly load the entry points with > Henri> wine_dlsym(). > > I wonder how it works in dll/glu32 or what is different in my approach? > > Marcus: Do you have an example that uses dll/glu32? No, I dont know. I have not used for years :/ Ciao, Marcus

Nov. 10, 2009

[Fwd: 64bit wine]

by Michael Stefaniuc

Nov. 10, 2009

Re: [PATCH] server, ntoskrnl: Implement support for IRP_MJ_CREATE requests.

by Charles Davis

Vitaliy Margolen wrote: > Charles Davis wrote: >> Also, add stub handlers in mountmgr. >> >> This patch fixes an issue with the previous one where wine would hang if >> ntoskrnl got a STATUS_BUFFER_OVERFLOW back from thes server. >> --- > >> +int suspend_thread( struct thread *thread ); >> +int resume_thread( struct thread *thread ); > >> +static void complete_open( struct open_call *open, unsigned int status ) >> +{ > ... >> + resume_thread( open->call.thread ); > > >> This static struct object *device_open_file( struct object *obj, unsigned int access, >> unsigned int sharing, unsigned int options ) >> { > ... >> + suspend_thread( current ); > > Looks like a hack to me. You shouldn't be suspending/resuming threads > directly like this. They should block on open operation. > > Vitaliy. > > I figured that. At the time, though, I thought that this was the path of least resistance, but I wasn't sure. I guess now I should make open_file_object return STATUS_PENDING, then make FILE_CreateFile() wait until some object is signaled. (This is the way we do IOCTLs.) I'll send a patch shortly for this. Chip

Nov. 10, 2009

Re: mciwave: Rework MCI notification system.

by Paul Vriens

On 11/10/2009 12:56 PM, Joerg-Cyril.Hoehle(a)t-systems.com wrote: > Hi, > > same patch as yesterday, with more spaces and volatile and mciSave fix thrown out. > > I was about to resend > [PATCH 2] winmm: MCI Close all causes one notification per open driver. > which I rebased as well when I realised that it's independent on the code here > (but should logically follow the present patch as #2). So I'm not resending that anew. > Please include it as well. > > Likewise, yesterday's > PATCH mciwave: Stop must not reset current position. > is completely independent on the notification stuff, so I'm not resubmitting it, > even though I sent it in third position. > > Well, I'll wait and see what feedback I get and what appears in git. > > Regards, > Jörg Höhle > Hi Jörg, Please resend all patches you want to have committed in a series with the proper numbering. If they can be applied independently then numbering is (strictly speeking) not needed but it does show where you're going. I'm not sure if it's only me but I have a hard time keeping tracking of all the patches you sent and what (or not) to apply in what order. -- Cheers, Paul.

Nov. 10, 2009

Re: Cpp Check Nov. 7th

by Alexandre Julliard

chris ahrendt <celticht32(a)yahoo.com> writes: > Well its been a few since I ran CPP Check and so I ran it today and got the following: > > New CPP Check Errors for the Wine Code: > > [/home/cahrendt/wine-git/dlls/comctl32/imagelist.c:1908]: (error) Uninitialized variable: bmi_buf > [/home/cahrendt/wine-git/dlls/ntdll/server.c:802]: (error) Resource leak: fd > [/home/cahrendt/wine-git/dlls/ntdll/server.c:882]: (error) Resource leak: fd_cwd > [/home/cahrendt/wine-git/dlls/shell32/shellord.c:416]: (error) Uninitialized variable: szText > [/home/cahrendt/wine-git/dlls/shell32/shellord.c:416]: (error) Uninitialized variable: szTitle > [/home/cahrendt/wine-git/dlls/shell32/shellpath.c:1360]: (error) Uninitialized variable: InfoBuffer > [/home/cahrendt/wine-git/dlls/winex11.drv/xfont.c:2963]: (possible error) Resource leak: fd > [/home/cahrendt/wine-git/server/mapping.c:118]: (error) Uninitialized variable: page_size These are once again all false positives. I don't think there's any point in continuing to post such reports, at least not until cppcheck becomes more reliable. -- Alexandre Julliard julliard(a)winehq.org

Nov. 10, 2009

Re: Wine 64-bit in Fedora ...

by Michael Stefaniuc

Uwe Bonnes wrote: >>>>>> "Michael" == Michael Stefaniuc <mstefani(a)redhat.com> writes: > > Michael> Fedora includes now a 64-bit Wine package (at least in F11) > Michael> which of course breaks existing Wine setups. "yum install wine" > Michael> will install the 64-bit version... Looks like we had our > Michael> WineConf too late for this. > > Michael> I have opened > Michael> https://bugzilla.redhat.com/show_bug.cgi?id=533988 for this > Michael> problem. > > Michael> The quick fix is to just remove the wine*.x86_64 rpms and > Michael> install the wine*.i586 instead. > > Did nobody of the packages test what they did? It is easy to miss as it requires: - Fresh install of Wine (via PackageKit or yum install) and - Old 32-bit WINEPREFIX. An update of Wine will pull the packages from the same arch aka i586 and thus the problem won't be visible for the people that just update their packages. That explains the limited outcry considering the impact of the problem. bye michael

Nov. 10, 2009

Re: Wine 64-bit in Fedora ...

by Damjan Jovanovic

On Tue, Nov 10, 2009 at 1:48 PM, Uwe Bonnes <bon(a)elektron.ikp.physik.tu-darmstadt.de> wrote: >>>>>> "Michael" == Michael Stefaniuc <mstefani(a)redhat.com> writes: > > Michael> Fedora includes now a 64-bit Wine package (at least in F11) > Michael> which of course breaks existing Wine setups. "yum install wine" > Michael> will install the 64-bit version... Looks like we had our > Michael> WineConf too late for this. > > Michael> I have opened > Michael> https://bugzilla.redhat.com/show_bug.cgi?id=533988 for this > Michael> problem. > > Michael> The quick fix is to just remove the wine*.x86_64 rpms and > Michael> install the wine*.i586 instead. > > Did nobody of the packages test what they did? No surprise there. At one point some distributions packaged wineserver as an /etc/init.d service, because it sounds like a system-wide server... Autopackage had the right idea: the developers of a project should do the packaging. > -- > Uwe Bonnes bon(a)elektron.ikp.physik.tu-darmstadt.de > > Institut fuer Kernphysik Schlossgartenstrasse 9 64289 Darmstadt > --------- Tel. 06151 162516 -------- Fax. 06151 164321 ---------- > > > Damjan

Nov. 10, 2009

Re: Wine 64-bit in Fedora ...

by Uwe Bonnes

>>>>> "Michael" == Michael Stefaniuc <mstefani(a)redhat.com> writes: Michael> Fedora includes now a 64-bit Wine package (at least in F11) Michael> which of course breaks existing Wine setups. "yum install wine" Michael> will install the 64-bit version... Looks like we had our Michael> WineConf too late for this. Michael> I have opened Michael> https://bugzilla.redhat.com/show_bug.cgi?id=533988 for this Michael> problem. Michael> The quick fix is to just remove the wine*.x86_64 rpms and Michael> install the wine*.i586 instead. Did nobody of the packages test what they did? -- Uwe Bonnes bon(a)elektron.ikp.physik.tu-darmstadt.de Institut fuer Kernphysik Schlossgartenstrasse 9 64289 Darmstadt --------- Tel. 06151 162516 -------- Fax. 06151 164321 ----------

Nov. 10, 2009

Re: Wine 64-bit in Fedora ...

by Michael Stefaniuc

Austin English wrote: > On Mon, Nov 9, 2009 at 5:54 PM, Michael Stefaniuc <mstefani(a)redhat.com> wrote: >> Fedora includes now a 64-bit Wine package (at least in F11) which of course >> breaks existing Wine setups. "yum install wine" will install the 64-bit >> version... Looks like we had our WineConf too late for this. >> >> I have opened https://bugzilla.redhat.com/show_bug.cgi?id=533988 for this >> problem. >> >> The quick fix is to just remove the wine*.x86_64 rpms and install the >> wine*.i586 instead. > > Actually, Roderick and I noticed this on IRC, with a user who couldn't > get wine to run more than a couple times on a single prefix. Oh, I didn't claim that I found the problem. I have seen arethusa on #winehq helping users get rid of Wine in the 64-bit flavor from Fedora. I have just verified that "yum install wine" will indeed give a 64-bit Wine, checked the (rpm) spec file and opened a bug in the Fedora bugzilla. Thanks for helping those users. bye michael

Nov. 10, 2009

Re: setupapi: add stub and some tests for SetupGetInfFileListW [resend]

by Henri Verbeet

2009/11/9 Stefan Leichter <Stefan.Leichter(a)camline.com>: > my crystal ball is still not working, therefore resending unchanged > Try http://source.winehq.org/patches/ And indeed, the patch doesn't apply: diff -urw wine-git/dlls/setupapi/install.c wine-build/dlls/setupapi/install.c --- a/wine-git/dlls/setupapi/install.c +++ b/wine-build/dlls/setupapi/install.c Note that you're also adding whitespace errors.

Nov. 10, 2009

Re: [PATCH 4] mciwave: Rework MCI notification system.

by Vitaliy Margolen

Vitaliy Margolen wrote: > Joerg-Cyril.Hoehle(a)t-systems.com wrote: >> + HANDLE old = InterlockedExchangePointer(&wmw->hCallback, NULL); >> + if (old) mciDriverNotify(old,wDevID,MCI_NOTIFY_SUPERSEDED); >> + mciDriverNotify(HWND_32(LOWORD(hWndCallBack)),wDevID,wStatus); > Here and everywhere else in your patches, please add space before commas. Of course I meant after comas. Vitaliy.

Nov. 10, 2009

Re: [PATCH] server, ntoskrnl: Implement support for IRP_MJ_CREATE requests.

by Vitaliy Margolen

Charles Davis wrote: > Also, add stub handlers in mountmgr. > > This patch fixes an issue with the previous one where wine would hang if > ntoskrnl got a STATUS_BUFFER_OVERFLOW back from thes server. > --- > +int suspend_thread( struct thread *thread ); > +int resume_thread( struct thread *thread ); > +static void complete_open( struct open_call *open, unsigned int status ) > +{ ... > + resume_thread( open->call.thread ); > This static struct object *device_open_file( struct object *obj, unsigned int access, > unsigned int sharing, unsigned int options ) > { ... > + suspend_thread( current ); Looks like a hack to me. You shouldn't be suspending/resuming threads directly like this. They should block on open operation. Vitaliy.

Nov. 10, 2009

Re: ntdll: fix memory leak in GetDiskFreeSpaceW

by Vitaliy Margolen

Dan Kegel wrote: > Found while valgrinding chromium's app_unittests. > > + done: > + RtlFreeHeap(GetProcessHeap(), 0, val.Buffer); > + You should us RtlFreeUnicodeString( &val ); instead. Vitaliy.

Nov. 10, 2009

Multithreaded debugging

by Peter Urbanec

Eric Pouech wrote: > Peter Urbanec a écrit : >> It can't be as simple as updating the value of dbg_curr_thread with >> the result of dbg_get_thread(dbg_curr_process, tid) and then calling >> GetThreadContext(dbg_curr_thread->handle, &dbg_context), can it? >> > the changes will need to be larger than that > winedbg assumes throughout the code that it's stopped in a single thread > but, if you change dbg_curr_thread, and enter the 'cont' command, you > have to decide which thread should be resumed > therefore, the changes imply to use two different objects: > - the thread on which an exception occured > - the thread you're currently examining > and that's not a simple task > moreover it'll be hard to understand the difference between the two > above mentionned thread objects > > so, finding another solution might be preferable > what are the thread's missing information you were thinking of ? I would like to do the following: [ exception / attach to process ] [ all threads in process are now suspended ] info threads thread 0x42 bt info locals print *some_class up info locals print *some_var Or in other words, being able to examine the state of any thread, including the state of local variables in various stack frames, would be invaluable in determining how various threads got to where they are and what they are doing. Obviously, the above example is only workable if all threads in the debugged process are suspended. If only the current thread is suspended, it makes it very difficult to debug multithreaded code anyway. Controlling execution of individual threads with cont is not very important to me. I'd be quite happy for the debugger to stop and start all threads in a process together. The application that I am working with routinely has around 20-30 threads in interactive use and the thread count goes even higher when rendering jobs are started. Best regards, Peter Urbanec

Nov. 10, 2009

Re: Wine 64-bit in Fedora ...

by Austin English

On Mon, Nov 9, 2009 at 5:54 PM, Michael Stefaniuc <mstefani(a)redhat.com> wrote: > Fedora includes now a 64-bit Wine package (at least in F11) which of course > breaks existing Wine setups. "yum install wine" will install the 64-bit > version... Looks like we had our WineConf too late for this. > > I have opened https://bugzilla.redhat.com/show_bug.cgi?id=533988 for this > problem. > > The quick fix is to just remove the wine*.x86_64 rpms and install the > wine*.i586 instead. Actually, Roderick and I noticed this on IRC, with a user who couldn't get wine to run more than a couple times on a single prefix. -- -Austin

Nov. 10, 2009

Wine 64-bit in Fedora ...

by Michael Stefaniuc

Fedora includes now a 64-bit Wine package (at least in F11) which of course breaks existing Wine setups. "yum install wine" will install the 64-bit version... Looks like we had our WineConf too late for this. I have opened https://bugzilla.redhat.com/show_bug.cgi?id=533988 for this problem. The quick fix is to just remove the wine*.x86_64 rpms and install the wine*.i586 instead. bye michael

Nov. 9, 2009

avifile bugfixes

by Julius Schwartzenberg

Here are some bugfixes for avifile that Maarten mainly did during WineConf. The behaviour corresponds to what native avifil32 does. I will add tests to show this and submit the patches then. (Maybe avifil32 does a few more hacks like these, I'll add those too when I find them when creating tests.) If anyone has any comments on the patches as they are now already, please let me know. Thanks! Julius >From 2e9c13211c0343fd59faf835a1d954416ab46203 Mon Sep 17 00:00:00 2001 From: Julius Schwartzenberg <julius.schwartzenberg(a)gmail.com> Date: Sun, 8 Nov 2009 15:18:26 +0100 Subject: [PATCH 1/3] avifile: Fix header for audio stream --- dlls/avifil32/avifile.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-) diff --git a/dlls/avifil32/avifile.c b/dlls/avifil32/avifile.c index 49e141f..4ecc6f6 100644 --- a/dlls/avifil32/avifile.c +++ b/dlls/avifil32/avifile.c @@ -1816,7 +1816,15 @@ static HRESULT AVIFILE_LoadFile(IAVIFileImpl *This) if (FAILED(hr)) return hr; }; - + if (pStream->lpFormat != NULL && pStream->sInfo.fccType == streamtypeAUDIO) + { + WAVEFORMATEX *wfx = pStream->lpFormat; + wfx->nBlockAlign = wfx->nChannels * wfx->wBitsPerSample / 8; + pStream->sInfo.dwSampleSize = wfx->nBlockAlign; + TRACE("Block size reset to %u, chan=%u bpp=%u\n", wfx->nBlockAlign, wfx->nChannels, wfx->wBitsPerSample); + pStream->sInfo.dwScale = 1; + pStream->sInfo.dwRate = wfx->nSamplesPerSec; + } if (mmioAscend(This->hmmio, &ck, 0) != S_OK) return AVIERR_FILEREAD; } -- 1.6.3.3 >From 969620de75ae75d13cf5f443842cee8bf28c4362 Mon Sep 17 00:00:00 2001 From: Julius Schwartzenberg <julius.schwartzenberg(a)gmail.com> Date: Sun, 8 Nov 2009 15:28:19 +0100 Subject: [PATCH 2/3] avifile: Fix header suggested buffersize --- dlls/avifil32/avifile.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-) diff --git a/dlls/avifil32/avifile.c b/dlls/avifil32/avifile.c index 4ecc6f6..c40cbd9 100644 --- a/dlls/avifil32/avifile.c +++ b/dlls/avifil32/avifile.c @@ -1652,7 +1652,7 @@ static HRESULT AVIFILE_LoadFile(IAVIFileImpl *This) This->fInfo.dwCaps = AVIFILECAPS_CANREAD|AVIFILECAPS_CANWRITE; This->fInfo.dwLength = MainAVIHdr.dwTotalFrames; This->fInfo.dwStreams = MainAVIHdr.dwStreams; - This->fInfo.dwSuggestedBufferSize = MainAVIHdr.dwSuggestedBufferSize; + This->fInfo.dwSuggestedBufferSize = 0; This->fInfo.dwWidth = MainAVIHdr.dwWidth; This->fInfo.dwHeight = MainAVIHdr.dwHeight; LoadStringW(AVIFILE_hModule, IDS_AVIFILETYPE, This->fInfo.szFileType, @@ -1754,8 +1754,7 @@ static HRESULT AVIFILE_LoadFile(IAVIFileImpl *This) pStream->sInfo.dwRate = streamHdr.dwRate; pStream->sInfo.dwStart = streamHdr.dwStart; pStream->sInfo.dwLength = streamHdr.dwLength; - pStream->sInfo.dwSuggestedBufferSize = - streamHdr.dwSuggestedBufferSize; + pStream->sInfo.dwSuggestedBufferSize = 0; pStream->sInfo.dwQuality = streamHdr.dwQuality; pStream->sInfo.dwSampleSize = streamHdr.dwSampleSize; pStream->sInfo.rcFrame.left = streamHdr.rcFrame.left; @@ -1905,6 +1904,13 @@ static HRESULT AVIFILE_LoadFile(IAVIFileImpl *This) } } + for (nStream = 0; nStream < This->fInfo.dwStreams; nStream++) + { + DWORD sugbuf = This->ppStreams[nStream]->sInfo.dwSuggestedBufferSize; + if (This->fInfo.dwSuggestedBufferSize < sugbuf) + This->fInfo.dwSuggestedBufferSize = sugbuf; + } + /* find other chunks */ FindChunkAndKeepExtras(&This->fileextra, This->hmmio, &ck, &ckRIFF, 0); -- 1.6.3.3 >From 39d6ecab76989b93d35a0568e105fd306fa32706 Mon Sep 17 00:00:00 2001 From: Julius Schwartzenberg <julius.schwartzenberg(a)gmail.com> Date: Sun, 8 Nov 2009 15:32:33 +0100 Subject: [PATCH 3/3] avifile: Fix playback of fixed sample size audio stream --- dlls/avifil32/avifile.c | 14 ++++++++++---- 1 files changed, 10 insertions(+), 4 deletions(-) diff --git a/dlls/avifil32/avifile.c b/dlls/avifil32/avifile.c index c40cbd9..5fa45ae 100644 --- a/dlls/avifil32/avifile.c +++ b/dlls/avifil32/avifile.c @@ -1093,6 +1093,9 @@ static HRESULT WINAPI IAVIStream_fnRead(IAVIStream *iface, LONG start, /* convert samples to bytes */ samples *= This->sInfo.dwSampleSize; + if (!buffer) + buffersize = samples; + while (samples > 0 && buffersize > 0) { if (block != This->dwCurrentFrame) { hr = AVIFILE_ReadBlock(This, block, NULL, 0); @@ -1102,11 +1105,12 @@ static HRESULT WINAPI IAVIStream_fnRead(IAVIStream *iface, LONG start, size = min((DWORD)samples, (DWORD)buffersize); size = min(size, This->cbBuffer - offset); - memcpy(buffer, ((BYTE*)&This->lpBuffer[2]) + offset, size); - + if (buffer) + memcpy(buffer, ((BYTE*)&This->lpBuffer[2]) + offset, size); block++; offset = 0; - buffer = ((LPBYTE)buffer)+size; + if (buffer) + buffer = ((LPBYTE)buffer)+size; samples -= size; buffersize -= size; @@ -1117,7 +1121,9 @@ static HRESULT WINAPI IAVIStream_fnRead(IAVIStream *iface, LONG start, *samplesread += size / This->sInfo.dwSampleSize; } - if (samples == 0) + TRACE("Returning %d/%d\n", bytesread ? *bytesread : -1, samplesread ? *samplesread : -1); + + if (samples == 0 || !buffer) return AVIERR_OK; else return AVIERR_BUFFERTOOSMALL; -- 1.6.3.3

Nov. 9, 2009

Re: Question about GetTokenInformation and memory leaks

by Reece Dunn

2009/11/9 Dan Kegel <dank(a)kegel.com>: > I've never used the security apis, so I'm pretty unfamiliar with them. > Valgrinding chromium's sandbox_unittests.exe shows the leak > > (I think the code leaks token_handle, but fixing > that doesn't get rid of the reported leak.) token_handle does not leak -- access_token is managing it via `access_token.Attach(token_handle);` (when OpenProcessToken succeeds and token_handle is not NULL).

Nov. 9, 2009

Re: Question about GetTokenInformation and memory leaks

by Rob Shearman

2009/11/9 Dan Kegel <dank(a)kegel.com>: > I've never used the security apis, so I'm pretty unfamiliar with them. > Valgrinding chromium's sandbox_unittests.exe shows the leak > > 16 bytes in 1 blocks are definitely lost in loss record 123 of 728 > at RtlAllocateHeap (heap.c:1423) > by RtlAllocateAndInitializeSid (sec.c:156) > by NtQueryInformationToken (nt.c:379) > by GetTokenInformation (security.c:676) > by ATL::CAccessToken::GetInfoConvert<ATL::CSid,_TOKEN_PRIMARY_GROUP> > (atlsecurity.h:754) > by ATL::CAccessToken::GetPrimaryGroup (atlsecurity.inl:3623) > by sandbox::RestrictedTokenTest_CustomInit_Test::TestBody > (restricted_token_unittest.cc:92) > > The test in question is on line 92 of > http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/src/restricted_toke… > > Can someone familiar with this stuff have a look? > I suspect it's a problem with the test, but > can't figure out what's going on. I would > have thought the destructors for ATL::CAccessToken > and ATL::CSid would have freed everything. > (I think the code leaks token_handle, but fixing > that doesn't get rid of the reported leak.) Hi Dan, The bug is in the Wine code. Try the attached patch, which should fix it as a side-effect. Thanks, -- Rob Shearman

Nov. 9, 2009

Question about GetTokenInformation and memory leaks

by Dan Kegel

I've never used the security apis, so I'm pretty unfamiliar with them. Valgrinding chromium's sandbox_unittests.exe shows the leak 16 bytes in 1 blocks are definitely lost in loss record 123 of 728 at RtlAllocateHeap (heap.c:1423) by RtlAllocateAndInitializeSid (sec.c:156) by NtQueryInformationToken (nt.c:379) by GetTokenInformation (security.c:676) by ATL::CAccessToken::GetInfoConvert<ATL::CSid,_TOKEN_PRIMARY_GROUP> (atlsecurity.h:754) by ATL::CAccessToken::GetPrimaryGroup (atlsecurity.inl:3623) by sandbox::RestrictedTokenTest_CustomInit_Test::TestBody (restricted_token_unittest.cc:92) The test in question is on line 92 of http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/src/restricted_toke… Can someone familiar with this stuff have a look? I suspect it's a problem with the test, but can't figure out what's going on. I would have thought the destructors for ATL::CAccessToken and ATL::CSid would have freed everything. (I think the code leaks token_handle, but fixing that doesn't get rid of the reported leak.) - Dan

Nov. 9, 2009

Re: Question on using winedbg when program does not crash, just becomes unresponsive

by Eric Pouech

Peter Urbanec a écrit : > > Dan Kegel wrote: >> start winedbg without any app >> say 'bt all' in winedbg >> Voila, you now have backtraces for every process, including the hung >> one. >> It's your job to get that into a text file and remove the >> uninteresting ones... > > > I've also been trying to debug a multithreaded app. There are at least > two complications that winedbg throws up: > > 1. Is now documented in http://bugs.winehq.org/show_bug.cgi?id=20617 > > 2. As far as I can tell, winedbg will not allow you to switch the > current thread context. This means that even though you can get a > backtrace of all threads in all processes, you actually can not > examine the state of a specific thread. > > I don't know enough about the internals of wine to go ahead and > implement a "thread tid" command. I could manage the code in dbg.y and > debug.l to implement the command line interface. When it comes to the > actual implementation, I can sort of guess at what may need to be done > there. However, I suspect that my naive attempts at an implementation > are unlikely to produce an acceptable patch. > > It can't be as simple as updating the value of dbg_curr_thread with > the result of dbg_get_thread(dbg_curr_process, tid) and then calling > GetThreadContext(dbg_curr_thread->handle, &dbg_context), can it? > the changes will need to be larger than that winedbg assumes throughout the code that it's stopped in a single thread but, if you change dbg_curr_thread, and enter the 'cont' command, you have to decide which thread should be resumed therefore, the changes imply to use two different objects: - the thread on which an exception occured - the thread you're currently examining and that's not a simple task moreover it'll be hard to understand the difference between the two above mentionned thread objects so, finding another solution might be preferable what are the thread's missing information you were thinking of ? A+ -- Eric Pouech "The problem with designing something completely foolproof is to underestimate the ingenuity of a complete idiot." (Douglas Adams)

Nov. 9, 2009

Re: The user32/win test crashes X on Intel cards

by Reece Dunn

2009/11/9 <Joerg-Cyril.Hoehle(a)t-systems.com>: > Hi, > >>Marcus got the same issue and he had an Intel card too > > Charles Davis wrote: >>This happens on Mac OS X, too. Check out >>http://xquartz.macosforge.org/trac/ticket/293 > > Charles forgot to mention that it affects NVidia on the Mac, which I did > not explicitly write in the above bug report (the Mac people will have > guessed it from the "early 2009 Mac mini" comment). > > More precisely > SetWindowPos(hwnd, 0, -32769, -40000, -32769, -90000, SWP_NOMOVE); > #if 0 > SetWindowPos(hwnd, 0, 32768, 40000, 32768, 40000, SWP_NOMOVE); > #endif > SetWindowPos(hwnd, 0, -32769, -40000, -32769, -90000, SWP_NOSIZE); > SetWindowPos(hwnd, 0, 32768, 40000, 32768, 40000, SWP_NOSIZE); > is enough to let the tests succeed with > Version: 2.3.3.2 (2.3.3) > Build Info: X11server-480700~6 > OS Version: Mac OS X 10.5.7 (9J61) > > This hints that the bug is in the X server, not the individual drivers? Shouldn't this be cross-posted to the Xorg mailing list so that the Xorg and driver devs can help out? Also, does anyone know if it is possible to see what calls to the X server are being made by these SetWindowPos calls (so that the Xorg devs have a better idea of what is going on). - Reece

Nov. 9, 2009

Re: To error out or to skip tests?

by Reece Dunn

2009/11/9 <Joerg-Cyril.Hoehle(a)t-systems.com>: > Reece, > > I think we reached agreement on: > a) Tests should "usually" succeed. > b) machines without sound are usual (e.g. in virtual server env.). > c) I'll modify the MCI tests to succeed on machines without sound. > But for some other points, I've trouble following you. Yes. >>> The relevant question becomes: when is a configuration unusual? When >>> can it be ignored for the purpose of Wine and when is it relevant? >>Answer: no configuration is unusual. > Huh? You asked "when is a configuration unusual?" That is, are the following unusual: 1/ using a non-English locale (e.g. German or Polish); 2/ using a non-Western locale (e.g. Chinese or Japanese); 3/ using classic theme in XP or later; 4/ using 120DPI or higher; 5/ using a high-contrast theme; 6/ not having DirectX installed; 7/ having a Windows install on a drive other than C:\ 8/ ... My point is that none of these (and other configurations/setups in Windows) should be considered unusual. They may (or may not) be the default, but they are possible. >From the perspective of Wine (and the Wine tests), if it is possible in Windows (on VMware or natively) then it should be handled properly in the tests and not fail the tests. >>The point is, if the machine has a broken soundcard (or in Wine does >>not have an available sound driver, or is broken due to PulseAudio on >>Ubuntu), the tests should still pass (or be skipped) as is >>appropriate. > > This seems wrong (in general) to me, even though Paul somehow > expresses an opinion similar to yours. I say you cannot write a > program meant to run in a completely unknown environment, doing > comprehensive tests and not stumbling upon strange behaviour. Write the tests initially for the known valid environments, then fix up issues as they are seen on test.winehq.org (for example, if some of the comctl32/user32 tests fail because they are run in 120DPI instead of 96DPI). You cannot cater for a completely unknown environment, but those on test.winehq.org are not unknown. Also, it is not feasible to always do comprehensive tests (e.g. on Win9x when *W functions are not present). The point is to do what you can in those cases. > The program is specifically designed to detect and flag deviant behaviour. > All that can be achieved in a program is to be prepared for a known > set of scenarios (a broken soundcard is such a scenerio), aka. use-case. > Outside these known use-cases, the tests throw an error. Otherwise > there's no point in ok() at all. You have a set of defined behaviour for the mci API when there is a sound card present. Good -- the tests should cover that. You also have a set of defined behaviour for the mci API when a sound card/driver is not present. Good -- the tests should handle this (e.g. by defining the set of valid return values). > I agree that for the known use-cases (these explicitly include > configurations without sound), the tests should succeed. Yes. > What can be done is what you say: >>If we find that other situations need correcting (due to >>failing tests), they can be looked at then. > ... then decide whether the cause of the failing test is "unusual" or not. No -- look at *why* the test is failing. This will either be that the test is doing something wrong (e.g. not creating a test file), or that it is not covering a valid situation. The cause being "unusual" or not has no relevance. Either the test is wrong, or Windows is broken. >>The point I am making is that: >> 1. The error and skip statements are saying the same thing; >> 2. But the skip message is misleading. > Indeed, sorry for that. > There's a dependency. The tests depend on record working to generate the > file to be played, instead of including one in git. > So they fail to test machines that can play but not record. Sure. > Obviously I'll have to do something else in the yet to be written MIDI tests. > > BTW, this raises the question whether Wine should include a tiny wave file > for completeness. Invoking the MCI Sound command always plays some sound. Is it possible to generate the wav file programatically, without using the record functionality? >>If I have counted the offsets correctly 275 is >>MCIERR_DEVICE_NOT_READY, *not* that tempfile.wav was not found. > Wrong. 275 - 256 is 19, which you look up in mmsystem.h: > #define MCIERR_FILE_NOT_FOUND (MCIERR_BASE + 19) Ah yes. >> default: >> ok(!err, "mciSendString('open') failed, expected NOERROR, got >>%s", dbg_mcierr(err)); >> } > In the default case, if (err) you must return (with or without skip message). > You cannot continue if open fails. So it summarizes to: > if (err) { > /* list of situations that were analysed as "usual": */ > ok(err=FILE_NOT_FOUND || err==MCIERR_CANNOT_LOAD_DRIVER || err==xyz ...); > skip("Cannot test playback, cause %s",dbg_mcierr(err)); > return > } > >> case MCIERR_CANNOT_LOAD_DRIVER: >> 2. sound is not present on the system. > Perhaps there's a misconception? That's not how MS-Windows systems without > sound drivers present themselves. Ge's, Paul's and my father in law's > machines all include the MCI (and waveaudio.dll etc.). It's only at > WAVE_Open time that failure is detected, e.g. Cue, Record or Play, and > the error is MCIERR_WAVE_INPUTSUNSUITABLE for instance. > (That's not entirely true, it does not explain the setformat pcm failure). > I.e. no mciwave present at all so far is an "unusual" situation, and generates an error. In the example I gave, don't look at the exact error codes -- look more at the situations (tempfile.wav does not exist, sound not available). Also, if (err==MCIERR_FILE_NOT_FOUND || err==...) { skip("Cannot test playback (err=%s)",dbg_mcierr(err)); return; } ok(!err, ...); The point here is that tempfile.wav not existing, sound not available, or a broken sound driver should not cause the ok line to fail. These are valid situations that mean that the playback tests cannot be run. Now, arguably the tests should be modified so that tempfile.wav always exist. This would then address a chunk of the test failures that are happening on all machines at present. IIUC, that file is being generated through the record API of mci? Isn't there a way to programatically create the WAV/PCM data through the use of math.h functions? - Reece

Nov. 9, 2009

Re: To error out or to skip tests?

by Reece Dunn

2009/11/9 Paul Vriens <paul.vriens.wine(a)gmail.com>: > On 11/09/2009 07:25 PM, Joerg-Cyril.Hoehle(a)t-systems.com wrote: >> >> Reece, >> >> I think we reached agreement on: >> a) Tests should "usually" succeed. > > I didn't read that in any of the emails. Tests should always succeed. the > broken() and skip() will help in achieving that. I used "usually" succeed to avoid handling out-of-memory cases and other similar cases. - Reece

Nov. 9, 2009

The user32/win test crashes X on Intel cards

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

Hi, >Marcus got the same issue and he had an Intel card too Charles Davis wrote: >This happens on Mac OS X, too. Check out >http://xquartz.macosforge.org/trac/ticket/293 Charles forgot to mention that it affects NVidia on the Mac, which I did not explicitly write in the above bug report (the Mac people will have guessed it from the "early 2009 Mac mini" comment). More precisely SetWindowPos(hwnd, 0, -32769, -40000, -32769, -90000, SWP_NOMOVE); #if 0 SetWindowPos(hwnd, 0, 32768, 40000, 32768, 40000, SWP_NOMOVE); #endif SetWindowPos(hwnd, 0, -32769, -40000, -32769, -90000, SWP_NOSIZE); SetWindowPos(hwnd, 0, 32768, 40000, 32768, 40000, SWP_NOSIZE); is enough to let the tests succeed with Version: 2.3.3.2 (2.3.3) Build Info: X11server-480700~6 OS Version: Mac OS X 10.5.7 (9J61) This hints that the bug is in the X server, not the individual drivers? Regards, Jörg Höhle

Nov. 9, 2009