- wine-devel - winehq.org

Re: winhttp: Fix the error returned from WinHttpGetProxyForUrl when autodetection fails.
by Marvin 01 Nov '12

01 Nov '12

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=22654 Your paranoid android. === WXPPROSP3 (32 bit winhttp) === winhttp.c:2405: Test failed: got 80072ee2 winhttp.c:2411: Test failed: got 8000000a

2 1

Re: gdiplus: Add support for extra transformation in GdipMeasureDriverString and GdipDrawDriverString.
by Vincent Povirk 31 Oct '12

31 Oct '12

> @@ -6337,6 +6345,11 @@ GpStatus WINGDIPAPI GdipMeasureDriverString(GpGraphics *graphics, GDIPCONST UINT > pt[2].X = 0.0; > pt[2].Y = 1.0; > GdipTransformPoints(graphics, CoordinateSpaceDevice, CoordinateSpaceWorld, pt, 3); > + if (matrix) > + { > + GpMatrix xform = *matrix; > + GdipTransformMatrixPoints(&xform, pt, 3); > + } > rel_width = sqrt((pt[1].Y-pt[0].Y)*(pt[1].Y-pt[0].Y)+ > (pt[1].X-pt[0].X)*(pt[1].X-pt[0].X)); > rel_height = sqrt((pt[2].Y-pt[0].Y)*(pt[2].Y-pt[0].Y)+ Without tests, I can only guess, but I think it's more likely that this matrix transform should be applied before the world->device transform. Apart from questions of whether this is correct behavior, the patch looks good to me.

2 4

Re: vbscript/tests: Skip some of the weekday tests if the first day of the week is not Sunday.
by Jacek Caban 31 Oct '12

31 Oct '12

Hi Huw, On 10/31/12 12:04, Huw Davies wrote: > --- > dlls/vbscript/tests/api.vbs | 17 +++++++++-------- > 1 files changed, 9 insertions(+), 8 deletions(-) Did you see those failures on Windows? test.winehq.org shows only some Linux failures, which suggests that the problem is in implementation, not tests. Jacek

2 1

Re: vbscript/tests: Skip some of the weekday tests if the first day of the week is not Sunday.
by Marvin 31 Oct '12

31 Oct '12

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=22625 Your paranoid android. === WNT4WSSP6 (32 bit) === No test summary line found === W2KPROSP4 (32 bit) === No test summary line found === WXPPROSP3 (32 bit) === No test summary line found === W2K3R2SESP2 (32 bit) === No test summary line found === WVISTAADM (32 bit) === No test summary line found === W2K8SE (32 bit) === No test summary line found === W7PRO (32 bit) === No test summary line found === W7PROX64 (32 bit) === No test summary line found === TEST64_W7SP1 (32 bit) === No test summary line found === W7PROX64 (64 bit) === No test summary line found === TEST64_W7SP1 (64 bit) === No test summary line found

1 0

Re: [PATCH] services: Fix non common sizeof usages. (resend)
by Alexandre Julliard 31 Oct '12

31 Oct '12

Christian Costa <titan.costa(a)gmail.com> writes: > Use parentesis is more common in Wine code and this makes all sizeof conform to file style anyway. You can clean this up if you are changing the code for other reasons, but otherwise there's no reason to do this. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: hh: Avoid handle leak (coverity)
by Alexandre Julliard 31 Oct '12

31 Oct '12

Frédéric Delanoy <frederic.delanoy(a)gmail.com> writes: > @@ -27,9 +27,13 @@ int WINAPI WinMain(HINSTANCE hInst, HINSTANCE hPrevInst, LPSTR cmdline, int cmds > > HMODULE hModule; > DOWINMAIN *doWinMain; > + int res; > > hModule = LoadLibraryA("hhctrl.ocx"); > doWinMain = (DOWINMAIN*) GetProcAddress(hModule, "doWinMain"); > > - return doWinMain(hInst, cmdline); > + res = doWinMain(hInst, cmdline); > + FreeLibrary(hModule); > + > + return res; Freeing things on process exit is not useful. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH] user32: Fix error handling in MapWindowPoints, ClientToScreen and ScreenToClient and add tests for them. (try 6) (resend)
by Dmitry Timoshkov 31 Oct '12

31 Oct '12

Christian Costa <titan.costa(a)gmail.com> wrote: > Based ont a patch of Rico Schuller. I'd suggest to make sure that the test with appropriate todo_wine statements is accepted first, and only then start thinking about a proper fix, or even leave creating the fix to someone else. -- Dmitry.

2 1

What parts of gettext does Wine need at runtime?
by Scott Ritchie 31 Oct '12

31 Oct '12

Is libgettextpo0 (in 32 and 64 bit) sufficient? Or do we need the sort of utilities in the gettext-base package? I'm trying to fix https://bugs.launchpad.net/ubuntu/precise/+source/gettext/+bug/954029 which was largely caused by my overcautiousness here.

2 1

Re: [PATCH 3/4] msvcp90: in istream<>::tellg don't use sentry
by Piotr Caban 30 Oct '12

30 Oct '12

On 10/30/12 10:16 PM, Daniel Lehman wrote: + tpos.pos = 0xdeadbeefdeadbeef; Long long constants are not supported in C89. You can use 0xdeadbeef here.

1 0

xapofx1_3.dll?
by Joerg-Cyril.Hoehle＠t-systems.com 30 Oct '12

30 Oct '12

Hi, why does Wine include a stub xapofx1_1.dll but no xapofx1_3.dll? Installing Wallace & Gromit 102 "The Last Resort" / "Urlaub unter Tage" adds xapofx1_3.dll XAudio2_4.dll and X3DAudio1_6.dll to system32 (though IIRC I asked the installer not to care about DirectX). This app loads none of these libraries (well, I solely tested winecfg's default xp mode, not w7. Presumably those dlls are only relevant since Vista with mmdevapi and the app uses DSound on older systems). http://bugs.winehq.org/show_bug.cgi?id=27279 is about apps linked against this dll such that they won't even start. Regards, Jörg Höhle

2 1

Re: scrrun: Add basic IDictionary getHasVal support
by Piotr Caban 30 Oct '12

30 Oct '12

Hi, On 10/30/12 09:51, Alistair Leslie-Hughes wrote: +static LONG create_hash_val(BSTR name) +{ + LONG hash = 0; + int len = SysStringLen(name); + int i; + + for(i =0; i < len; i++) + { + hash += name[i]; + } + + return hash; +} This hashing function will have lots of conflicts when used in hash-table. It's quite easy to guess how it's done in native dll: for(i=0; i<len; i++) hash = (hash<<4) + hash + name[i]; hash %= 1201; It would be nice to add some tests for strings containing '\0' characters and for other VARIANT types. I don't know how scrrun stores data internally, if it's using a hash-table, the function may change after adding some elements.

1 0

Re: ntdll: Do not execute callbacks past DeleteTimer(INVALID_HANDLE_VALUE)
by Alexandre Julliard 30 Oct '12

30 Oct '12

<Joerg-Cyril.Hoehle(a)t-systems.com> writes: > You know this patch from March this year. Initially, this patch was > the first of 2-3. I decided to release it on its own after finding > out yesterday that in July, Fran,cois Gouget added a todo_wine for a > bug that I fixed monthes earlier! Now I had to augment my patch > with a one-liner removing that todo_wine from kernel32/tests. > > Let me thank him for writing a test case that demonstrates the > effectiveness of my patch. Or lack thereof ;-) ../../../tools/runtest -q -P wine -M kernel32.dll -T ../../.. -p kernel32_test.exe.so sync.c && touch sync.ok sync.c:808: Test failed: Timer callback 0 expected 1 got 0 sync.c:1166: Tests skipped: one-time initialization API not supported make[1]: *** [sync.ok] Error 1 -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 4/5] include: Don't add d3d8 LP* interface typedefs for __WINESRC__.
by Alexandre Julliard 30 Oct '12

30 Oct '12

Henri Verbeet <hverbeet(a)codeweavers.com> writes: > @@ -1128,6 +1105,21 @@ DECLARE_INTERFACE_(IDirect3DDevice8,IUnknown) > #define IDirect3DDevice8_DeletePatch(p,a) (p)->DeletePatch(a) > #endif > > +#ifndef __WINESRC__ > +typedef struct IDirect3D8 *LPDIRECT3D8; > +typedef struct IDirect3DDevice8 *LPDIRECT3DDEVICE8; > +typedef struct IDirect3DResource8 *LPDIRECT3DRESOURCE8, *PDIRECT3DRESOURCE8; > +typedef struct IDirect3DVertexBuffer8 *LPDIRECT3DVERTEXBUFFER8, *PDIRECT3DVERTEXBUFFER8; > +typedef struct IDirect3DVolume8 *LPDIRECT3DVOLUME8, *PDIRECT3DVOLUME8; > +typedef struct IDirect3DSwapChain8 *LPDIRECT3DSWAPCHAIN8, *PDIRECT3DSWAPCHAIN8; > +typedef struct IDirect3DSurface8 *LPDIRECT3DSURFACE8, *PDIRECT3DSURFACE8; > +typedef struct IDirect3DIndexBuffer8 *LPDIRECT3DINDEXBUFFER8, *PDIRECT3DINDEXBUFFER8; > +typedef struct IDirect3DBaseTexture8 *LPDIRECT3DBASETEXTURE8, *PDIRECT3DBASETEXTURE8; > +typedef struct IDirect3DTexture8 *LPDIRECT3DTEXTURE8, *PDIRECT3DTEXTURE8; > +typedef struct IDirect3DCubeTexture8 *LPDIRECT3DCUBETEXTURE8, *PDIRECT3DCUBETEXTURE8; > +typedef struct IDirect3DVolumeTexture8 *LPDIRECT3DVOLUMETEXTURE8, *PDIRECT3DVOLUMETEXTURE8; > +#endif I don't think we want to do that sort of thing, otherwise we'll end up with ifdef __WINESRC__ all over the standard headers. If we really want to disable interface typedefs this should be done in a generic way, preferably when generating headers from idl. -- Alexandre Julliard julliard(a)winehq.org

3 3

Re: [PATCH] kernel32: Implement CompareStringOrdinal. (try 2)
by Nikolay Sivov 30 Oct '12

30 Oct '12

On 10/30/2012 01:43, Christian Costa wrote: > Try 2: > - Don't use CompareStringEx. > - Add tests > --- > dlls/kernel32/kernel32.spec | 1 + > dlls/kernel32/locale.c | 28 ++++++++++++++++++++ > dlls/kernel32/tests/locale.c | 60 +++++++++++++++++++++++++++++++++++++++++- > 3 files changed, 88 insertions(+), 1 deletion(-) > > diff --git a/dlls/kernel32/kernel32.spec b/dlls/kernel32/kernel32.spec > index b7efa0f..0bd1adc 100644 > --- a/dlls/kernel32/kernel32.spec > +++ b/dlls/kernel32/kernel32.spec > @@ -201,6 +201,7 @@ > @ stdcall CompareStringA(long long str long str long) > @ stdcall CompareStringW(long long wstr long wstr long) > @ stdcall CompareStringEx(wstr long wstr long wstr long ptr ptr long) > +@ stdcall CompareStringOrdinal(wstr long wstr long long) > @ stdcall ConnectNamedPipe(long ptr) > @ stub ConsoleMenuControl > @ stub ConsoleSubst > diff --git a/dlls/kernel32/locale.c b/dlls/kernel32/locale.c > index c41442c..07eb688 100644 > --- a/dlls/kernel32/locale.c > +++ b/dlls/kernel32/locale.c > @@ -3047,6 +3047,34 @@ INT WINAPI CompareStringA(LCID lcid, DWORD flags, > return ret; > } > > +/****************************************************************************** > + * CompareStringOrdinal (KERNEL32.@) > + */ > +INT WINAPI CompareStringOrdinal(const WCHAR *str1, INT len1, const WCHAR *str2, INT len2, BOOL ignore_case) > +{ > + int ret, len; > + > + if (!str1 || !str2) > + { > + SetLastError(ERROR_INVALID_PARAMETER); > + return 0; > + } > + if (len1 < 0) len1 = strlenW(str1); > + if (len2 < 0) len2 = strlenW(str2); > + > + len = min(len1, len2); > + if (ignore_case) > + ret = strncmpiW(str1, str2, len); > + else > + ret = strncmpW(str1, str2, len); > + > + if ((ret < 0) || (!ret && (len1 < len2))) > + return CSTR_LESS_THAN; > + else if ((ret > 0) || (!ret && (len1 > len2))) > + return CSTR_GREATER_THAN; > + return CSTR_EQUAL; > +} > + This is almost the same as your first try. You're testing a trivial case only - ASCII range. I feel like it should behave more like memcmp for case insensitive comparison, that's what documentation mildly suggests.

3 6

Re: vbscript: Added support for more builtin constants
by Marvin 30 Oct '12

30 Oct '12

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=22595 Your paranoid android. === WNT4WSSP6 (32 bit) === No test summary line found === W2KPROSP4 (32 bit) === No test summary line found === WXPPROSP3 (32 bit) === No test summary line found === W2K3R2SESP2 (32 bit) === No test summary line found === WVISTAADM (32 bit) === No test summary line found === W2K8SE (32 bit) === No test summary line found === W7PRO (32 bit) === No test summary line found === W7PROX64 (32 bit) === No test summary line found === TEST64_W7SP1 (32 bit) === No test summary line found === W7PROX64 (64 bit) === No test summary line found === TEST64_W7SP1 (64 bit) === No test summary line found

1 0

Re: [PATCH] wininet: TLS fallback mechanism (take2)
by Alexandre Julliard 29 Oct '12

29 Oct '12

Hiroshi Miura <miurahr(a)linux.com> writes: > +#ifdef SONAME_LIBSSL > + /* can't connect if we are already connected */ > + if (connection->ssl_s) > + { > + ERR("already connected\n"); > + return ERROR_INTERNET_CANNOT_CONNECT; > + } > + > + /* connect with given TLS options */ > + res = netcon_secure_connect_setup(connection, get_tls_option()); > + if (res == ERROR_SUCCESS) > + return res; You missed some #ifdefs, this won't compile. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: crypt32: Avoid double free in CRYPT_LoadSIP on error path (coverity))
by Nikolay Sivov 29 Oct '12

29 Oct '12

On 10/29/2012 11:33, Frédéric Delanoy wrote: > FreeLibrary(temp) can be called twice on error. > > CID 714004 > --- > > error: > FreeLibrary(lib); > - FreeLibrary(temp); > + if (temp != NULL) FreeLibrary(temp); > SetLastError(TRUST_E_SUBJECT_FORM_UNKNOWN); > return FALSE; > } This is useless, FreeLibrary() checks for NULL.

2 1

Re: [PATCH] kernel32: Implement CompareStringOrdinal.
by Nikolay Sivov 29 Oct '12

29 Oct '12

On 10/29/2012 00:53, Christian Costa wrote: > --- > dlls/kernel32/kernel32.spec | 1 + > dlls/kernel32/locale.c | 8 ++++++++ > 2 files changed, 9 insertions(+) > > diff --git a/dlls/kernel32/kernel32.spec b/dlls/kernel32/kernel32.spec > index b7efa0f..0bd1adc 100644 > --- a/dlls/kernel32/kernel32.spec > +++ b/dlls/kernel32/kernel32.spec > @@ -201,6 +201,7 @@ > @ stdcall CompareStringA(long long str long str long) > @ stdcall CompareStringW(long long wstr long wstr long) > @ stdcall CompareStringEx(wstr long wstr long wstr long ptr ptr long) > +@ stdcall CompareStringOrdinal(wstr long wstr long long) > @ stdcall ConnectNamedPipe(long ptr) > @ stub ConsoleMenuControl > @ stub ConsoleSubst > diff --git a/dlls/kernel32/locale.c b/dlls/kernel32/locale.c > index c41442c..83de815 100644 > --- a/dlls/kernel32/locale.c > +++ b/dlls/kernel32/locale.c > @@ -3047,6 +3047,14 @@ INT WINAPI CompareStringA(LCID lcid, DWORD flags, > return ret; > } > > +/****************************************************************************** > + * CompareStringOrdinal (KERNEL32.@) > + */ > +INT WINAPI CompareStringOrdinal(LPCWSTR str1, INT len1, LPCWSTR str2, INT len2, BOOL ignore_case) > +{ > + return CompareStringEx(NULL, ignore_case ? NORM_IGNORECASE : 0, str1, len1, str2, len2, NULL, NULL, 0); > +} > + > /************************************************************************* > * lstrcmp (KERNEL32.@) > * lstrcmpA (KERNEL32.@) This is not a way to do it, it doesn't simply work as that. See http://msdn.microsoft.com/en-us/library/windows/desktop/dd318144%28v=vs.85%… And it need tests of course.

2 1

Re: [PATCH 1/2] msvcp90: set eof state in istream<>::peek if got eof
by Alexandre Julliard 29 Oct '12

29 Oct '12

Daniel Lehman <dlehman(a)esri.com> writes: > +static void test_istream_peek(void) > +{ > + unsigned short testus, nextus; > + basic_stringstream_wchar wss; > + basic_stringstream_char ss; > + basic_string_wchar wstr; > + basic_string_char str; > + IOSB_iostate state; > + int i, next, peek; > + wchar_t wide[64]; > + > + struct _test_istream_peek { > + const char *str; > + int peek; > + int next; > + IOSB_iostate state; > + } tests[] = { > + { "", EOF, EOF, IOSTATE_eofbit }, > + { "ABCDEF", 'A', 'A', IOSTATE_goodbit }, > + }; > + > + for(i=0; i<sizeof(tests)/sizeof(tests[0]); i++) { > + /* char version */ > + call_func2(p_basic_string_char_ctor_cstr, &str, tests[i].str); > + call_func4(p_basic_stringstream_char_ctor_str, &ss, &str, OPENMODE_out|OPENMODE_in, TRUE); > + > + peek = (int)call_func1(p_basic_istream_char_peek, &ss.base.base1); > + state = (IOSB_iostate)call_func1(p_ios_base_rdstate, &ss.basic_ios.base); > + next = (int)call_func1(p_basic_istream_char_get, &ss.base.base1); 'peek' is not tested anywhere. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: gdiplus: fix rounding error (?)
by Vincent Povirk 29 Oct '12

29 Oct '12

That was the worst possible defence of this patch, but I think it should be included. What it's actually doing is removing the duplicated logic between the earlier bit of GdipDrawString, where out-of-range sizes are set to 1 << 23, and this if statement where we only apply a clipping rectangle if the sizes are out of range. We could also have added a check for e.g. scaled_rect.Width >= INT_MAX here, but I think it's better that we don't duplicate the logic.

1 0

Re: dplayx: Reorder some code to avoid memory leaks (coverity)
by Rico Schüller 29 Oct '12

29 Oct '12

On 28.10.2012 16:13, André Hentschel wrote: > - lpGData->lpRemoteData = lpNewData; > + lpGData->lpRemoteData = HeapAlloc( GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof( dwDataSize ) ); > + CopyMemory( lpGData->lpRemoteData, lpData, dwDataSize ); > lpGData->dwRemoteDataSize = dwDataSize; > - lpPData->lpRemoteData = lpNewData; > + lpPData->lpRemoteData = HeapAlloc( GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof( dwDataSize ) ); > + CopyMemory( lpPData->lpRemoteData, lpData, dwDataSize ); > lpPData->dwRemoteDataSize = dwDataSize; Is the HEAP_ZERO_MEMORY really needed? You may kill that too while you change those lines. Cheers Rico

2 1

Re: [3/5] d3dx9: Introduce a function for copying pixels.
by Christian Costa 29 Oct '12

29 Oct '12

> > pixel_format = get_format_info(src_desc.Format); > - if (pixel_format->type != FORMAT_ARGB) > - { > - FIXME("Unsupported pixel format %#x\n", src_desc.Format); > - return E_NOTIMPL; > - } > + if (pixel_format->type == FORMAT_UNKNOWN) return E_NOTIMPL; > > Doesn't this still require a FIXME or at least a WARN?

2 1

Re: vbscript: Added support for negative constants
by Marvin 29 Oct '12

29 Oct '12

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=22550 Your paranoid android. === WNT4WSSP6 (32 bit) === No test summary line found === W2KPROSP4 (32 bit) === No test summary line found === WXPPROSP3 (32 bit) === No test summary line found === W2K3R2SESP2 (32 bit) === No test summary line found === WVISTAADM (32 bit) === No test summary line found === W2K8SE (32 bit) === No test summary line found === W7PRO (32 bit) === No test summary line found === W7PROX64 (32 bit) === No test summary line found === TEST64_W7SP1 (32 bit) === No test summary line found === W7PROX64 (64 bit) === No test summary line found === TEST64_W7SP1 (64 bit) === No test summary line found

1 0

Re: [PATCH] Potential reference count races
by Nikolay Sivov 29 Oct '12

29 Oct '12

On 10/28/2012 17:44, Max TenEyck Woodbury wrote: > On 10/28/2012 02:40 AM, Nikolay Sivov wrote: >> On 10/28/2012 04:59, max(a)mtew.isa-geek.net wrote: >>> From: Max TenEyck Woodbury <max(a)mtew.isa-geek.net> >>> >>> I have been looking at the Microsoft COM and related documentations >>> and noticed that they emphatically recommend using the Interlocked... >>> functions when manipulating reference counts. I managed to set up a >>> search that showed where many of the reference count updates occur and >>> was somewhat surprised at how often this advice was not followed. >> It doesn't mean it always has to be followed. > True in a limited sense, but there is a good reason behind their > recommendation. Unless there is a good reason not to do so, this > particular piece of advice should be followed. COM objects in wine follow this recommendation in general, even object itself is not thread safe. This doesn't mean however that you need this every time you have some kind of refcount of any sort. > >>> While I have not converted every reference count update to use the >>> Interlocked... functions, this set of patches fixes a fair number of >>> them. >>> >>> These are not associated with any particular bug report; they are simply >>> a general precaution against operations that are known to be associated >>> with race conditions. >> This precaution doesn't work in general. It's not enough to atomically >> update refcount to make an implementation thread safe. Also not everything >> is supposed to be thread safe in a first place. >> > First, explain what does not have to be thread safe. Anything really, COM objects in particular if you were talking about them. > I believe that > application may try to use multiple threads anywhere, so everything > that can be made thread safe, should be. No. > Using interlocks on the > reference count updates is a necessary step for thread safety. You > are correct that it is not sufficient, but it is necessary. Again, it depends. > >> Changes like this: >> >>> - for (i=0;i<howmuch;i++) >>> + for (i=0;i<howmuch;++i) >>> TRACE("notify at %d to %p\n", >>> notify[i].dwOffset,notify[i].hEventNotify); >> are not helpful at all. > The post increment and decrement operation are specified as saving > the original value for use in the evaluation of the expression they > are part of and modifying the underlying stored value. In expressions > like this, that saved value is then discarded. The optimization phase > of the compilation usually removes both the save and discard operations. Sure, but I don't think it's enough to justify such changes all over the place, in existing code. > > Murphy's law suggests that this might cause problems at some point when > least desired. Please don't do this. > > Specifying the unnecessary use of a temporary store is a bad habit to > have. You should tell the compiler exactly what needs to be done, not > throw in extraneous operations. > > So, the use of a post operator where a prefix operator is sufficient, > while almost certainly harmless, is still technically a mistake. > > At the minimum, these changes provide examples of the correct way these > statements should be coded. So, I have to disagree, although very > mildly, that changes like that are not helpful. This kind of change > does not deserve a separate patch, but should be perfectly acceptable as > an adjunct to other patches.

4 4

Re: [PATCH] Remove potential reference count races
by Nikolay Sivov 29 Oct '12

29 Oct '12

On 10/28/2012 18:00, Max TenEyck Woodbury wrote: > On 10/28/2012 03:13 AM, Nikolay Sivov wrote: >> On 10/28/2012 04:59, max(a)mtew.isa-geek.net wrote: >>> From: Max TenEyck Woodbury <max(a)mtew.isa-geek.net> >>> >>> --- >>> dlls/shlwapi/thread.c | 4 ++-- >>> 1 files changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/dlls/shlwapi/thread.c b/dlls/shlwapi/thread.c >>> index eb2c35d..43e0433 100644 >>> --- a/dlls/shlwapi/thread.c >>> +++ b/dlls/shlwapi/thread.c >>> @@ -157,7 +157,7 @@ static ULONG WINAPI threadref_AddRef(IUnknown *iface) >>> threadref * This = impl_from_IUnknown(iface); >>> TRACE("(%p)\n", This); >>> - return InterlockedIncrement(This->ref); >>> + return InterlockedIncrement(&This->ref); >>> } >>> static ULONG WINAPI threadref_Release(IUnknown *iface) >>> @@ -167,7 +167,7 @@ static ULONG WINAPI threadref_Release(IUnknown >>> *iface) >>> TRACE("(%p)\n", This); >>> - refcount = InterlockedDecrement(This->ref); >>> + refcount = InterlockedDecrement(&This->ref); >>> if (!refcount) >>> HeapFree(GetProcessHeap(), 0, This); >>> >> Did you try to build this? >> >> > Yes, it built. I don't submit stuff that causes the compiler to barf > unless I am completely totally zonked from lack of sleep and this was > not what happened here. I will admit that I have not looked at the > build and test logs closely, so there may be problems I missed. > These changes I wrong that's what I'm saying. P.S. please CC wine-devel list in your replies.

2 1

on UTF-8, other Unicode encodings and WinAPI
by Saulius Krasuckas 28 Oct '12

28 Oct '12

Proposal with probably several insights: http://www.utf8everywhere.org/ Lyrical, older version: http://www.joelonsoftware.com/articles/Unicode.html S.

1 0

VBSCRIPT.DLL errors with CorelDRAW 12 installer (Wine 1.1.15)
by Gerold Jens Wucherpfennig 28 Oct '12

28 Oct '12

Hi, can someone help me with CorelDRAW 12: http://appdb.winehq.org/objectManager.php?sClass=version&iId=2390&iTestingI… Wine 1.5.15 vanilla without external DLLs Output from setup.exe in output.txt (only uniq lines saved; error code 67 on exit) vbscript errors (lines copied from output.txt): fixme:vbscript:parse_script parser failed on parsing L"1\r\nConst msiInstallStateAdvertise = 1\r\nConst msiInstallStateAbsent = 2\r\nConst msiInstallStateLocal = 3\r\nConst msiInstallStateSource = 4\r\nConst msiInstallStateDefault = 5\r\nDim nRemoveVBAEN:nRemoveVBAEN = 0\r\nDim nRemoveVBACZ:nRemoveVBACZ = 0\r\nDim nRemoveVBANL:nRemoveVBAPL = 0\r\n\r\nOn "... fixme:vbscript:parse_script parser failed on parsing L"1\r\nConst msiInstallStateAdvertise = 1\r\nConst msiInstallStateAbsent = 2\r\nConst msiInstallStateLocal = 3\r\nConst msiInstallStateSource = 4\r\nConst msiInstallStateDefault = 5\r\nDim nRemoveVBAEN:nRemoveVBAEN = 0\r\nDim nRemoveVBACZ:nRemoveVBACZ = 0\r\nDim nRemoveVBANL:nRemoveVBAPL = 0\r\n\r\nOn "... fixme:vbscript:parse_script parser failed on parsing L"1\r\nConst msiInstallStateAdvertise = 1\r\nConst msiInstallStateAbsent = 2\r\nConst msiInstallStateLocal = 3\r\nConst msiInstallStateSource = 4\r\nConst msiInstallStateDefault = 5\r\n\r\nOn Error Resume Next\r\n\r\nIf (IsFeatureSelected(\"FontNav\") = True) Then\r\n If ((Session.Property(\"PLLANG\""... fixme:vbscript:parse_script parser failed on parsing L"1\r\nConst msiInstallStateAdvertise = 1\r\nConst msiInstallStateAbsent = 2\r\nConst msiInstallStateLocal = 3\r\nConst msiInstallStateSource = 4\r\nConst msiInstallStateDefault = 5\r\n\r\nOn Error Resume Next\r\n\r\nIf (IsFeatureSelected(\"FontNav\") = True) Then\r\n If ((Session.Property(\"PLLANG\""... fixme:vbscript:parse_script parser failed on parsing L"1\r\nConst msiInstallStateAdvertise = 1\r\nConst msiInstallStateAbsent = 2\r\nConst msiInstallStateLocal = 3\r\nConst msiInstallStateSource = 4\r\nConst msiInstallStateDefault = 5\r\n\r\nOn Error Resume Next\r\n\t\r\nIf (IsFeatureExists(\"LocalInstallsOnly\")=\"1\") Then\r\n\tIf ((Session.Property(\""... Best Regards, Gerold

2 1

Re: [PATCH] Remove potential reference count races
by Nikolay Sivov 28 Oct '12

28 Oct '12

On 10/28/2012 04:59, max(a)mtew.isa-geek.net wrote: > From: Max TenEyck Woodbury <max(a)mtew.isa-geek.net> > > --- > dlls/shlwapi/thread.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/dlls/shlwapi/thread.c b/dlls/shlwapi/thread.c > index eb2c35d..43e0433 100644 > --- a/dlls/shlwapi/thread.c > +++ b/dlls/shlwapi/thread.c > @@ -157,7 +157,7 @@ static ULONG WINAPI threadref_AddRef(IUnknown *iface) > threadref * This = impl_from_IUnknown(iface); > > TRACE("(%p)\n", This); > - return InterlockedIncrement(This->ref); > + return InterlockedIncrement(&This->ref); > } > > static ULONG WINAPI threadref_Release(IUnknown *iface) > @@ -167,7 +167,7 @@ static ULONG WINAPI threadref_Release(IUnknown *iface) > > TRACE("(%p)\n", This); > > - refcount = InterlockedDecrement(This->ref); > + refcount = InterlockedDecrement(&This->ref); > if (!refcount) > HeapFree(GetProcessHeap(), 0, This); > Did you try to build this?

1 0

Re: [PATCH] Remove potential reference count races
by Nikolay Sivov 28 Oct '12

28 Oct '12

On 10/28/2012 04:59, max(a)mtew.isa-geek.net wrote: > From: Max TenEyck Woodbury <max(a)mtew.isa-geek.net> > > --- > dlls/jscript/engine.h | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > Take a look at http://source.winehq.org/git/wine.git/commit/79a30a4e0ae3a7ef640fa38e3a9f77…

1 0

Re: [PATCH] Potential reference count races
by Nikolay Sivov 28 Oct '12

28 Oct '12

On 10/28/2012 04:59, max(a)mtew.isa-geek.net wrote: > From: Max TenEyck Woodbury <max(a)mtew.isa-geek.net> > > I have been looking at the Microsoft COM and related documentations > and noticed that they emphatically recomend using the Interlocked... > functions when manipulating reference counts. I managed to set up a > search that showed where many of the reference count updates occur and > was somewhat surprised at how often this advice was not followed. It doesn't mean it always has to be followed. > > While I have not converted every reference count update to use the > Interlocked... functions, this set of patches fixes a fair number of > them. > > These are not associated with any particular bug report; they are simply > a general precausion against operations that are known to be associated > with race conditions. This precaution doesn't work in general. It's not enough to atomically update refcount to make an implementation thread safe. Also not everything is supposed to be thread safe in a first place. Changes like this: > - for (i=0;i<howmuch;i++) > + for (i=0;i<howmuch;++i) > TRACE("notify at %d to %p\n", > notify[i].dwOffset,notify[i].hEventNotify); are not helpful at all. > > The patches are independent of each other. They are NOT a series. >

1 0

Re: [PATCH] Remove potential reference count races.
by Nikolay Sivov 28 Oct '12

28 Oct '12

On 10/28/2012 04:59, max(a)mtew.isa-geek.net wrote: > From: Max TenEyck Woodbury <max(a)mtew.isa-geek.net> > > --- > dlls/gdi32/freetype.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/dlls/gdi32/freetype.c b/dlls/gdi32/freetype.c > index 6af977c..3379c6e 100644 > --- a/dlls/gdi32/freetype.c > +++ b/dlls/gdi32/freetype.c > @@ -3820,7 +3820,7 @@ static struct font_mapping *map_font_file( const char *name ) > { > if (mapping->dev == st.st_dev && mapping->ino == st.st_ino) > { > - mapping->refcount++; > + InterlockedIncrement(&mapping->refcount); > close( fd ); > return mapping; > } > @@ -3850,7 +3850,7 @@ error: > > static void unmap_font_file( struct font_mapping *mapping ) > { > - if (!--mapping->refcount) > + if (!InterlockedDecrement(&mapping->refcount)) > { > list_remove( &mapping->entry ); > munmap( mapping->data, mapping->size ); How does it help exactly and what is the problem with this code?

1 0

Re: [PATCH] wininet: TLS fallback mechanism (take2)
by Hiroshi Miura 27 Oct '12

27 Oct '12

Hi, My updated patch which is Message-ID: <5089BDA7.3040806(a)linux.com> is not properly treated in http://source.winehq.org/patches http://source.winehq.org/patches/data/91337 New one is removed, and the one that make compile error in older openssl is tagged as new. Hiroshi Miura wrote: > OpenSSL 1.0.x now support TLSv1.1 and TLSv1.2. > Sometimes TLSv1.1/1.2 negotiation fails bacause of combination > of server/client ssl library versions. > It fixes the error connecting Evernote server with evernote client. > http://bugs.winehq.org/show_bug.cgi?id=30598 > > It has also mechanism enable/disable TLSv1.1/1.2 by registry entry under > SYSTEM/CurrentControlSet/Control/SecurityProvider/SCHANNEL/Protocols/ > where is as same place as Windows7. > TLSv1.1/1.2 is enabled when the registry entry does not exist. > > Signed-off-by: Hiroshi Miura <miurahr(a)linux.com> > --- > dlls/wininet/netconnection.c | 169 ++++++++++++++++++++++++++++++++---------- > 1 file changed, 130 insertions(+), 39 deletions(-) > > diff --git a/dlls/wininet/netconnection.c b/dlls/wininet/netconnection.c > index c944ff3..9b5f7e7 100644 > --- a/dlls/wininet/netconnection.c > +++ b/dlls/wininet/netconnection.c <snip> > +#ifdef SSL_OP_NO_TLSv1_2 > +static long get_tls_option(void) { > + long tls_option; <snip> Hiroshi

1 0

Re: [PATCH 3/3] winmm: Call IsFormatSupported for WAVE_FORMAT_QUERY only.
by Andrew Eikum 26 Oct '12

26 Oct '12

Ran tests on all four backends and everything seems okay. On Fri, Oct 26, 2012 at 10:31:33AM +0200, Joerg-Cyril.Hoehle(a)t-systems.com wrote: > Hi, > > With the previous code in place, we can derive sane values for WAVE_FORMAT_QUERY. > > Given the amount of work and side-effects that the audio backends perform, better not > open them twice with IsFormatSupported then Initialize. Just use one or the other. > > Regards, > Jörg Höhle >

1 0

Re: ntdll: Do not execute callbacks past DeleteTimer(INVALID_HANDLE_VALUE)
by Marvin 26 Oct '12

26 Oct '12

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=22495 Your paranoid android. === WVISTAADM (32 bit sync) === sync.c:946: Test failed: DeleteTimerQueueTimer

2 1

Re: [PATCH(1/1)] wininet: TLS fallback mechanism
by Alexandre Julliard 25 Oct '12

25 Oct '12

"Hiroshi Miura(@osmf)" <miurahr(a)osmf.jp> writes: > OpenSSL 1.0.x now support TLSv1.1 and TLSv1.2. > Sometimes TLSv1.1/1.2 negotiation fails because of combination > of server/client ssl library versions. > It fixes the error connecting Evernote server with evernote client. > http://bugs.winehq.org/show_bug.cgi?id=30598 > > It has a mechanism to fallback to SSLv3/TLSv1 when fails with TLSv1.1/1.2. > > It has also mechanism enable/disable TLSv1.1/1.2 by registry entry under > SYSTEM/CurrentControlSet/Control/SecurityProvider/SCHANNEL/Protocols/ > where is as same place as Windows7. > TLSv1.1/1.2 is enabled when the registry entry does not exist. It doesn't build on older openssl: gcc -m32 -c -I. -I. -I../../include -I../../include -D__WINESRC__ -D_WINX32_ -D_REENTRANT -fPIC -Wall -pipe -fno-strict-aliasing -Wdeclaration-after-statement -Wstrict-prototypes -Wwrite-strings -gstabs+ -fno-omit-frame-pointer -Wpointer-arith -I/usr/local/include/freetype2 -I/usr/local/include -g -O2 -o netconnection.o netconnection.c netconnection.c: In function 'get_tls_option': netconnection.c:478: error: 'SSL_OP_NO_TLSv1_2' undeclared (first use in this function) netconnection.c:478: error: (Each undeclared identifier is reported only once netconnection.c:478: error: for each function it appears in.) netconnection.c:486: error: 'SSL_OP_NO_TLSv1_1' undeclared (first use in this function) netconnection.c: In function 'NETCON_secure_connect': netconnection.c:925: error: 'SSL_OP_NO_TLSv1_1' undeclared (first use in this function) netconnection.c:925: error: 'SSL_OP_NO_TLSv1_2' undeclared (first use in this function) make[1]: *** [netconnection.o] Error 1 -- Alexandre Julliard julliard(a)winehq.org

2 1

Re: [PATCH 3/5] d3dx9_36: Implement ID3DXFileImpl_CreateEnumObject with stubbed ID3DXFileEnumObject interface. (try 5)
by Nikolay Sivov 25 Oct '12

25 Oct '12

On 10/25/2012 10:34, Christian Costa wrote: > --- > dlls/d3dx9_36/xfile.c | 172 ++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 170 insertions(+), 2 deletions(-) > > diff --git a/dlls/d3dx9_36/xfile.c b/dlls/d3dx9_36/xfile.c > index ebaefaf..32c3edd 100644 > --- a/dlls/d3dx9_36/xfile.c > +++ b/dlls/d3dx9_36/xfile.c > @@ -32,12 +32,123 @@ typedef struct { > IDirectXFile *dxfile; > } ID3DXFileImpl; > > +typedef struct { > + ID3DXFileEnumObject ID3DXFileEnumObject_iface; > + LONG ref; > + IDirectXFileEnumObject *dxfile_enum_object; > +} ID3DXFileEnumObjectImpl; You remove that field in next patch anyway.

2 1

Re: [PATCH 4/5] d3dx9_36: Implement ID3DXFileEnumObjectImpl_GetChildren & ID3DXFileEnumObjectImpl_GetChild and add stubbed interface for ID3DXFileData. (try 4)
by Nikolay Sivov 25 Oct '12

25 Oct '12

On 10/25/2012 10:34, Christian Costa wrote: > > + > +static HRESULT WINAPI ID3DXFileDataImpl_Create(IDirectXFileObject *dxfile_object, ID3DXFileData **ret_iface) > +{ > + ID3DXFileDataImpl *object; > + > + TRACE("(%p, %p)\n", dxfile_object, ret_iface); > + > + *ret_iface = NULL; > + > + object = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*object)); > + if (!object) > + { > + ERR("Out of memory\n"); > + return E_OUTOFMEMORY; > + } > + > + object->ID3DXFileData_iface.lpVtbl = &ID3DXFileData_Vtbl; > + object->ref = 1; > + object->dxfile_object = (IDirectXFileData*)dxfile_object; > + > + *ret_iface = &object->ID3DXFileData_iface; > + > + return S_OK; > +} > + You don't need WINAPI for it. > > + ret = ID3DXFileDataImpl_Create((IDirectXFileObject*)data_object, &object->children[object->nb_children]); It seems you're casting it back and forth, is it possible to use proper type for data_object? > + { > + (&object->ID3DXFileEnumObject_iface)->lpVtbl->Release(&object->ID3DXFileEnumObject_iface); Hmm, what's wrong with object->ID3DXFileEnumObject_iface.lpVtbl->Release ?

2 1

Re: [PATCH 5/5] d3dx9_36: Implement ID3DXFileDataImpl_GetType. (try 5)
by Nikolay Sivov 25 Oct '12

25 Oct '12

On 10/25/2012 10:34, Christian Costa wrote: > --- > dlls/d3dx9_36/xfile.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/dlls/d3dx9_36/xfile.c b/dlls/d3dx9_36/xfile.c > index a1e0596..4c216da 100644 > --- a/dlls/d3dx9_36/xfile.c > +++ b/dlls/d3dx9_36/xfile.c > @@ -154,9 +154,19 @@ static HRESULT WINAPI ID3DXFileDataImpl_Unlock(ID3DXFileData *iface) > > static HRESULT WINAPI ID3DXFileDataImpl_GetType(ID3DXFileData *iface, GUID *guid) > { > - FIXME("(%p)->(%p): stub\n", iface, guid); > + ID3DXFileDataImpl *This = impl_from_ID3DXFileData(iface); > + const GUID *dxfile_guid; > + HRESULT ret; > > - return E_NOTIMPL; > + TRACE("(%p)->(%p)\n", iface, guid); > + > + ret = IDirectXFileData_GetType(This->dxfile_object, &dxfile_guid); > + if (ret != S_OK) > + return ret; > + > + *guid = *dxfile_guid; > + > + return S_OK; > } > Why not simply forward to IDirectXFileData here? What is all the surrounding code for?

2 1

Add tests fo D3DXSHRotateZ
by Nozomi Kodama 25 Oct '12

25 Oct '12

Hello, Is this patch not good enough to be commited? Nozomi

1 0

Re: [PATCH 1/5] d3dx9_36: Implement D3DXFileCreate. (try 3)
by Dmitry Timoshkov 25 Oct '12

25 Oct '12

Christian Costa <titan.costa(a)gmail.com> wrote: > +static HRESULT WINAPI ID3DXFileImpl_QueryInterface(ID3DXFile *iface, REFIID riid, void **ret_iface) > +{ > + TRACE("(%p)->(%s, %p)\n", iface, debugstr_guid(riid), ret_iface); > + > + if (IsEqualGUID(riid, &IID_IUnknown) || > + IsEqualGUID(riid, &IID_ID3DXFile)) > + { > + iface->lpVtbl->AddRef(iface); Isn't there an appropriate xxx_AddRef() macro? > + *ret_iface = iface; > + return S_OK; > + } > + > + ERR("(%p)->(%s, %p), not found\n", iface, debugstr_guid(riid), ret_iface); FIXME seems more appropriate here. > + *ret_iface = NULL; > + return E_NOINTERFACE; > +} ... > +HRESULT WINAPI D3DXFileCreate(ID3DXFile **dxfile) > +{ > + ID3DXFileImpl *object; > + > + TRACE("(%p)\n", dxfile); > + > + object = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*object)); What's the reason for HEAP_ZERO_MEMORY here? > + if (!object) > + { > + ERR("Out of memory\n"); > + return E_OUTOFMEMORY; > + } The ERR() is useless here, just return E_OUTOFMEMORY in such situations. -- Dmitry.

5 13

[PATCH(0/1)] wininet: TLS fallback mechanism
by Hiroshi Miura(＠osmf) 24 Oct '12

24 Oct '12

Hi, Evernote windows client on Wine/OpenSSL 1.0.1/Ubuntu12.04/12.10 cannot connect its server. It is caused by server is only support TLS1.0/SSL3.0, but client ask TLS1.1/1.2 and then server returns that session is fails. A wininet in Microsoft Windows7 implementation has a behavior to re-try by TLS1.0 after TLS1.2 fails and got FIN from server. This patch enabled fall back mechanism to SSLv3/TLSv1. It is also a response for Alexandre's suggestion. >> How do you think whether we should implement similar way or not? >> If we adopt similar fall back mechanism, compatibility may become well, >> but is it good behavior? > > If it's possible to implement some sort of automatic fall back, that > would certainly be preferable. > > -- > Alexandre Julliard Hiroshi Hiroshi Miura (1): wininet: TLS fallback mechanism dlls/wininet/netconnection.c | 163 ++++++++++++++++++++++++++++++++---------- 1 file changed, 124 insertions(+), 39 deletions(-) -- 1.7.9.5

2 1

windowscodecs questions.
by Max TenEyck Woodbury 24 Oct '12

24 Oct '12

Background: I am trying to improve the performance/reliability of GuildWars2 under wine. In particular, PNG handling seems very slow, so I am looking at windowscodecs and I seem to be misunderstanding some things. I have read the PNG spec. First: The CRC check. The spec says that a 'chunk' contains a four byte size of data field, a four byte 'chunk identifier', the data and a CRC32 of the data. I found some code that reads a generic chunk. I see where the size and type are read, where a correctly sized record is allocated and where the data is read into the record. There is a 'FIXME' that asks about checking the CRC, but no code to actually read or check the CRC. I added code to read and check the CRC to my working copy of the git tree. That threw off the block synchronization, so I think there is code someplace else that either checks or skips the CRC, but I have not found it after a few hours searching for it. An indication on where to look and other advice would be very helpful at this time. Second: At least two of the methods GuildWars2 wants to use are stubbed. To implement those methods, the frame section of the WIC object should contain an array (or something with similar properties) of pointers to chunks. From what I have read about WIC, other formats could use a similar array. Some could even use an array of pointers to frames. Before I go messing with that level of change, I think I should listen to other peoples opinions of the subject. Third: On a very broad level, the whole OLE implementation seems to be very messed up. In particular, the usual practice for doing inheritance in C does not seem to be being used. That practice is to have the elements common to the base and extending object be placed at the beginning of the implementing data structures. While the member names need not be the same in all instances, the function, type and order of the common elements must be the same for economical implementation. This has been done for the basic object; all start with a pointer to the 'vtable'. What I think should follow is the critical section lock structure and, for 'IUnknown', the reference count. What I see is that these common elements are placed more or less at random in each extended object. Is there any reason, other than inertia, that at least these two fields should not be moved to the beginning of implementing objects. This can be done one object at a time and would allow the changed objects to use a common implementation of 'AddRef' at least, and another common routine or macro that handles the critical part of 'Release'.

2 2

[PATCH 5/6] dsound: rework ugly mixer logic
by Joerg-Cyril.Hoehle＠t-systems.com 24 Oct '12

24 Oct '12

Hi, Maarten Lankhorst wrote: >only oss4 and (maybe, just guessing?) coreaudio don't support float MacOS' CoreAudio supports floats. Actually, given that it's an entire audio pipeline like quartz, I bet it even prefers floats. >I also refer you to GetStreamLatency where I was getting the period from, >Please take it from me: IT'S OK TO FAIL THOSE MMDEVAPI PERIOD TESTS, I agree that some tests are extremely strict and that a "good enough" driver could work extremely well in practice despite failing some tests. However, please specify exactly which tests you have in mind so that we all talk about the same set. >BUT PLEASE DON'T LIE ABOUT THE HARDWARE CAPABILITIES REGARDING >TO MINIMUM PERIOD, DEFAULT PERIOD AND STREAM LATENCY. The reasoning is as follows: First, tests show that native exhibits a 10ms period, hence we can assume that every PC SW has been tested with that period only. Using another period in Wine might reveal bugs in the apps, but users would blame Wine. This is why I've been arguing no later than last week in bug #28622, comment #58 that Wine should fully decouple ALSA/OSS periods from the published 10ms mmdevapi periods. OTOH, winecoreaudio has been using 20ms periods since day 1 in 2011. I'm not aware of bugs in bugzilla caused by that deviation. Second, the shared mode audio engine IMHO presents an abstraction over any particular HW, providing the same behviour on all machines and eliminating the need for app X to talk to or work around bugs in sound driver Y. Thus I wonder why you talk about hardware capabilities at all. IOW the shared mode caps are: fixed period=10ms and unlimited buffer (MSDN says 2s). a) Minimum Period Why do you care? It's only relevant in exclusive mode. I'd say that minimum = shared mode default period is a reasonable value. And if default=10ms, minimum = 5ms seems ok if it has been tested and works well. OTOH, 200 periods and wakeups per second may be unrealistic as we alreadty struggle to sustain 100. Do you know an app that uses mmdevapi in exclusive mode? b) Default Period == shared mode audio engin period The target is 10ms, see above. Yet we could make exemptions: for instance, we might observe that all apps using mmdevapi use it via the XAudio2 libraries and that XA2 is well debugged and would bear a larger period, e.g. 50ms. There should be a registry setting for users to play with. I'm not happy with Wine's DSound+WinMM->mmdevapi. That may be a good move for MS, but it's a bad move for Wine. Wine's audio drivers don't bang the HW, they have huge SW stacks like PA or Jack in their back. c) Stream Latency Bear in mind that the mmdevapi stream latency is a static property of an audio SW transformation graph. It does not vary if some buffer is full or empty. It is unlike snd_pcm_delay. I believe winealsa outputs a reasonable stream latency. One may argue that it is too low: as winealsa always buffers 3 periods internally the latency should count those. I kept it low because native's mmdevapi is reported to introduce 30-40ms delays yet its GetStreamLatency ignores that and reports values like 10.6666ms. Perhaps we are wrong about GetStreamLatency. Perhaps it's always a local call to one element of the audio transformation graph and should never sum other elements itself. Perhaps the app should use some enumeration facility to walk the audio graph and sum each element's latency? Perhaps there's some native graph element that would report said 30-40ms latency? Regards, Jörg Höhle

4 4

vcomp: single-threaded implementation of _vcomp_fork (try 6)
by Joerg-Cyril.Hoehle＠t-systems.com 24 Oct '12

24 Oct '12

Hi, Alexandre Julliard wrote: > +void WINAPIV _vcomp_fork(BOOL ifval, int nargs, void *wrapper, ...) > + __ms_va_list valist; >it's ugly to use varargs only as a hack to get a pointer to the first >argument. Would taking the address of &wrapper (as in asm("...","r=&wrapper") or any C code like "void* stack = &wrapper") suffice to tell the x64 compiler to save register parameters into their allocated stack locations? My belief was that this is specifically the magic performed when the compiler sees a [__ms_]va_list. You seem to imply that va_list is superfluous and that taking the address of any parameter (not necessarily &wrapper but &ifval or &nargs too) would achieve the same effect: all 4 registers rcx, rdx, r8 and r9 get saved to the stack such that the & abstraction works even when the user performs pointer arithmetic. This is the effect that Dan Kegel needs for vcomp. In such a case he would use &wrapper, bypass va_list and correct the stack-relative offsets in the x86 and x64 assembly code by one pointer unit. Regards, Jörg Höhle

3 3

Re: [PATCH] user32: Fix error handling in MapWindowPoints, ClientToScreen and ScreenToClient and add tests for them. (try 3)
by Dmitry Timoshkov 24 Oct '12

24 Oct '12

Christian Costa <titan.costa(a)gmail.com> wrote: > BOOL WINAPI ClientToScreen( HWND hwnd, LPPOINT lppnt ) > { > + DWORD error = GetLastError(); > + > + if (!hwnd) > + { > + SetLastError( ERROR_INVALID_WINDOW_HANDLE ); > + return FALSE; > + } > + > + SetLastError( 0xdeadbeef ); > MapWindowPoints( hwnd, 0, lppnt, 1 ); > + > + if (GetLastError() != 0xdeadbeef) > + return FALSE; > + > + SetLastError(error); > return TRUE; > } As been said before these games with saving/restoring last error value are broken. -- Dmitry.

3 3

Re: [PATCH 1/2] user32: Remove redundant sizeof check.
by Ken Thomases 24 Oct '12

24 Oct '12

On Oct 24, 2012, at 3:24 AM, Michael Stefaniuc wrote: > @@ -1330,9 +1330,6 @@ static HICON CURSORICON_LoadFromFile( LPCWSTR filename, > } > > dir = (const CURSORICONFILEDIR*) bits; > - if ( filesize < sizeof(*dir) ) > - goto end; > - > if ( filesize < (sizeof(*dir) + sizeof(dir->idEntries[0])*(dir->idCount-1)) ) > goto end; > That doesn't seem redundant to me. It's not safe to access dir->idCount if the file isn't known to be big enough. Regards, Ken

2 1

Re: d3dx9_36/tests: Fix sizeof to be more portable (coverity)
by Henri Verbeet 24 Oct '12

24 Oct '12

On 24 October 2012 02:05, André Hentschel <nerv(a)dawncrow.de> wrote: > face_remap = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, > - tc[i].num_faces*sizeof(face_remap)); > + tc[i].num_faces*sizeof(DWORD*)); > I'm don't know what Coverity actually says about this, but this looks just as wrong to me. It's probably supposed to be "sizeof(*face_remap)".

2 1

Re: [PATCH 3/3] ntdll: Add NULL test for NtQuerySystemInformation with SystemProcessInformation.
by Marvin 23 Oct '12

23 Oct '12

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=22435 Your paranoid android. === W7PROX64 (32 bit info) === info.c:297: Test failed: Expected STATUS_ACCESS_VIOLATION got c0000017 === TEST64_W7SP1 (32 bit info) === info.c:297: Test failed: Expected STATUS_ACCESS_VIOLATION got c0000017

1 0

Re: [PATCH 3/3] mshtml: Added HTML comments in VBScript code tests
by Marvin 23 Oct '12

23 Oct '12

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=22431 Your paranoid android. === WINEBUILD (build) === Patch failed to apply

1 0

Re: [PATCH 1/3] ntdll: Implement nanosecond precision file time storage (try 2).
by Alexandre Julliard 23 Oct '12

23 Oct '12

"Erich E. Hoover" <ehoover(a)mymail.mines.edu> writes: > @@ -2048,17 +2048,26 @@ NTSTATUS WINAPI NtSetInformationFile(HANDLE handle, PIO_STATUS_BLOCK io, > > if (info->LastAccessTime.QuadPart || info->LastWriteTime.QuadPart) > { > +#ifdef HAVE_FUTIMENS > + struct timespec tv[2]; > +#else > struct timeval tv[2]; > +#endif > + int ret; > > if (!info->LastAccessTime.QuadPart || !info->LastWriteTime.QuadPart) > { > - > - tv[0].tv_sec = tv[0].tv_usec = 0; > - tv[1].tv_sec = tv[1].tv_usec = 0; > + memset(&tv, 0x0, sizeof(tv)); > if (!fstat( fd, &st )) > { > tv[0].tv_sec = st.st_atime; > tv[1].tv_sec = st.st_mtime; > +#if defined(HAVE_FUTIMENS) && defined(HAVE_STRUCT_STAT_ST_ATIM) > + tv[0].tv_nsec = st.st_atim.tv_nsec; > +#endif > +#if defined(HAVE_FUTIMENS) && defined(HAVE_STRUCT_STAT_ST_MTIM) > + tv[1].tv_nsec = st.st_mtim.tv_nsec; > +#endif You want UTIME_OMIT. Also please try to cut down on the number of #ifdefs. -- Alexandre Julliard julliard(a)winehq.org

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

2003

2002

2001

wine-devel