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

Re: [4/4] ws2_32: Avoid a crash with an optional pointer in WSAAsyncGetServByName.

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=11362 Your paranoid android. === WXPPROSP3 (32 bit sock) === sock.c:501: Test failed: oob_server (218): unexpectedly at the OOB mark: 0 sock.c:511: Test failed: oob_server (218): unexpectedly at the OOB mark: 0

June 1, 2011

mmdevapi buffering

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

Hi, I have one big concern about MSDN's "Rendering a stream" (shared mode) example w.r.t wrap-around of buffer pointers and how Wine implements it. The example is at http://msdn.microsoft.com/en-us/library/dd316756(v=vs.85).aspx Suppose the write pointer is near the end of the buffer and the app is a little late, so the HW read pointer is not far away. |---------------------------------------------------^read------^write-----| What is GetCurrentPadding going to return? - the tiny write-read portion, says MSDN AFAICT - 0, because there's a new fresh buffer waiting to be filled? MSDN's example then computes avail = GetBufferSize() - GetCurrentPadding(); What is GetBuffer(avail) going to return?!? - If it's a pointer to the above buffer, it can only succeed if avail is tiny, otherwise there's a buffer overflow. However avail, as defined above and in MSDN's example, will be large. Wine tricks around that by returning a dynamic local tmp buffer larger than GetBufferSize (I wonder why a fixed one instead, twice GetBufferSize would not suffice?) Does native behave like that? - E_BUFFER_TOO_LARGE ? The MSDN example would exit. - Could it be a pointer to the beginning of the buffer? - Or it's a pointer to another buffer! That's not compatible with Wine's current local_buffer implementation... Isn't Wine's mmdevapi going to produces glitches? The problematic situation is when avail is small, as the following sleep for half a buffer size in MSDN's example will inevitably produce an xrun. So what actually is the buffer size and how does GetCurrentPadding behave? I don't expect MSDN's example to be fundamentally flawed. IMHO, they designed their API such that it is the typical use case. I believe that multiple buffers (of size GetBufferSize) are involved (much like ALSA's periods) and that GetBuffer switches among them -- exactly like they say about the exclusive mode ping pong. I suppose native's mixer (in shared mode) would mix these possibly incompletely filled buffers and everything is fine there. No need to deal with wrap-around! Why should Wine do something entirely different? Regards, Jörg Höhle

June 1, 2011

winealsa.drv:mmdevapi input validation

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

Hi, because of bug #27087, I had a look at the emerging mmdevapi code. Actually, it's the first time I read its documentation on MSDN. Here are a few comments of mine while reading the code. The handling of duration (-> buffer size) in winealsa.drv/mmdevdrv.c:AudioClient_Initialize does not perform enough validation. MSDN explicitly acknowledges that it could be 0. I believe it should be related to ALSA's buffer, or a minimum of something like 2*period_size. Or perhaps even 3*period size. IIRC MSDN also documents a maximum of 2s for renderer and 0.5 for capture. AudioRenderClient_ReleaseBuffer does not check that written_frames is what GetBuffer returned. Use with AUDCLNT_BUFFERFLAGS_SILENT and you'll have memcpy overwrite arbitrary amounts of memory. MSDN says: AUDCLNT_E_INVALID_SIZE I believe Wine should currently include if (AUDCLNT_STREAMFLAGS_EVENTCALLBACK && SHAREMODE_EXCLUSIVE) { FIXME("unsupported") & return E_ } because none of the buffer allocation and parameter checking specific to that mode that MSDN mentions in Initialize is implemented. When it (the ping pong buffer logic) will be implemented, some better logic to trigger SetEvent() will probably be needed, because I'm unsure whether CreateTimerQueueTimer() provides the required stability over time and possibly ignores a laptop's suspend/resume cycle or xruns. In exclusive mode, reception of an event is a guarantee that GetBuffer(GetBufferSize frames) will succeed. I suggest the values returned by AudioClient_GetDevicePeriod be adjusted to match what test.winehq shows. 10ms/3ms makes some sense, 10ms is mentioned a lot in MSDN and blogs. 3ms is small yet has a chance to work without glitch with MSDN's exclusive mode example. I can't remember seeing input validation of broken avgbytes and blockalign in the WAVEFORMAT parameter. I showed some tests some month ago that prove that unlike Wine, winmm is not disturbed by bad values. How does mmdevapi react to them? Regards, Jörg Höhle

June 1, 2011

Re: infosoft/tests: COM cleanup in infosoft.c.

by Michael Stefaniuc

Marvin wrote: > 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=11350 This one seems to be a limitation in the WTB: the failing machines don't have the infosoft DLL. Can be checked in the wintest.exe results of those machines, they show all "infosoft missing". > Your paranoid android. > > > === WVISTAADM (32 bit infosoft) === > infosoft.c:183: Test failed: failed > infosoft.c:197: Test failed: failed > > === W2K8SE (32 bit infosoft) === > infosoft.c:183: Test failed: failed > infosoft.c:197: Test failed: failed > > === W7PRO (32 bit infosoft) === > infosoft.c:183: Test failed: failed > infosoft.c:197: Test failed: failed > > === W7PROX64 (32 bit infosoft) === > infosoft.c:183: Test failed: failed > infosoft.c:197: Test failed: failed > > === W7PROX64 (64 bit infosoft) === > infosoft.c:183: Test failed: failed > infosoft.c:197: Test failed: failed bye michael

June 1, 2011

Re: [4/5] ddraw: Extend the lifetime of the IDirect3D7::EnumDevices strings beyond function scope.

by Stefan Dösinger

Am 31.05.2011 um 18:00 schrieb Andrew Nguyen: > No, I simply defined the structure there to be consistent with the role > of the unused structure definitions I removed earlier. I can certainly > move the definition to ddraw.c. Yes, please. And yeah, the existing code could use some improvement in this regard too, but that is of course beyond the scope of this patch.

June 1, 2011

ddraw: Add tests for Setcooperativelevel

by paulo lesgaz

Hi, any problem with the patch http://source.winehq.org/patches/data/74702 A+ David

June 1, 2011