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

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