August 2017 - Wine-Devel - list.winehq.org

Re: Invitations to WineConf

by Rosanne DiMesio

On Mon, 7 Aug 2017 09:50:45 -0500 Jeremy White <jwhite(a)codeweavers.com> wrote: > > Roseanne, I'm particularly hoping you can reach out to the wine users in > some way. A post on the forums? Could we / should we do a big news item > so it's on the main page? > I'd do both. The forum announcement should come from you rather than me. Maybe make the main page announcement a WWN article? My big question is: what are you planning for users? Users who don't live near enough to just drop in will want to know what's on the agenda before deciding. > I've asked PlayOnLinux folks as well as the Wineskin folks to come. > Anyone else? > AppDB admins and maintainers? Most won't be interested, but some long-time, very active ones might. It's very easy to send an email to all of them through the AppDB. -- Rosanne DiMesio dimesio(a)earthlink.net

Aug. 7, 2017

Invitations to WineConf

by Jeremy White

Hey folks, Last year we said that we were going to try to broaden our invitations to the Wine conference to try to draw in more voices. This means potentially asking users, and making personal appeals to people that we would like to have at the conference. I plan to write a few personal notes extending invitations to folks that I've thought of (or have had suggested to me). James in our office is also going to reach out. But I'd like to ask everyone to help with that process. Roseanne, I'm particularly hoping you can reach out to the wine users in some way. A post on the forums? Could we / should we do a big news item so it's on the main page? I've asked PlayOnLinux folks as well as the Wineskin folks to come. Anyone else? Cheers, Jeremy

Aug. 7, 2017

Re: [PATCH 2/2] wined3d: do not call queue_is_empty in wined3d_cs_mt_finish.

by Henri Verbeet

On 4 August 2017 at 10:21, Stefan Dösinger <stefan(a)codeweavers.com> wrote: > +/* This function assumes that it is called from the command stream thread. */ > static BOOL wined3d_cs_queue_is_empty(const struct wined3d_cs_queue *queue) It should probably have a wined3d_from_cs() then.

Aug. 7, 2017

Re: [PATCH v2] shell32: Implement SHCreateItemInKnownFolder.

by Huw Davies

On Fri, Aug 04, 2017 at 02:54:15PM +0800, Jactry Zeng wrote: > diff --git a/dlls/shell32/tests/shlfolder.c b/dlls/shell32/tests/shlfolder.c > index 9e7127dfea..b77619d428 100644 > --- a/dlls/shell32/tests/shlfolder.c > +++ b/dlls/shell32/tests/shlfolder.c > @@ -2519,6 +2521,120 @@ static void test_SHCreateShellItem(void) > else > win_skip("No SHCreateItemFromIDList\n"); > > + /* SHCreateItemInKnownFolder */ > + if(pSHCreateItemInKnownFolder) > + { > + WCHAR *desktop_path; > + WCHAR testfile_path[MAX_PATH] = {0}; > + HANDLE file; > + WCHAR *displayname = NULL; > + int order; > + LPITEMIDLIST pidl_desktop_testfile = NULL; > + > + if(0) > + { > + /* crashes on Windows */ > + pSHCreateItemInKnownFolder(NULL, 0, NULL, NULL, NULL); > + pSHCreateItemInKnownFolder(&FOLDERID_Desktop, 0, NULL, NULL, NULL); > + pSHCreateItemInKnownFolder(&FOLDERID_Desktop, 0, testfileW, &IID_IShellItem, NULL); > + } > + > + shellitem = (void*)0xdeadbeef; > + ret = pSHCreateItemInKnownFolder(&FOLDERID_Desktop, 0, NULL, &IID_IShellItem, > + (void**)&shellitem); > + ok(ret == S_OK, "SHCreateItemInKnownFolder failed: 0x%08x.\n", ret); > + ok(shellitem != NULL, "shellitem was %p.\n", shellitem); > + if(SUCCEEDED(ret)) > + { > + shellitem2 = (void*)0xdeadbeef; > + ret = pSHCreateShellItem(NULL, NULL, pidl_desktop, &shellitem2); > + ok(SUCCEEDED(ret), "SHCreateShellItem returned %x\n", ret); > + if(SUCCEEDED(ret)) > + { > + ret = IShellItem_Compare(shellitem, shellitem2, 0, &order); > + ok(ret == S_OK, "IShellItem_Compare failed: 0x%08x\n", ret); > + ok(!order, "order got wrong value: %d\n", order); > + IShellItem_Release(shellitem2); > + } > + IShellItem_Release(shellitem); > + } > + > + /* Test with a non-existent file */ > + shellitem = (void*)0xdeadbeef; > + ret = pSHCreateItemInKnownFolder(&FOLDERID_Desktop, 0, testfileW, &IID_IShellItem, > + (void**)&shellitem); > + ok(ret == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND), "Expected 0x%08x but SHCreateItemInKnownFolder return: 0x%08x.\n", > + HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND), ret); > + ok(shellitem == NULL, "shellitem was %p.\n", shellitem); > + > + SHGetKnownFolderPath(&FOLDERID_Desktop, 0, NULL, &desktop_path); It looks like SHGetKnownFolderPath is not available on WinXP, so you'll have to load it dynamically. OTOH I think SHCreateItemInKnownFolder() is available on all the platforms that we care about, so you can static link to that one. Huw.

Aug. 7, 2017

Re: winebuild: Improved position independent code generation

by Martin Storsjö

On Sun, 6 Aug 2017, André Hentschel wrote: > Android only accepts proper position independent binaries, but until now we generated code which led to a TEXTREL flag > > Signed-off-by: André Hentschel <nerv(a)dawncrow.de> > --- > tools/winebuild/import.c | 34 ++++++++++++++-------------------- > tools/winebuild/spec32.c | 4 ++-- > 2 files changed, 16 insertions(+), 22 deletions(-) Signed-off-by: Martin Storsjo <martin(a)martin.st> // Martin

Aug. 6, 2017

Re: D3D8 Software Vertex Processing

by Jan Havran

On 6.8.2017 20:36, Stefan Dösinger wrote: > On 06/08/17 17:19, Jan Havran wrote: >> Can I simply edit some code in Wine (some D3D flag) >> so the game will think that it use SW vertexes, so after checking some >> flags by game it will not jump into some unfinished branches for HW >> vertexes? I just want to test if it is really caused by this. From >> application view, what is difference between using SW and HW vertexes, >> do you think that using HW vertexes with code done for SW vertexes will >> work? > This behavior should already be in place. If the app requsts a software > vertex processing device and asks us for the device description later we > should report that this is a software vertex processing device. We even > do some software-like things if the app wants SWVP, like ignore > D3DLOCK_DISCARD on buffers. > > The game may check some derived properties though and expect some > capability that SWVP has, but HWVP does not. The most likely case is > support for more than 256 vertex shader constants. Modern GPUs support > much more than that in OpenGL, but not in d3d8/9 on Windows. Because of > this we filter this in dlls/d3d8/device.c, d3dcaps_from_wined3dcaps(). > Look for D3D8_MAX_VERTEX_SHADER_CONSTANTF at the end. It is likely that > this restriction is not in place on Windows with SWVP. You can try to > just disable this line. Wine is big magic to me right now, so thank you for your tips, I will try to test that. > > Another possible culprit is fixed function vertex blending using matrix > indices. You can try to set MaxVertexBlendMatrices and > MaxVertexBlendMatrixIndex to 4 and 256 respectively (check on Windows if > those values are accurate, I might misremember them). This functionality > is unimplemented in Wine, so expect rendering problems if the game uses > this. It should be possible to implement this feature in our fixed > function replacement shader though. Ok, I will test it. Unfrotunately I didnt find piece of code where the game decide which vertex processing it will use yet - maybe I will install Windows so I will be able to compare execution of branches between Windows and Linux. > The bug report doesn't have a crash dump. The similarity to windows HWVP > might mislead you. Try to compare your crash to the one of windows HWVP. > in particular, see if the crash address is the same. Well, I was talking about my issue with Brchi and we came to the conclusion that it was crashing in the same function for both Linux (Wine) and Windows (with enabled HWVP). Unfortunately, there is version of his util without HWVP enabled (which doesnt work in Wine as I said, because the game use branches for HWVP here) and after that version with HWVP enabled only, but with fixed game code (by memory patching) so there is no way how can I test it on Windows (because I dont have version of the util where it is crashing for Windows, since there is no version with HWVP enabled and without fix of this issue at the same time). Anyway, this game has plenty of problems on newer Windows, but I never heard about this issue even on Windows 10. I will try some tests you described and report back as soon as possible. Thanks for your time.

Aug. 6, 2017

Re: D3D8 Software Vertex Processing

by Stefan Dösinger

On 06/08/17 17:19, Jan Havran wrote: > Can I simply edit some code in Wine (some D3D flag) > so the game will think that it use SW vertexes, so after checking some > flags by game it will not jump into some unfinished branches for HW > vertexes? I just want to test if it is really caused by this. From > application view, what is difference between using SW and HW vertexes, > do you think that using HW vertexes with code done for SW vertexes will > work? This behavior should already be in place. If the app requsts a software vertex processing device and asks us for the device description later we should report that this is a software vertex processing device. We even do some software-like things if the app wants SWVP, like ignore D3DLOCK_DISCARD on buffers. The game may check some derived properties though and expect some capability that SWVP has, but HWVP does not. The most likely case is support for more than 256 vertex shader constants. Modern GPUs support much more than that in OpenGL, but not in d3d8/9 on Windows. Because of this we filter this in dlls/d3d8/device.c, d3dcaps_from_wined3dcaps(). Look for D3D8_MAX_VERTEX_SHADER_CONSTANTF at the end. It is likely that this restriction is not in place on Windows with SWVP. You can try to just disable this line. Another possible culprit is fixed function vertex blending using matrix indices. You can try to set MaxVertexBlendMatrices and MaxVertexBlendMatrixIndex to 4 and 256 respectively (check on Windows if those values are accurate, I might misremember them). This functionality is unimplemented in Wine, so expect rendering problems if the game uses this. It should be possible to implement this feature in our fixed function replacement shader though. The bug report doesn't have a crash dump. The similarity to windows HWVP might mislead you. Try to compare your crash to the one of windows HWVP. in particular, see if the crash address is the same.

Aug. 6, 2017

D3D8 Software Vertex Processing

by Jan Havran

Hello, i am trying to fix #9337 [1] for VietCong game (game is crashing during loading 3D objects) for which exists community util which fixes it. This util was made to fix issues for VC on newer Windows - fixing this bug in Wine was just side effect. Anyway, i have talked to creator of this util (Brchi from The Archi Family) about it. The game originally use software vertexes only, but he edited the game and switched to hardware vertexes (which increased FPS rapidly on Windows), but he was facing the same problem - the game was crashing. Fortunately, he found that the game uses different code branches when hardware vertex are used - developers probably never finished support for hardware vertexes and that is reason why it is crashing when HW vertexes are used. He fixed game code so it works with HW vertexes (even in Wine). The problem is that game uses only SW vertexes and these unfinished branches are called only when HW vertexes are used, which is not a problem for Windows (which support SW vertexes). But for some reason Wine uses HW vertexes even if game wants to use SW vertexes. Thats why the game works in Wine only with this util applied. I can see from report [2] that game tries to use SW vertexes, which are unsupported by Wine. Can I simply edit some code in Wine (some D3D flag) so the game will think that it use SW vertexes, so after checking some flags by game it will not jump into some unfinished branches for HW vertexes? I just want to test if it is really caused by this. From application view, what is difference between using SW and HW vertexes, do you think that using HW vertexes with code done for SW vertexes will work? [1] https://bugs.winehq.org/show_bug.cgi?id=9337 [2] https://bugs.winehq.org/show_bug.cgi?id=9337#c35 Thanks, Jan

Aug. 6, 2017

Re: [PATCH] msvcrt: Fix strcpy implementation so it works on overlapping buffers

by Piotr Caban

On 08/05/17 18:21, Ken Thomases wrote: > On Aug 5, 2017, at 10:43 AM, Piotr Caban <piotr(a)codeweavers.com> wrote: >> +char* CDECL MSVCRT_strcpy(char *dst, const char *src) >> +{ >> + char *ret = dst; >> + >> + while(1) { >> + *dst = *src; >> + if (!*src) break; >> + src++; >> + *dst++; > > What's the dereference for? My bad, thanks for spotting it. I've sent a fixed version. Thanks, Piotr

Aug. 5, 2017

Re: [PATCH] msvcrt: Fix strcpy implementation so it works on overlapping buffers

by Ken Thomases

On Aug 5, 2017, at 10:43 AM, Piotr Caban <piotr(a)codeweavers.com> wrote: > +char* CDECL MSVCRT_strcpy(char *dst, const char *src) > +{ > + char *ret = dst; > + > + while(1) { > + *dst = *src; > + if (!*src) break; > + src++; > + *dst++; What's the dereference for? > + } > + > + return ret; > +} -Ken

Aug. 5, 2017

Re: [4/6] wsdapi: Implement "Hello" message creation support. [try 3]

by Vincent Povirk

Why would it be the same for each interface and protocol? If the purpose is to prevent one big spike of traffic, it seems like you'd want different delays. On Fri, Aug 4, 2017 at 4:34 AM, Owen Rudge <owen(a)owenrudge.net> wrote: >> I think it would make more sense to do the initial random delay on the >> background threads. > > I did consider that. The delay is only intended to be once per message. I could do simultaneous identical delays on each background thread, or I could have a 'delay' thread that then spawns the background threads, instead. For simplicity though I felt I'd just leave the initial delay on the main thread. It does mean there is a slight delay on the main thread compared with Windows when a 'hello' or 'probe matches' message is sent, but compared with the initial version of my patch (which included retransmission delays for every adapter on the main thread) it is much improved. > > If however you feel strongly that this should be altered, I can submit an updated version. > > Cheers, > > -- > Owen Rudge > http://www.owenrudge.net/ > >

Aug. 4, 2017

Re: [4/6] wsdapi: Implement "Hello" message creation support. [try 3]

by Owen Rudge

> I think it would make more sense to do the initial random delay on the > background threads. I did consider that. The delay is only intended to be once per message. I could do simultaneous identical delays on each background thread, or I could have a 'delay' thread that then spawns the background threads, instead. For simplicity though I felt I'd just leave the initial delay on the main thread. It does mean there is a slight delay on the main thread compared with Windows when a 'hello' or 'probe matches' message is sent, but compared with the initial version of my patch (which included retransmission delays for every adapter on the main thread) it is much improved. If however you feel strongly that this should be altered, I can submit an updated version. Cheers, -- Owen Rudge http://www.owenrudge.net/

Aug. 4, 2017

Re: shell32: Implement SHCreateItemInKnownFolder.

by Jactry Zeng

2017-08-02 16:04 GMT+08:00 Huw Davies <huw(a)codeweavers.com>: > > This is presumably a bug in SHGetKnownFolderIDList(), which we should fix. > Yes, there is a bug in SHGetKnownFolderIDList(), but I find it will not hinder implementing SHCreateItemInKnownFolder(). I will send tests and possible fix for SHGetKnownFolderIDList() later. Thanks! -- Regards, Jactry Zeng

Aug. 4, 2017

Re: [4/6] wsdapi: Implement "Hello" message creation support. [try 3]

by Vincent Povirk

I think it would make more sense to do the initial random delay on the background threads.

Aug. 3, 2017

Re: [4/4] kernel32/tests: Add tests for LoadLibrary with LIBRARY_SEARCH_* flags

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=32575 Your paranoid android. === w2003std (32 bit module) === module.c:443: Test failed: Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x), got 87 module.c:471: Test failed: Could not load test library testFlags.dll from C:\DOCUME~1\ADMINI~1\LOCALS~1\Temp\ module.c:472: Test failed: Expected 0xdeadbeef or ERROR_SUCCESS, got 87 module.c:481: Test failed: Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x), got 87 module.c:496: Test failed: Library testFlags.dll was expected to be loaded from 'system32' module.c:497: Test failed: Expected 0xdeadbeef or ERROR_SUCCESS, got 87 module.c:505: Test failed: Library testFlags.dll was expected to be loaded from 'system32' module.c:506: Test failed: Expected 0xdeadbeef or ERROR_SUCCESS, got 87 === w7u (32 bit loader) === loader.c:2199: Test failed: expected WAIT_OBJECT_0, got 0x102 loader.c:2209: Test failed: expected thread exit code 196, got 259 loader.c:1713: Test failed: expected thread exit code 196, got 195

Aug. 3, 2017

Re: [PATCH 2/2] winebrowser: Open the default browser if called without an argument.

by Zebediah Figura

On 08/03/2017 01:24 AM, Alexandre Julliard wrote: > Vincent Povirk <madewokherd(a)gmail.com> writes: > >>> I'd suggest to simply open the default start page in that case. That's >>> what iexplore does when there are no arguments. >> >> That's the intent. We don't know what the default start page is for >> the user's default browser, but it should open it when started with no >> arguments. > > I don't mean the default page for the Unix browser, the behavior is > apparently not guaranteed in that case. I mean get the default page > configured on the Windows side, and open that. This way we don't need to > worry about what the Unix browser will do with no arguments. > The program in question loads the value of HKCR/htmlfile/shell/open/command and calls it with no arguments. If I change that to something other than iexplore, the behaviour is left up to that program. So I'm not sure that just following iexplore's behaviour is necessarily appropriate.

Aug. 3, 2017

Re: [PATCH 2/2] winebrowser: Open the default browser if called without an argument.

by Alexandre Julliard

Vincent Povirk <madewokherd(a)gmail.com> writes: > So, we'd just load winehq.org? Seems pretty weird to me. You can open the default search page instead. > I think it's reasonable to expect that a browser will open when its > desktop file is called with no arguments. Whether it loads to its > default start page is unimportant. It's reasonable to expect that winebrowser will open, yes. That's precisely what's broken if we simply pass through to the Unix browser, so providing a default argument seems like an obvious fix. I don't see any benefit in adding complexity to try to make the Unix browser use its default page instead of ours. -- Alexandre Julliard julliard(a)winehq.org

Aug. 3, 2017

Re: [PATCH 2/2] winebrowser: Open the default browser if called without an argument.

by Vincent Povirk

So, we'd just load winehq.org? Seems pretty weird to me. I think it's reasonable to expect that a browser will open when its desktop file is called with no arguments. Whether it loads to its default start page is unimportant. On Thu, Aug 3, 2017 at 1:24 AM, Alexandre Julliard <julliard(a)winehq.org> wrote: > Vincent Povirk <madewokherd(a)gmail.com> writes: > >>> I'd suggest to simply open the default start page in that case. That's >>> what iexplore does when there are no arguments. >> >> That's the intent. We don't know what the default start page is for >> the user's default browser, but it should open it when started with no >> arguments. > > I don't mean the default page for the Unix browser, the behavior is > apparently not guaranteed in that case. I mean get the default page > configured on the Windows side, and open that. This way we don't need to > worry about what the Unix browser will do with no arguments. > > -- > Alexandre Julliard > julliard(a)winehq.org

Aug. 3, 2017

Re: [1/2] msvcrt/tests: Add tests for mbstowcs and wcstombs with empty strings.

by Piotr Caban

Hi, the patch causes compilation warnings on 64-bit wine. ../../../../wine_src/dlls/msvcrt/tests/string.c: In function 'test_mbstowcs': ../../../../wine_src/dlls/msvcrt/tests/string.c:1869:123: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t {aka long unsigned int}' [-Wformat=] ../../../../wine_src/dlls/msvcrt/tests/string.c:1872:123: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t {aka long unsigned int}' [-Wformat=] ../../../../wine_src/dlls/msvcrt/tests/string.c:1887:123: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t {aka long unsigned int}' [-Wformat=] ../../../../wine_src/dlls/msvcrt/tests/string.c:1890:123: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t {aka long unsigned int}' [-Wformat=] ../../../../wine_src/dlls/msvcrt/tests/string.c:1903:228: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t {aka long unsigned int}' [-Wformat=] ../../../../wine_src/dlls/msvcrt/tests/string.c:1911:123: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t {aka long unsigned int}' [-Wformat=] ../../../../wine_src/dlls/msvcrt/tests/string.c:1932:228: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t {aka long unsigned int}' [-Wformat=] ../../../../wine_src/dlls/msvcrt/tests/string.c:1951:123: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t {aka long unsigned int}' [-Wformat=] Thanks, Piotr

Aug. 3, 2017

Re: ntdll: Support receiving server reply data to memory with write watch.

by Alexandre Julliard

Jacek Caban <jacek(a)codeweavers.com> writes: > On 03.08.2017 09:48, Alexandre Julliard wrote: >> Jacek Caban <jacek(a)codeweavers.com> writes: >> >>> @@ -256,6 +256,7 @@ static void read_reply_data( void *buffer, size_t size ) >>> if (!ret) break; >>> if (errno == EINTR) continue; >>> if (errno == EPIPE) break; >>> + if (errno == EFAULT && virtual_uninterrupted_write_memory( buffer, NULL, size ) >= size) continue; >>> server_protocol_perror("read"); >> You can't enter a critical section in the middle of a server call. > > Oh, right, we could need to do another server call there. Unless I'm > missing something, it should work fine for platforms that have optimized > CS implementation (Linux and Mac) as they don't do server calls. We > could limit effect of the patch to those platform, but that would be ugly. > > Could we just use pthread mutexes for > server_enter_uninterrupted_section/server_leave_uninterrupted_section > instead of critical sections? I don't think you should be doing that sort of thing inside server calls at all. Some sort of lock/unlock_buffer methods that could be wrapped around server calls where necessary would probably work better. -- Alexandre Julliard julliard(a)winehq.org

Aug. 3, 2017

Re: ntdll: Support receiving server reply data to memory with write watch.

by Jacek Caban

On 03.08.2017 09:48, Alexandre Julliard wrote: > Jacek Caban <jacek(a)codeweavers.com> writes: > >> @@ -256,6 +256,7 @@ static void read_reply_data( void *buffer, size_t size ) >> if (!ret) break; >> if (errno == EINTR) continue; >> if (errno == EPIPE) break; >> + if (errno == EFAULT && virtual_uninterrupted_write_memory( buffer, NULL, size ) >= size) continue; >> server_protocol_perror("read"); > You can't enter a critical section in the middle of a server call. Oh, right, we could need to do another server call there. Unless I'm missing something, it should work fine for platforms that have optimized CS implementation (Linux and Mac) as they don't do server calls. We could limit effect of the patch to those platform, but that would be ugly. Could we just use pthread mutexes for server_enter_uninterrupted_section/server_leave_uninterrupted_section instead of critical sections? Thanks, Jacek

Aug. 3, 2017

Re: [PATCH v3] ole32: Store proxy/stub CLSIDs per process, not per apartment.

by Huw Davies

On Wed, Aug 02, 2017 at 11:00:04AM -0500, Zebediah Figura wrote: > +static void COM_RevokeAllPSClsids(void) > +{ How about calling it revoke_registered_psclsids() ? Otherwise, this looks good. Huw.

Aug. 3, 2017

Re: ntdll: Support receiving server reply data to memory with write watch.

by Alexandre Julliard

Jacek Caban <jacek(a)codeweavers.com> writes: > @@ -256,6 +256,7 @@ static void read_reply_data( void *buffer, size_t size ) > if (!ret) break; > if (errno == EINTR) continue; > if (errno == EPIPE) break; > + if (errno == EFAULT && virtual_uninterrupted_write_memory( buffer, NULL, size ) >= size) continue; > server_protocol_perror("read"); You can't enter a critical section in the middle of a server call. -- Alexandre Julliard julliard(a)winehq.org

Aug. 3, 2017

Re: [PATCH 2/2] winebrowser: Open the default browser if called without an argument.

by Alexandre Julliard

Vincent Povirk <madewokherd(a)gmail.com> writes: >> I'd suggest to simply open the default start page in that case. That's >> what iexplore does when there are no arguments. > > That's the intent. We don't know what the default start page is for > the user's default browser, but it should open it when started with no > arguments. I don't mean the default page for the Unix browser, the behavior is apparently not guaranteed in that case. I mean get the default page configured on the Windows side, and open that. This way we don't need to worry about what the Unix browser will do with no arguments. -- Alexandre Julliard julliard(a)winehq.org

Aug. 3, 2017

Re: [PATCH 2/2] winebrowser: Open the default browser if called without an argument.

by Vincent Povirk

> I'd suggest to simply open the default start page in that case. That's > what iexplore does when there are no arguments. That's the intent. We don't know what the default start page is for the user's default browser, but it should open it when started with no arguments.

Aug. 2, 2017

Re: [PATCH 2/2] winebrowser: Open the default browser if called without an argument.

by Alexandre Julliard

Zebediah Figura <zfigura(a)codeweavers.com> writes: > xdg-open can't handle this, so work around with the following command > (suggested by Vincent Povirk): > gtk-launch `xdg-mime query default x-scheme-handler/http` > > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> > --- > programs/winebrowser/main.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) I'd suggest to simply open the default start page in that case. That's what iexplore does when there are no arguments. -- Alexandre Julliard julliard(a)winehq.org

Aug. 2, 2017

Re: [4/6] wsdapi: Implement "Hello" message creation support. [try 2]

by Owen Rudge

On 02/08/2017 19:36, Vincent Povirk wrote: > You're still using rand/srand in this one. Sorry, I removed them from the network code but missed the initial delay required by some of the messages. Now corrected as well. Cheers, -- Owen Rudge http://www.owenrudge.net/

Aug. 2, 2017

Re: [2/6] wsdapi: Add code to create SOAP envelope from header and body elements. [try 2]

by Owen Rudge

On 02/08/2017 19:17, Vincent Povirk wrote: > I think this should be parent->FirstAttribute = curAttrib->Next. Thanks. > You still don't seem to be adding the new text node to newElement. Ah, I had corrected the memory allocation parenting, but not the actual nodes. This is now fixed in my new revision. Thanks, -- Owen Rudge http://www.owenrudge.net/

Aug. 2, 2017

Re: [4/6] wsdapi: Implement "Hello" message creation support. [try 2]

by Vincent Povirk

You're still using rand/srand in this one.

Aug. 2, 2017

Re: [2/6] wsdapi: Add code to create SOAP envelope from header and body elements. [try 2]

by Vincent Povirk

+static void remove_attribute(WSDXML_ELEMENT *parent, WSDXML_ATTRIBUTE *attribute) +{ + WSDXML_ATTRIBUTE *curAttrib; + + /* Find the last attribute and add this as the next one */ + curAttrib = parent->FirstAttribute; + + if (curAttrib == attribute) + { + parent->FirstAttribute = NULL; + } I think this should be parent->FirstAttribute = curAttrib->Next. + } + else if (curNode->Type == TextType) + { + textNode = WSDAllocateLinkedMemory(newElement, sizeof(WSDXML_TEXT)); + if (textNode == NULL) goto cleanup; + + textNode->Node.Next = NULL; + textNode->Node.Parent = newElement; + textNode->Node.Type = TextType; + textNode->Text = duplicate_string(textNode, ((WSDXML_TEXT *)curNode)->Text); + if (textNode->Text == NULL) goto cleanup; + } You still don't seem to be adding the new text node to newElement.

Aug. 2, 2017

Re: [PATCH v2] ole32: Store proxy/stub CLSIDs per process, not per apartment.

by Zebediah Figura

On 08/02/2017 04:32 AM, Huw Davies wrote: > On Tue, Aug 01, 2017 at 04:54:45PM -0500, Zebediah Figura wrote: >> v2: use proper synchronization; add an MTA test (thanks Sebastian) >> Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> >> --- >> dlls/ole32/compobj.c | 48 ++++++++++++++++++++++++++++++------------ >> dlls/ole32/compobj_private.h | 1 - >> dlls/ole32/tests/compobj.c | 50 ++++++++++++++++++++++++++++++++++++++++++-- >> 3 files changed, 83 insertions(+), 16 deletions(-) >> >> diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c >> index 60244485249..ee904adbc38 100644 >> --- a/dlls/ole32/compobj.c >> +++ b/dlls/ole32/compobj.c >> @@ -170,8 +170,20 @@ struct registered_psclsid >> struct list entry; >> IID iid; >> CLSID clsid; >> + OXID apartment_id; >> }; >> >> +static struct list RegisteredPSCLSIDList = LIST_INIT(RegisteredPSCLSIDList); > > Please don't use CamelCase. > >> + >> +static CRITICAL_SECTION csRegisteredPSCLSIDList; >> +static CRITICAL_SECTION_DEBUG psclsid_cs_debug = >> +{ >> + 0, 0, &csRegisteredPSCLSIDList, >> + { &psclsid_cs_debug.ProcessLocksList, &psclsid_cs_debug.ProcessLocksList }, >> + 0, 0, { (DWORD_PTR)(__FILE__ ": csRegisteredPSCLSIDList") } >> +}; >> +static CRITICAL_SECTION csRegisteredPSCLSIDList = { &psclsid_cs_debug, -1, 0, 0, 0, 0 }; >> + >> /* >> * This is a marshallable object exposing registered local servers. >> * IServiceProvider is used only because it happens meet requirements >> @@ -622,7 +634,6 @@ static APARTMENT *apartment_construct(DWORD model) >> >> list_init(&apt->proxies); >> list_init(&apt->stubmgrs); >> - list_init(&apt->psclsids); >> list_init(&apt->loaded_dlls); >> apt->ipidc = 0; >> apt->refs = 1; >> @@ -1173,15 +1184,22 @@ DWORD apartment_release(struct apartment *apt) >> stub_manager_int_release(stubmgr); >> } >> >> - LIST_FOR_EACH_SAFE(cursor, cursor2, &apt->psclsids) >> + EnterCriticalSection(&csRegisteredPSCLSIDList); >> + >> + LIST_FOR_EACH_SAFE(cursor, cursor2, &RegisteredPSCLSIDList) >> { >> struct registered_psclsid *registered_psclsid = >> LIST_ENTRY(cursor, struct registered_psclsid, entry); >> >> - list_remove(&registered_psclsid->entry); >> - HeapFree(GetProcessHeap(), 0, registered_psclsid); >> + if (registered_psclsid->apartment_id == apt->oxid) >> + { >> + list_remove(&registered_psclsid->entry); >> + HeapFree(GetProcessHeap(), 0, registered_psclsid); >> + } >> } >> >> + LeaveCriticalSection(&csRegisteredPSCLSIDList); >> + >> /* if this assert fires, then another thread took a reference to a >> * stub manager without taking a reference to the containing >> * apartment, which it must do. */ > > > It seems it's more complicated than this. Consider two already > existing STAs, if the PSClsid is registered in STA1 which then leaves > the apt, a call in STA2 to CoGetPSCLSID() still succeeds. > > You'll want to expand the tests to cover things like this to really figure out > what's going on. > > Also note that the MTA -> MTA test isn't very convincing, since there is only > one MTA and hence both threads are in the same apt. > > Huw. > > I see; I'll send an updated patch.

Aug. 2, 2017

Re: [PATCHv2] msvcrt: Export -f suffixed float functions functions on arm64

by Martin Storsjö

Hi Piotr, On Wed, 2 Aug 2017, Piotr Caban wrote: > Hi Martin, > > On 07/31/17 14:47, Martin Storsjo wrote: >> --- a/dlls/api-ms-win-crt-math-l1-1-0/api-ms-win-crt-math-l1-1-0.spec >> +++ b/dlls/api-ms-win-crt-math-l1-1-0/api-ms-win-crt-math-l1-1-0.spec >> @@ -77,7 +77,7 @@ >> @ cdecl _fdtest(ptr) ucrtbase._fdtest >> @ stub _fdunscale >> @ cdecl _finite(double) ucrtbase._finite >> -@ cdecl -arch=arm,x86_64 _finitef(float) ucrtbase._finitef >> +@ cdecl -arch=arm,x86_64,arm64 _finitef(float) ucrtbase._finitef >> @ cdecl _fpclass(double) ucrtbase._fpclass >> @ stub _fpclassf >> @ cdecl -arch=i386 -ret64 _ftol() ucrtbase._ftol >> @@ -85,7 +85,7 @@ >> @ cdecl _hypot(double double) ucrtbase._hypot >> @ cdecl _hypotf(float float) ucrtbase._hypotf >> @ cdecl _isnan(double) ucrtbase._isnan >> -@ cdecl -arch=arm,x86_64 _isnanf(float) ucrtbase._isnanf >> +@ cdecl -arch=arm,x86_64,arm64 _isnanf(float) ucrtbase._isnanf > Maybe I'm checking it incorrectly but I don't see isnanf in the lib file. Is > it really exported on arm64? I didn't check all of the functions but _scalbf > also seems to be not exported. Hmm, indeed - thanks for checking thoroughly. I'll recheck them one by one with the import library. FWIW it seems like this also is wrong for _isnanf already now. For that case, it can be traced back to 599d92a2, where I bootstrapped ucrtbase, with a list of symbols dumped from real DLLs, but with the -arch flags taken from msvcrt (where it was added to a bunch of float functions in 599d0802). // Martin

Aug. 2, 2017

Re: [PATCHv2] msvcrt: Export -f suffixed float functions functions on arm64

by Piotr Caban

Hi Martin, On 07/31/17 14:47, Martin Storsjo wrote: > --- a/dlls/api-ms-win-crt-math-l1-1-0/api-ms-win-crt-math-l1-1-0.spec > +++ b/dlls/api-ms-win-crt-math-l1-1-0/api-ms-win-crt-math-l1-1-0.spec > @@ -77,7 +77,7 @@ > @ cdecl _fdtest(ptr) ucrtbase._fdtest > @ stub _fdunscale > @ cdecl _finite(double) ucrtbase._finite > -@ cdecl -arch=arm,x86_64 _finitef(float) ucrtbase._finitef > +@ cdecl -arch=arm,x86_64,arm64 _finitef(float) ucrtbase._finitef > @ cdecl _fpclass(double) ucrtbase._fpclass > @ stub _fpclassf > @ cdecl -arch=i386 -ret64 _ftol() ucrtbase._ftol > @@ -85,7 +85,7 @@ > @ cdecl _hypot(double double) ucrtbase._hypot > @ cdecl _hypotf(float float) ucrtbase._hypotf > @ cdecl _isnan(double) ucrtbase._isnan > -@ cdecl -arch=arm,x86_64 _isnanf(float) ucrtbase._isnanf > +@ cdecl -arch=arm,x86_64,arm64 _isnanf(float) ucrtbase._isnanf Maybe I'm checking it incorrectly but I don't see isnanf in the lib file. Is it really exported on arm64? I didn't check all of the functions but _scalbf also seems to be not exported. Thanks, Piotr

Aug. 2, 2017

Re: [PATCH] comctl32/taskdialog: Add callback and tests

by Nikolay Sivov

On 28.07.2017 19:53, Fabian Maurer wrote: > > +static HRESULT callback(struct taskdialog_info *dialog_info, UINT uNotification, WPARAM wParam, LPARAM lParam) > +{ > + if(dialog_info->callback) > + return dialog_info->callback(dialog_info->hwnd, uNotification, wParam, lParam, dialog_info->callback_data); > + return S_OK; > +} Please use consistent naming and formatting, like space after 'if' for example, and uNotification -> notification. > + > static INT_PTR CALLBACK taskdialog_proc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) > { > + static const WCHAR taskdialog_info_propnameW[] = {'T','a','s','k','D','i','a','l','o','g','I','n','f','o',0}; > + struct taskdialog_info *dialog_info; > + > TRACE("hwnd=%p msg=0x%04x wparam=%lx lparam=%lx\n", hwnd, msg, wParam, lParam); > > + if(msg != WM_INITDIALOG && msg != WM_NCDESTROY) > + dialog_info = GetPropW(hwnd, taskdialog_info_propnameW); > + Why WM_NCDESTROY is a special case? > switch (msg) > { > + case WM_INITDIALOG: > + dialog_info = (struct taskdialog_info *)lParam; > + dialog_info->hwnd = hwnd; > + SetPropW(hwnd, taskdialog_info_propnameW, dialog_info); > + > + callback(dialog_info, TDN_DIALOG_CONSTRUCTED, 0, 0); > + callback(dialog_info, TDN_CREATED, 0, 0); > + break; This looks suspicious, documentation is not strict on this, but it seems TDN_CREATED corresponds for to WM_SHOWWINDOW stage. > case WM_COMMAND: > if (HIWORD(wParam) == BN_CLICKED) > { > WORD command_id = LOWORD(wParam); > - EndDialog(hwnd, command_id); > + click_button(dialog_info, command_id); > return TRUE; > } > break; > + case WM_DESTROY: > + callback(dialog_info, TDN_DESTROYED, 0, 0); > + RemovePropW(hwnd, taskdialog_info_propnameW); > + break; > + > + /* Custom messages*/ What do you mean by "custom"? > + > + case TDM_CLICK_BUTTON: > + click_button(dialog_info, wParam); > + break; > } > return FALSE; > } > @@ -507,17 +556,24 @@ HRESULT WINAPI TaskDialogIndirect(const TASKDIALOGCONFIG *taskconfig, int *butto > int *radio_button, BOOL *verification_flag_checked) > { > DLGTEMPLATE *template; > + struct taskdialog_info dialog_info; > INT ret; > > TRACE("%p, %p, %p, %p\n", taskconfig, button, radio_button, verification_flag_checked); > > + if(!taskconfig || taskconfig->cbSize != sizeof(TASKDIALOGCONFIG)) > + return E_INVALIDARG; Tests you added only check cbSize == 0 case. > + > + dialog_info.callback = taskconfig->pfCallback; > + dialog_info.callback_data = taskconfig->lpCallbackData; > + > template = create_taskdialog_template(taskconfig); > - ret = DialogBoxIndirectParamW(taskconfig->hInstance, template, taskconfig->hwndParent, taskdialog_proc, 0); > + ret = DialogBoxIndirectParamW(taskconfig->hInstance, template, taskconfig->hwndParent, taskdialog_proc, (LPARAM)&dialog_info); > Free(template); > > if (button) *button = ret; > if (radio_button) *radio_button = taskconfig->nDefaultButton; > - if (verification_flag_checked) *verification_flag_checked = TRUE; > + if (verification_flag_checked) *verification_flag_checked = FALSE; Why do you need this flag value change? > > return S_OK; > } > diff --git a/dlls/comctl32/tests/taskdialog.c b/dlls/comctl32/tests/taskdialog.c > index c937127897..867361617c 100644 > --- a/dlls/comctl32/tests/taskdialog.c > +++ b/dlls/comctl32/tests/taskdialog.c > @@ -24,13 +24,250 @@ > #include "winuser.h" > #include "commctrl.h" > > +#include "wine/list.h" > #include "wine/test.h" > #include "v6util.h" > +#include "msg.h" > + > +#define WM_TD_CALLBACK (WM_APP) /* Custom dummy message to wrap callback notifications */ > + > +#define NUM_MSG_SEQUENCES 1 > +#define TASKDIALOG_SEQ_INDEX 0 > + > +#define TEST_NUM_BUTTONS 20 /* Number of custom buttons to test with */ > + > +#define ID_START 20 /* Lower IDs might be used by the system */ > +#define ID_START_BUTTON (ID_START + 0) > > static HRESULT (WINAPI *pTaskDialogIndirect)(const TASKDIALOGCONFIG *, int *, int *, BOOL *); > static HRESULT (WINAPI *pTaskDialog)(HWND, HINSTANCE, const WCHAR *, const WCHAR *, const WCHAR *, > TASKDIALOG_COMMON_BUTTON_FLAGS, const WCHAR *, int *); > > +static struct msg_sequence *sequences[NUM_MSG_SEQUENCES]; > + > +/* Message lists to test against */ > + > +struct message_send_info > +{ > + UINT send_message; > + WPARAM send_wparam; > + LPARAM send_lparam; > + > + BOOL post; /* post instead of send */ > + const CHAR *title_target; /* control text, 0 means it's send to the dialog form instead */ > +}; > + > +struct message_info > +{ > + UINT recv_message; /* Message the callback receives */ > + WPARAM recv_wparam; > + LPARAM recv_lparam; > + > + HRESULT ret; /* Value the callback should return */ > + > + struct message_send_info send[9]; /* Message to send to trigger the next callback message */ > +}; > + > +static const struct message_info *current_message_info; > + > +static const struct message_info mes_return_press_ok[] = { > + { TDN_CREATED, 0, 0, S_OK, { > + { WM_KEYDOWN, VK_RETURN, 0, TRUE }, > + { 0 }}}, > + { TDN_BUTTON_CLICKED, IDOK, 0, S_OK, {{ 0 }}}, > + { 0 } > +}; > + > +static const struct message_info mes_cancel_button_press[] = { > + { TDN_CREATED, 0, 0, S_OK, { > + { TDM_CLICK_BUTTON, IDOK, 0, TRUE }, > + { TDM_CLICK_BUTTON, IDOK, 0, TRUE }, > + { TDM_CLICK_BUTTON, IDOK, 0, TRUE }, > + { 0 }}}, > + { TDN_BUTTON_CLICKED, IDOK, 0, S_FALSE, {{ 0 }}}, > + { TDN_BUTTON_CLICKED, IDOK, 0, 0xFF, {{ 0 }}}, /* Random return value tested here */ > + { TDN_BUTTON_CLICKED, IDOK, 0, S_OK, {{ 0 }}}, > + { 0 } > +}; > + > +static const struct message_info mes_send_button_directly[] = { > + { TDN_CREATED, 0, 0, S_OK, { > + { WM_LBUTTONDOWN, MK_LBUTTON, 0, TRUE, "02" }, > + { WM_LBUTTONUP, 0, 0, TRUE, "02" }, > + { 0 }}}, > + { TDN_BUTTON_CLICKED, ID_START + 2, 0, S_OK, {{ 0 }}}, > + { 0 } > +}; This looks too complicated, I'd just check notifications callback received, using 'struct message' data, same way WM_NOTIFY tests work. > + > +/* Create a message to test against */ > +static struct message create_test_message(UINT message, WPARAM wParam, LPARAM lParam) > +{ > + struct message mes; > + > + mes.message = WM_TD_CALLBACK; > + mes.id = message; > + mes.flags = sent|wparam|lparam|id; > + mes.wParam = wParam; > + mes.lParam = lParam; > + > + return mes; > +} > + > +/* Our only way to get a button handle, since GetDlgItem and FindWindowEx don't work for the official taskdialog */ > + > +static HWND taskdialog_child; > +BOOL CALLBACK enum_taskdialog_children_proc(HWND hwnd, LPARAM lParam) > +{ > + CHAR text[100]; > + const CHAR *title = (const CHAR *)lParam; > + > + GetWindowTextA(hwnd, text, sizeof(text)); > + > + if(lstrcmpA(text, title) == 0) > + { > + taskdialog_child = hwnd; > + return FALSE; > + } > + > + return TRUE; > +} > + > +static HWND get_child_from_title(HWND hwnd_parent, const CHAR *title) > +{ > + taskdialog_child = NULL; > + EnumChildWindows(hwnd_parent, enum_taskdialog_children_proc, (LPARAM)title); > + return taskdialog_child; > +} I don't think we need to get that deep. > > + init_msg_sequences(sequences, NUM_MSG_SEQUENCES); > + test_TaskDialogIndirect(); This name is too generic, in a test function like that you can test invalid parameter handling for example. Then have another function for callback notifications, and another one for TDM_* messages. I think for first iteration test should simply check creation/destruction notifications and callback user data value.

Aug. 2, 2017

Re: [PATCH v2] ole32: Store proxy/stub CLSIDs per process, not per apartment.

by Huw Davies

On Tue, Aug 01, 2017 at 04:54:45PM -0500, Zebediah Figura wrote: > v2: use proper synchronization; add an MTA test (thanks Sebastian) > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> > --- > dlls/ole32/compobj.c | 48 ++++++++++++++++++++++++++++++------------ > dlls/ole32/compobj_private.h | 1 - > dlls/ole32/tests/compobj.c | 50 ++++++++++++++++++++++++++++++++++++++++++-- > 3 files changed, 83 insertions(+), 16 deletions(-) > > diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c > index 60244485249..ee904adbc38 100644 > --- a/dlls/ole32/compobj.c > +++ b/dlls/ole32/compobj.c > @@ -170,8 +170,20 @@ struct registered_psclsid > struct list entry; > IID iid; > CLSID clsid; > + OXID apartment_id; > }; > > +static struct list RegisteredPSCLSIDList = LIST_INIT(RegisteredPSCLSIDList); Please don't use CamelCase. > + > +static CRITICAL_SECTION csRegisteredPSCLSIDList; > +static CRITICAL_SECTION_DEBUG psclsid_cs_debug = > +{ > + 0, 0, &csRegisteredPSCLSIDList, > + { &psclsid_cs_debug.ProcessLocksList, &psclsid_cs_debug.ProcessLocksList }, > + 0, 0, { (DWORD_PTR)(__FILE__ ": csRegisteredPSCLSIDList") } > +}; > +static CRITICAL_SECTION csRegisteredPSCLSIDList = { &psclsid_cs_debug, -1, 0, 0, 0, 0 }; > + > /* > * This is a marshallable object exposing registered local servers. > * IServiceProvider is used only because it happens meet requirements > @@ -622,7 +634,6 @@ static APARTMENT *apartment_construct(DWORD model) > > list_init(&apt->proxies); > list_init(&apt->stubmgrs); > - list_init(&apt->psclsids); > list_init(&apt->loaded_dlls); > apt->ipidc = 0; > apt->refs = 1; > @@ -1173,15 +1184,22 @@ DWORD apartment_release(struct apartment *apt) > stub_manager_int_release(stubmgr); > } > > - LIST_FOR_EACH_SAFE(cursor, cursor2, &apt->psclsids) > + EnterCriticalSection(&csRegisteredPSCLSIDList); > + > + LIST_FOR_EACH_SAFE(cursor, cursor2, &RegisteredPSCLSIDList) > { > struct registered_psclsid *registered_psclsid = > LIST_ENTRY(cursor, struct registered_psclsid, entry); > > - list_remove(&registered_psclsid->entry); > - HeapFree(GetProcessHeap(), 0, registered_psclsid); > + if (registered_psclsid->apartment_id == apt->oxid) > + { > + list_remove(&registered_psclsid->entry); > + HeapFree(GetProcessHeap(), 0, registered_psclsid); > + } > } > > + LeaveCriticalSection(&csRegisteredPSCLSIDList); > + > /* if this assert fires, then another thread took a reference to a > * stub manager without taking a reference to the containing > * apartment, which it must do. */ It seems it's more complicated than this. Consider two already existing STAs, if the PSClsid is registered in STA1 which then leaves the apt, a call in STA2 to CoGetPSCLSID() still succeeds. You'll want to expand the tests to cover things like this to really figure out what's going on. Also note that the MTA -> MTA test isn't very convincing, since there is only one MTA and hence both threads are in the same apt. Huw.

Aug. 2, 2017

Re: shell32: Implement SHCreateItemInKnownFolder.

by Huw Davies

On Wed, Aug 02, 2017 at 10:58:34AM +0800, Jactry Zeng wrote: > 2017-07-31 15:47 GMT+08:00 Huw Davies <huw(a)codeweavers.com>: > > > > You probably want to use SHGetKnownFolderIDList() instead of these > > last two calls. > > > I had some more tests of SHGetKnownFolderIDList() and IShellFolder_BindToObject > (), I found that in some cases SHGetKnownFolderIDList() will return a pidl > which ILIsEmpty() thinks it is empty. This is presumably a bug in SHGetKnownFolderIDList(), which we should fix. Huw.

Aug. 2, 2017

Re: [PATCH 2/3] msvcrt: Set errno through the _matherr function.

by Alex Henrie

2017-08-01 11:25 GMT-06:00 Piotr Caban <piotr.caban(a)gmail.com>: > On 08/01/17 08:13, Alex Henrie wrote: >> >> @@ -252,7 +320,7 @@ float CDECL MSVCRT_powf( float x, float y ) >> { >> /* FIXME: If x < 0 and y is not integral, set EDOM */ >> float z = powf(x,y); >> - if (!finitef(z)) *MSVCRT__errno() = MSVCRT_EDOM; >> + if (!finitef(z)) math_error(_DOMAIN, "powf", x, 0, ret); > > ret is not defined here. Good catch, thanks. I will test a 64-bit build of Wine before resubmitting. 2017-08-01 11:32 GMT-06:00 Piotr Caban <piotr.caban(a)gmail.com>: > On 08/01/17 19:25, Piotr Caban wrote: >> >> Could you please also add exception.name tests (in this case it should be >> set to pow instead of powf). > > I've done the testing incorrectly. Adding a test for exception.name value > will be useful anyway. I thought about testing exception.name, but then I found that on Windows a lot of the reported names don't match the functions' real names. For example, atanh reports itself to be asin, which is almost certainly a bug. I'd prefer to not write tests for buggy behavior that no application depends on. Who knows, maybe Microsoft will fix it in a future release. -Alex

Aug. 2, 2017

Re: shell32: Implement SHCreateItemInKnownFolder.

by Jactry Zeng

Hi Huw, Thanks for review! 2017-07-31 15:47 GMT+08:00 Huw Davies <huw(a)codeweavers.com>: > > You probably want to use SHGetKnownFolderIDList() instead of these > last two calls. > I had some more tests of SHGetKnownFolderIDList() and IShellFolder_BindToObject(), I found that in some cases SHGetKnownFolderIDList() will return a pidl which ILIsEmpty() thinks it is empty. And IShellFolder_BindToObject() will return E_INVALIDARG for an empty pidl. So keep using SHGetKnownFolderPath() and IShellFolder_ParseDisplayName() seems can make this work more easy? -- Regards, Jactry Zeng

Aug. 1, 2017

Re: [5/6] wsdapi: Add support for sending messages via UDP multicast.

by Owen Rudge

On 27/07/2017 21:22, Vincent Povirk wrote: > Using rand() for this doesn't seem very random, and it could interfere > with an application's use of rand(). I've altered this to use BCryptGenRandom instead. > Sending messages to every adapter in sequence, with a delay for each, > doesn't seem right. The spec doesn't explicitly state how multiple adapters should be handled. The SOAP over UDP spec just suggests how multicast messages should be sent (including delays and repeats as appropriate). However, I've changed this now so that a new thread is spawned for each adapter's transmission, which should reduce the overall time taken, and will also allow the Publish/UnPublish etc method to return immediately (or near enough), which appears to be the behaviour on Windows, as you've pointed out. > It seems like this will call WSACleanup even when freeing an object > that was never used. Thanks, I've amended this too. -- Owen Rudge http://www.owenrudge.net/

Aug. 1, 2017

Re: [PATCH] ole32: Store proxy/stub CLSIDs per process, not per apartment.

by Sebastian Lackner

On 01.08.2017 23:27, Zebediah Figura wrote: > @@ -1173,13 +1175,16 @@ DWORD apartment_release(struct apartment *apt) > stub_manager_int_release(stubmgr); > } > > - LIST_FOR_EACH_SAFE(cursor, cursor2, &apt->psclsids) > + LIST_FOR_EACH_SAFE(cursor, cursor2, &RegisteredPSCLSIDList) Accessing a global list from multiple threads requires synchronization (for example, using a CS to avoid conflicting access). Using apt->cs is not sufficient because it might be a different one for each thread. The same also applies to all other places where the list is used. > @@ -1177,6 +1196,15 @@ static void test_CoRegisterPSClsid(void) > hr = CoRegisterPSClsid(&IID_IWineTest, &CLSID_WineTestPSFactoryBuffer); > ok_ole_success(hr, "CoRegisterPSClsid"); > > + hr = CoGetPSClsid(&IID_IWineTest, &clsid); > + ok_ole_success(hr, "CoGetPSClsid"); > + ok(IsEqualGUID(&clsid, &CLSID_WineTestPSFactoryBuffer), "expected %s, got %s\n", > + wine_dbgstr_guid(&CLSID_WineTestPSFactoryBuffer), wine_dbgstr_guid(&clsid)); > + > + thread = CreateThread(NULL, 0, register_ps_clsid_thread, NULL, 0, &tid); > + ok(thread != NULL, "CreateThread failed with error %d\n", GetLastError()); > + ok(!WaitForSingleObject(thread, 10000), "wait timed out\n"); The thread handle is leaked here. Also, it would be more convincing to run the tests for both STA and MTA.

Aug. 1, 2017

Re: [2/6] wsdapi: Add code to create SOAP envelope from header and body elements.

by Owen Rudge

On 31/07/2017 21:50, Vincent Povirk wrote: > Is there something special about SequenceId compared to InstanceId and > MessageNumber? Hm, no, and it looks like it does work correctly without the namespace being passed in. -- Owen Rudge http://www.owenrudge.net/

Aug. 1, 2017

Re: [PATCH 1/2] winebrowser: Try other browsers on non-zero exit code.

by Zebediah Figura

On 08/01/2017 12:58 PM, Vincent Povirk wrote: > Using system is usually a bad idea for security reasons. > Will fix, thanks. > I also don't think waiting for the browser command to return makes > sense. It may keep winebrowser running longer than expected, and we > don't know for sure what the return value means. > How is this different from the current behaviour?

Aug. 1, 2017

Re: [PATCH 1/2] winebrowser: Try other browsers on non-zero exit code.

by Vincent Povirk

Using system is usually a bad idea for security reasons. I also don't think waiting for the browser command to return makes sense. It may keep winebrowser running longer than expected, and we don't know for sure what the return value means.

Aug. 1, 2017

Re: [PATCH 2/3] msvcrt: Set errno through the _matherr function.

by Piotr Caban

On 08/01/17 19:25, Piotr Caban wrote: > Could you please also add exception.name tests (in this case it should > be set to pow instead of powf). I've done the testing incorrectly. Adding a test for exception.name value will be useful anyway.

Aug. 1, 2017

Re: [PATCH 5/6 v3] ws2_32: Added support for overlapped GetAddrInfoExW.

by Sebastian Lackner

On 01.08.2017 19:08, Jacek Caban wrote: > + > + overlapped->Internal = ret; > + if (overlapped->hEvent) SetEvent(overlapped->hEvent); Not sure how it is implemented on Windows, but caching overlapped->hEvent would ensure that it also works when the overlapped struct is immediately reused after ->Internal changed.

Aug. 1, 2017

Re: [PATCH 2/3] msvcrt: Set errno through the _matherr function.

by Piotr Caban

On 08/01/17 08:13, Alex Henrie wrote: > @@ -252,7 +320,7 @@ float CDECL MSVCRT_powf( float x, float y ) > { > /* FIXME: If x < 0 and y is not integral, set EDOM */ > float z = powf(x,y); > - if (!finitef(z)) *MSVCRT__errno() = MSVCRT_EDOM; > + if (!finitef(z)) math_error(_DOMAIN, "powf", x, 0, ret); ret is not defined here. Could you please also add exception.name tests (in this case it should be set to pow instead of powf). Thanks, Piotr

Aug. 1, 2017

Re: [PATCH 5/6] ws2_32: Added support for overlapped GetAddrInfoExW.

by Jacek Caban

On 01.08.2017 18:52, Sebastian Lackner wrote: > On 01.08.2017 18:29, Jacek Caban wrote: >> +static void WINAPI getaddrinfo_callback(TP_CALLBACK_INSTANCE *instance, void *context, TP_WORK *work) >> +{ >> + struct getaddrinfo_args *args = context; >> + OVERLAPPED *overlapped = args->overlapped; >> + struct WS_addrinfo *res; >> + >> + overlapped->Internal = WS_getaddrinfo(args->nodename, args->servname, NULL, &res); > What if the app does busy-looping instead of using the event, and expects the result to > be set after overlapped->Internal changed? > > My understanding is that setting overlapped->Internal should be the very last thing in > the function, the app could decide to deallocate / reuse the memory after the async > operation has finished. It's not clear for me if it's supposed to be guaranteed, but it surely won't hurt. I will change it. >> + if (res) >> + { >> + *args->result = addrinfo_list_AtoW(res); >> + overlapped->u.Pointer = args->result; >> + WS_freeaddrinfo(res); >> + } >> [...] >> + tp_work = CreateThreadpoolWork(getaddrinfo_callback, args, NULL); >> + if (!tp_work) >> + { >> + HeapFree(GetProcessHeap(), 0, args); >> + ret = GetLastError(); >> + goto end; >> + } >> >> + overlapped->Internal = WSAEINPROGRESS; >> + SubmitThreadpoolWork(tp_work); > I don't see a CloseThreadpoolWork call or cleanup group anywhere, so there is likely a > leak in this code. Also, when all callbacks have different arguments, it would be better > to use TrySubmitThreadpoolCallback(). Work callbacks are typically used when the same > task is queued multiple times. TrySubmitThreadpoolCallback() looks like what I need, I will change it. Thanks, Jacek

Aug. 1, 2017

Re: [PATCH 5/6] ws2_32: Added support for overlapped GetAddrInfoExW.

by Sebastian Lackner

On 01.08.2017 18:29, Jacek Caban wrote: > +static void WINAPI getaddrinfo_callback(TP_CALLBACK_INSTANCE *instance, void *context, TP_WORK *work) > +{ > + struct getaddrinfo_args *args = context; > + OVERLAPPED *overlapped = args->overlapped; > + struct WS_addrinfo *res; > + > + overlapped->Internal = WS_getaddrinfo(args->nodename, args->servname, NULL, &res); What if the app does busy-looping instead of using the event, and expects the result to be set after overlapped->Internal changed? My understanding is that setting overlapped->Internal should be the very last thing in the function, the app could decide to deallocate / reuse the memory after the async operation has finished. > + if (res) > + { > + *args->result = addrinfo_list_AtoW(res); > + overlapped->u.Pointer = args->result; > + WS_freeaddrinfo(res); > + } >[...] > + tp_work = CreateThreadpoolWork(getaddrinfo_callback, args, NULL); > + if (!tp_work) > + { > + HeapFree(GetProcessHeap(), 0, args); > + ret = GetLastError(); > + goto end; > + } > > + overlapped->Internal = WSAEINPROGRESS; > + SubmitThreadpoolWork(tp_work); I don't see a CloseThreadpoolWork call or cleanup group anywhere, so there is likely a leak in this code. Also, when all callbacks have different arguments, it would be better to use TrySubmitThreadpoolCallback(). Work callbacks are typically used when the same task is queued multiple times. > + if (local_nodenameW != nodename) > + HeapFree(GetProcessHeap(), 0, local_nodenameW); > + WSASetLastError(ERROR_IO_PENDING); > + return ERROR_IO_PENDING; > + } > + > + ret = WS_getaddrinfo(nodenameA, servnameA, hints, &resA);

Aug. 1, 2017

Re: [PATCH 6/6] ws2_32/tests: Added GetAddrInfoExW tests.

by Jacek Caban

On 01.08.2017 18:26, Alexandre Julliard wrote: > Jacek Caban <jacek(a)codeweavers.com> writes: > >> @@ -7334,6 +7343,95 @@ static void test_GetAddrInfoW(void) >> ok(result2 == NULL, "got %p\n", result2); >> } >> >> +static void test_GetAddrInfoExW(void) >> +{ > You are not calling this anywhere. The call didn't survive the last clean up :) Sorry about that, I sent the right version. Jacek

Aug. 1, 2017

Re: [PATCH 6/6] ws2_32/tests: Added GetAddrInfoExW tests.

by Alexandre Julliard

Jacek Caban <jacek(a)codeweavers.com> writes: > @@ -7334,6 +7343,95 @@ static void test_GetAddrInfoW(void) > ok(result2 == NULL, "got %p\n", result2); > } > > +static void test_GetAddrInfoExW(void) > +{ You are not calling this anywhere. -- Alexandre Julliard julliard(a)winehq.org

Aug. 1, 2017

Re: [PATCH v2 2/3] include: Add initial IDL for cmnquery.h

by Alexandre Julliard

Zebediah Figura <zfigura(a)codeweavers.com> writes: > +[ > + object, > + uuid(1a3114b8-a62e-11d0-a6c5-00a0c906af45), > + local > +] > +interface IPersistQuery : IPersist > +{ > + HRESULT Clear(); > + > + HRESULT ReadInt([in] LPCWSTR section, [in] LPCWSTR name, [out] INT *value); > + > + HRESULT ReadString([in] LPCWSTR section, [in] LPCWSTR name, [out, size_is(buflen)] LPWSTR buffer, [in] INT buflen); > + > + HRESULT ReadStruct([in] LPCWSTR section, [in] LPCWSTR name, [out, size_is(buflen)] LPVOID buffer, [in] DWORD buflen); > + > + HRESULT WriteInt([in] LPCWSTR section, [in] LPCWSTR name, [in] INT value); > + > + HRESULT WriteString([in] LPCWSTR section, [in] LPCWSTR name, [in] LPCWSTR value); > + > + HRESULT WriteStruct([in] LPCWSTR section, [in] LPCWSTR name, [in] LPVOID value, [in] DWORD size); > +}; This doesn't match the PSDK, the order matters. -- Alexandre Julliard julliard(a)winehq.org

Aug. 1, 2017

Re: [PATCH v2 2/3] include: Add initial IDL for cmnquery.h

by Alistair Leslie-Hughes

Hi Zebediah, On 01/08/17 05:24, Zebediah Figura wrote: > v2: fix a typo in MSDN > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> > +cpp_quote("DEFINE_GUID(CLSID_CommonQuery, 0x83BC5EC0, 0x6F2A, 0x11D0, 0xA1,0xC4, 0x00,0xAA,0x00,0xC1,0x6E,0x65);") > Please make the GUID lowercase. Best Regards Alistair

Aug. 1, 2017

Re: [PATCH v2 2/3] include: Add initial IDL for cmnquery.h

by Alistair Leslie-Hughes

Hi Zebediah, On 01/08/17 05:24, Zebediah Figura wrote: > v2: fix a typo in MSDN > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> > +cpp_quote("DEFINE_GUID(CLSID_CommonQuery, 0x83BC5EC0, 0x6F2A, 0x11D0, 0xA1,0xC4, 0x00,0xAA,0x00,0xC1,0x6E,0x65);") > Please make the GUID lowercase. Best Regards Alistair

Aug. 1, 2017

Re: [PATCH] kernel32: Load library search in system32 directory.

by Alexandre Julliard

Carlos Palminha <CARLOS.PALMINHA(a)synopsys.com> writes: > On 31-07-2017 19:56, Alexandre Julliard wrote: >> Carlos Palminha <CARLOS.PALMINHA(a)synopsys.com> writes: >> >>> I think today its working for some of those unsupported flags because it's a side effect of calling MODULE_get_dll_load_path with NULL. >>> Probably using only system32 path it might break some of the application that are working based on a side effect. Nevertheless i run the existing tests for module and its ok. >>> >>> I will look at it and add the other SEARCH flags. If i check the other flags then there is no need to fallback to MODULE_get_dll_load_path(NULL), do you agree? >> >> My guess is that you'd want to integrate the flags handling into >> MODULE_get_dll_load_path instead of bypassing it. >> > > i thought on that but it seems wine_kernel_init and SearchPathW are also using it. > Do you think its acceptable for these functions to call dll_load_path with the SEARCH flags? That should be fine. -- Alexandre Julliard julliard(a)winehq.org

Aug. 1, 2017

Re: [PATCH] kernel32: Load library search in system32 directory.

by Carlos Palminha

On 31-07-2017 19:56, Alexandre Julliard wrote: > Carlos Palminha <CARLOS.PALMINHA(a)synopsys.com> writes: > >> I think today its working for some of those unsupported flags because it's a side effect of calling MODULE_get_dll_load_path with NULL. >> Probably using only system32 path it might break some of the application that are working based on a side effect. Nevertheless i run the existing tests for module and its ok. >> >> I will look at it and add the other SEARCH flags. If i check the other flags then there is no need to fallback to MODULE_get_dll_load_path(NULL), do you agree? > > My guess is that you'd want to integrate the flags handling into > MODULE_get_dll_load_path instead of bypassing it. > i thought on that but it seems wine_kernel_init and SearchPathW are also using it. Do you think its acceptable for these functions to call dll_load_path with the SEARCH flags? Regards, C.Palminha

Aug. 1, 2017

Re: [PATCH 3/3] ucrtbase/tests: Add tests for math function errors.

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=32534 Your paranoid android. === wvistau64 (64 bit misc) === misc.c:691: Test failed: log2(-1.#INF00) got exception type 1 misc.c:691: Test failed: log2(-1.000000) got exception type 1 misc.c:691: Test failed: log2(0.000000) got exception type 2 misc.c:721: Test failed: _scalb(1.000000, -1000000000) got exception type -1 === w1064 (64 bit misc) === misc.c:691: Test failed: log2(-1.#INF00) got exception type 1 misc.c:691: Test failed: log2(-1.000000) got exception type 1 misc.c:691: Test failed: log2(0.000000) got exception type 2 misc.c:721: Test failed: _scalb(1.000000, -1000000000) got exception type -1

Aug. 1, 2017

Re: [PATCH] opengl32: Update OpenGL Registry files URLs.

by Austin English

On Mon, Jul 31, 2017 at 5:14 PM, Józef Kucia <jkucia(a)codeweavers.com> wrote: > The OpenGL Registry has been moved. See > https://cvs.khronos.org/svn/repos/ogl/trunk/doc/registry/public/README.txt. > > Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com> > --- > dlls/opengl32/make_opengl | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/dlls/opengl32/make_opengl b/dlls/opengl32/make_opengl > index 45e3f720ea..ef4156686a 100755 > --- a/dlls/opengl32/make_opengl > +++ b/dlls/opengl32/make_opengl > @@ -8,8 +8,8 @@ use XML::Simple; > # > # - It needs files from the OpenGL extension registry: > # > -# https://cvs.khronos.org/svn/repos/ogl/trunk/doc/registry/public/api/gl.xml > -# https://cvs.khronos.org/svn/repos/ogl/trunk/doc/registry/public/api/wgl.xml > +# https://raw.github.com/KhronosGroup/OpenGL-Registry/master/xml/gl.xml > +# https://raw.github.com/KhronosGroup/OpenGL-Registry/master/xml/wgl.xml > # > # If they are not found in the current directory the script will > # attempt to download them from there. > @@ -404,8 +404,8 @@ if ($version eq "1.0") { > # > # Fetch the registry files > # > --f "gl.xml" || system "wget --no-check-certificate https://cvs.khronos.org/svn/repos/ogl/trunk/doc/registry/public/api/gl.xml" || die "cannot download gl.xml"; > --f "wgl.xml" || system "wget --no-check-certificate https://cvs.khronos.org/svn/repos/ogl/trunk/doc/registry/public/api/wgl.xml" || die "cannot download wgl.xml"; > +-f "gl.xml" || system "wget --no-check-certificate https://raw.github.com/KhronosGroup/OpenGL-Registry/master/xml/gl.xml" || die "cannot download gl.xml"; > +-f "wgl.xml" || system "wget --no-check-certificate https://raw.github.com/KhronosGroup/OpenGL-Registry/master/xml/wgl.xml" || die "cannot download wgl.xml"; > > > # > -- > 2.13.0 Do we really want to keep the --no-check-certificate now the file is on github.com? I feel like github's cert is pretty widely trusted, and likely to stay working. Especially since all of *winehq.org is finally on HTTPS.. -- -Austin GPG: 14FB D7EA A041 937B

July 31, 2017