February 2015 - Wine-Devel - list.winehq.org

Re: [1/3] winmm/tests: Add tests for PlaySound().

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 https://testbot.winehq.org/JobDetails.pl?Key=11693 Your paranoid android. === w2000pro (32 bit wave) === wave.c:1679: Test failed: PlaySound succeeded, got 1 === wxppro (32 bit wave) === wave.c:1679: Test failed: PlaySound succeeded, got 1 === w2003std (32 bit wave) === wave.c:1679: Test failed: PlaySound succeeded, got 1 === wvistau64 (32 bit wave) === wave.c:1662: Test failed: PlaySound failed, got 0 wave.c:1666: Test failed: PlaySound failed, got 0 wave.c:1674: Test failed: PlaySound failed, got 0 === w2008s64 (32 bit wave) === wave.c:1662: Test failed: PlaySound failed, got 0 wave.c:1666: Test failed: PlaySound failed, got 0 wave.c:1674: Test failed: PlaySound failed, got 0 === wvistau64 (64 bit wave) === wave.c:1662: Test failed: PlaySound failed, got 0 wave.c:1666: Test failed: PlaySound failed, got 0 wave.c:1674: Test failed: PlaySound failed, got 0 === w2008s64 (64 bit wave) === wave.c:1662: Test failed: PlaySound failed, got 0 wave.c:1666: Test failed: PlaySound failed, got 0 wave.c:1674: Test failed: PlaySound failed, got 0

Feb. 24, 2015

Re: [3/5] comctl32: Implement TaskDialogIndirect as a custom dialog box (try 2)

by Joachim Priesner

Am Dienstag, 24. Februar 2015 schrieb Alexandre Julliard: > Joachim Priesner <joachim.priesner(a)web.de> writes: > > > Am Dienstag, 24. Februar 2015 schrieb Alexandre Julliard: > >> There shouldn't be any need to distinguish. If you get 0 then there's no > >> defined string and you use the fallback. > > > > Windows makes that distinction though: > > - String of length zero: TaskDialogIndirect succeeds, but no text is displayed for that parameter > > - Invalid resource: TaskDialogIndirect fails with ERROR_RESOURCE_NAME_NOT_FOUND > > This is broken behavior, I don't see much point in reproducing this > (unless of course you can find an app that depends on it). I'd argue that a) the goal of "bug-for-bug" compatibility is really easy to achieve in this case and b) it saves debugging time in the long run should there be an application that does rely on it (even if only accidentally). But I'm fine with just ignoring invalid string resources (instead of returning failure), which should be the "safe" variant.

Feb. 24, 2015

Re: [3/5] comctl32: Implement TaskDialogIndirect as a custom dialog box (try 2)

by Alexandre Julliard

Joachim Priesner <joachim.priesner(a)web.de> writes: > Am Dienstag, 24. Februar 2015 schrieb Alexandre Julliard: >> There shouldn't be any need to distinguish. If you get 0 then there's no >> defined string and you use the fallback. > > Windows makes that distinction though: > - String of length zero: TaskDialogIndirect succeeds, but no text is displayed for that parameter > - Invalid resource: TaskDialogIndirect fails with ERROR_RESOURCE_NAME_NOT_FOUND This is broken behavior, I don't see much point in reproducing this (unless of course you can find an app that depends on it). -- Alexandre Julliard julliard(a)winehq.org

Feb. 24, 2015

Re: [3/5] comctl32: Implement TaskDialogIndirect as a custom dialog box (try 2)

by Joachim Priesner

Am Dienstag, 24. Februar 2015 schrieb Alexandre Julliard: > Nikolay Sivov <bunglehead(a)gmail.com> writes: > > > On 24.02.2015 17:37, Joachim Priesner wrote: > >> Am Dienstag, 24. Februar 2015 schrieb Nikolay Sivov: > >>> On 23.02.2015 15:42, Joachim Priesner wrote: > >>>> + if (IS_INTRESOURCE(text)) > >>>> + { > >>>> + SetLastError(S_OK); > >>>> + LoadStringW(instance, LOWORD(text), (LPWSTR)result, 0); > >>>> + return HRESULT_FROM_WIN32(GetLastError()); > >>>> + } > >>> > >>> You don't need SetLastError(), especially not with HRESULT value. And > >>> GetLastError() should only be used when LoadStringW() fails. > >> > >> Thanks for the feedback, again. The rationale behind SetLastError is that the following three cases can occur: > >> 1) LoadStringW succeeds, string length > 0 => return value > 0 > >> 2) LoadStringW succeeds, string length == 0 => return value 0 > >> 3) LoadStringW fails => return value 0, error code available via GetLastError. > >> > >> Because LastError is not modified in case 2), I have to set it to S_OK beforehand in order to distinguish between 2) and 3). Does that make sense? > >> > > > > Ok, what happens on windows if resource length is 0? If it still works > > and text is simply empty you can detect a failure another way - by > > testing returned resource pointer (I'd expect NULL if resource id was > > not found), and in that case use last error. > > There shouldn't be any need to distinguish. If you get 0 then there's no > defined string and you use the fallback. Windows makes that distinction though: - String of length zero: TaskDialogIndirect succeeds, but no text is displayed for that parameter - Invalid resource: TaskDialogIndirect fails with ERROR_RESOURCE_NAME_NOT_FOUND

Feb. 24, 2015

Re: [PATCH] uxtheme: support images in GetThemeBackgroundRegion

by Alexandre Julliard

Mark Harmstone <hellas(a)burntcomma.com> writes: > + dc = CreateCompatibleDC(NULL); > + if (!dc) { > + WARN("CreateCompatibleDC failed\n"); > + DeleteObject(hrgn); > + return E_FAIL; > + } > + > + bmp = CreateBitmap(rect->right - rect->left, rect->bottom - rect->top, 1, 32, NULL); You need a DIB for this, not a device bitmap. -- Alexandre Julliard julliard(a)winehq.org

Feb. 24, 2015

Re: [3/5] comctl32: Implement TaskDialogIndirect as a custom dialog box (try 2)

by Joachim Priesner

> Ok, what happens on windows if resource length is 0? If it still works > and text is simply empty Yes, this is the case. > you can detect a failure another way - by > testing returned resource pointer (I'd expect NULL if resource id was > not found), and in that case use last error. No, if the resource is not found, lpBuffer remains unchanged on Windows and isn't set to NULL. I could of course set it to NULL beforehand instead of setting SetLastError. But the current solution uses less code than that. (Except of course I should use "SetLastError(ERROR_SUCCESS)" instead of S_OK, as you noted.) Which one do you prefer?

Feb. 24, 2015

Re: [3/5] comctl32: Implement TaskDialogIndirect as a custom dialog box (try 2)

by Alexandre Julliard

Nikolay Sivov <bunglehead(a)gmail.com> writes: > On 24.02.2015 17:37, Joachim Priesner wrote: >> Am Dienstag, 24. Februar 2015 schrieb Nikolay Sivov: >>> On 23.02.2015 15:42, Joachim Priesner wrote: >>>> + if (IS_INTRESOURCE(text)) >>>> + { >>>> + SetLastError(S_OK); >>>> + LoadStringW(instance, LOWORD(text), (LPWSTR)result, 0); >>>> + return HRESULT_FROM_WIN32(GetLastError()); >>>> + } >>> >>> You don't need SetLastError(), especially not with HRESULT value. And >>> GetLastError() should only be used when LoadStringW() fails. >> >> Thanks for the feedback, again. The rationale behind SetLastError is that the following three cases can occur: >> 1) LoadStringW succeeds, string length > 0 => return value > 0 >> 2) LoadStringW succeeds, string length == 0 => return value 0 >> 3) LoadStringW fails => return value 0, error code available via GetLastError. >> >> Because LastError is not modified in case 2), I have to set it to S_OK beforehand in order to distinguish between 2) and 3). Does that make sense? >> > > Ok, what happens on windows if resource length is 0? If it still works > and text is simply empty you can detect a failure another way - by > testing returned resource pointer (I'd expect NULL if resource id was > not found), and in that case use last error. There shouldn't be any need to distinguish. If you get 0 then there's no defined string and you use the fallback. -- Alexandre Julliard julliard(a)winehq.org

Feb. 24, 2015

Re: [3/5] comctl32: Implement TaskDialogIndirect as a custom dialog box (try 2)

by Nikolay Sivov

On 24.02.2015 17:37, Joachim Priesner wrote: > Am Dienstag, 24. Februar 2015 schrieb Nikolay Sivov: >> On 23.02.2015 15:42, Joachim Priesner wrote: >>> + if (IS_INTRESOURCE(text)) >>> + { >>> + SetLastError(S_OK); >>> + LoadStringW(instance, LOWORD(text), (LPWSTR)result, 0); >>> + return HRESULT_FROM_WIN32(GetLastError()); >>> + } >> >> You don't need SetLastError(), especially not with HRESULT value. And >> GetLastError() should only be used when LoadStringW() fails. > > Thanks for the feedback, again. The rationale behind SetLastError is that the following three cases can occur: > 1) LoadStringW succeeds, string length > 0 => return value > 0 > 2) LoadStringW succeeds, string length == 0 => return value 0 > 3) LoadStringW fails => return value 0, error code available via GetLastError. > > Because LastError is not modified in case 2), I have to set it to S_OK beforehand in order to distinguish between 2) and 3). Does that make sense? > Ok, what happens on windows if resource length is 0? If it still works and text is simply empty you can detect a failure another way - by testing returned resource pointer (I'd expect NULL if resource id was not found), and in that case use last error.

Feb. 24, 2015

Re: [3/5] comctl32: Implement TaskDialogIndirect as a custom dialog box (try 2)

by Joachim Priesner

Am Dienstag, 24. Februar 2015 schrieb Nikolay Sivov: > On 23.02.2015 15:42, Joachim Priesner wrote: > > + if (IS_INTRESOURCE(text)) > > + { > > + SetLastError(S_OK); > > + LoadStringW(instance, LOWORD(text), (LPWSTR)result, 0); > > + return HRESULT_FROM_WIN32(GetLastError()); > > + } > > You don't need SetLastError(), especially not with HRESULT value. And > GetLastError() should only be used when LoadStringW() fails. Thanks for the feedback, again. The rationale behind SetLastError is that the following three cases can occur: 1) LoadStringW succeeds, string length > 0 => return value > 0 2) LoadStringW succeeds, string length == 0 => return value 0 3) LoadStringW fails => return value 0, error code available via GetLastError. Because LastError is not modified in case 2), I have to set it to S_OK beforehand in order to distinguish between 2) and 3). Does that make sense?

Feb. 24, 2015

Re: [1/2] ntdll/tests: Add tests for TpSimpleTryPost function.

by Alexandre Julliard

Sebastian Lackner <sebastian(a)fds-team.de> writes: > + /* post the callback using new threadpool */ > + userdata = 0; > + memset(&environment, 0, sizeof(environment)); > + environment.Version = 1; > + environment.Pool = pool; > + status = pTpSimpleTryPost(simple_cb, &userdata, &environment); > + ok(!status, "TpSimpleTryPost failed with status %x\n", status); > + while (userdata == 0) Sleep(10); > + ok(userdata == 1, "expected userdata = 1, got %u\n", userdata); Please try to use synchronization objects instead of sleeps. > + /* make sure worker threads have terminated */ > + Sleep(100); Same here. Why do you need that? -- Alexandre Julliard julliard(a)winehq.org

Feb. 24, 2015