September 2017 - Wine-Devel

Re: [PATCH v2 1/3] mshtml: Add a null parameter check in navigate_new_window().
by Jacek Caban Sept. 15, 2017

Sept. 15, 2017

On 15.09.2017 07:34, Zebediah Figura wrote: > v2: check in caller instead of do_query_service(). > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> > --- > dlls/mshtml/navigate.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/dlls/mshtml/navigate.c b/dlls/mshtml/navigate.c > index 423d650a8c..06425c3f2b 100644 > --- a/dlls/mshtml/navigate.c > +++ b/dlls/mshtml/navigate.c > @@ -2246,6 +2246,12 @@ HRESULT navigate_new_window(HTMLOuterWindow *window, IUri *uri, const WCHAR *nam > nsChannelBSC *bsc; > HRESULT hres; > > + if (!window->doc_obj->client) > + { > + FIXME("no attached client site; cannot query INewWindowManager\n"); > + return E_NOTIMPL; > + } This doesn't look right. Look at HlinkTarget_Navigate. We explicitly choose this code path because client is NULL. This seems to be a regression from commit 63205bf01d6 and the right solution seems to be simply ignoring the fact that we don't have INewWindowManager and continue. Thanks, Jacek

2 1

Re: [PATCH v4 2/2] ieframe: Add stub implementation of InternetExplorerManager.
by Jacek Caban Sept. 15, 2017

Sept. 15, 2017

On 15.09.2017 07:43, Zebediah Figura wrote: > +HRESULT WINAPI InternetExplorerManager_Create(IClassFactory *iface, IUnknown *pOuter, REFIID riid, void **ppv) > +{ > + InternetExplorerManager *ret; > + HRESULT hr; > + > + TRACE("(%p %s %p)\n", pOuter, debugstr_guid(riid), ppv); > + > + if (!(ret = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*ret)))) > + return E_OUTOFMEMORY; > + > + ret->IInternetExplorerManager_iface.lpVtbl = &InternetExplorerManager_vtbl; > + ret->ref = 1; > + > + hr = IInternetExplorerManager_QueryInterface(&ret->IInternetExplorerManager_iface, riid, ppv); > + IInternetExplorerManager_Release(&ret->IInternetExplorerManager_iface); > + > + return hres; That doesn't compile. Jacek

2 1

Re: [PATCH] dsquery: Register CommonQuery coclass.
by Alexandre Julliard Sept. 15, 2017

Sept. 15, 2017

Zebediah Figura <zfigura(a)codeweavers.com> writes: > + > +[ > + threading(apartment), > + uuid(83bc5ec0-6f2a-11d0-a1c4-00aa00c16e65) > +] > +coclass CommonQuery { interface ICommonQuery; } This should most likely go in the public header. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH] d3d9: Ask for texture GetDC() capability GET_DC based on texture format instead of initial swapchain buffer format
by Henri Verbeet Sept. 15, 2017

Sept. 15, 2017

On 15 September 2017 at 07:14, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > Initially application sets buffer format to UNKNOWN that is resolved later > in wined3d to something compatible with GetDC() functionality. > A small test would be great.

1 0

Re: [PATCH 2/2] d2d1: Implement GetBounds() for path geometries
by Henri Verbeet Sept. 15, 2017

Sept. 15, 2017

On 14 September 2017 at 14:10, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > + /* GetBounds() with bezier segments. */ > + hr = ID2D1Factory_CreatePathGeometry(factory, &geometry); > + ok(SUCCEEDED(hr), "Failed to create path geometry, hr %#x.\n", hr); > + hr = ID2D1PathGeometry_Open(geometry, &sink); > + ok(SUCCEEDED(hr), "Failed to open geometry sink, hr %#x.\n", hr); > + fill_geometry_sink_bezier(sink); > + hr = ID2D1GeometrySink_Close(sink); > + ok(SUCCEEDED(hr), "Failed to close geometry sink, hr %#x.\n", hr); > + ID2D1GeometrySink_Release(sink); > + > + set_rect(&rect, 0.0f, 0.0f, 0.0f, 0.0f); > + hr = ID2D1PathGeometry_GetBounds(geometry, NULL, &rect); > + ok(SUCCEEDED(hr), "Failed to get geometry bounds, hr %#x.\n", hr); > + match = compare_rect(&rect, 5.0f, 20.0f, 75.0f, 752.0f, 0); > + ok(match, "Got unexpected rectangle {%.8e, %.8e, %.8e, %.8e}.\n", > + rect.left, rect.top, rect.right, rect.bottom); Note that the way fill_geometry_sink_bezier() is constructed, the bounds tend to coincide with the control points, probably even under a carefully chosen rotation. The figure from test_bezier_intersect() under various rotations may be more interesting.

1 0

Re: [PATCH 1/2] d2d1: Use better bezier segment bounds approximation when updating figure bounds
by Henri Verbeet Sept. 15, 2017

Sept. 15, 2017

On 14 September 2017 at 14:10, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > --- > dlls/d2d1/geometry.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > This is fine, but note that d2d_geometry_sink_AddQuadraticBeziers() also exists.

1 0

Re: [PATCH v2 1/2] secur32: Fix race between schan_(Encrypt|Decrypt)Message
by Jacek Caban Sept. 15, 2017

Sept. 15, 2017

Hi Anton, On 14.09.2017 18:30, Anton Romanov wrote: > ctx->req_ctx_attr = fContextReq; > > - transport.ctx = ctx; > - init_schan_buffers(&transport.in, pInput, schan_init_sec_ctx_get_next_input_buffer); > - transport.in.limit = expected_size; > - init_schan_buffers(&transport.out, pOutput, schan_init_sec_ctx_get_next_output_buffer); > - schan_imp_set_session_transport(ctx->session, &transport); > + ctx->transport.ctx = ctx; One last thing, please move this line closer to schan_imp_create_session, where it belongs. transport.ctx should be set once, during context creation, and not touched later. Other than that, the patch looks good. Thanks, Jacek

2 2

Re: [2/10] wsdapi: Add helper functions for populating XML strings.
by Huw Davies Sept. 15, 2017

Sept. 15, 2017

On Thu, Sep 14, 2017 at 10:54:17PM +0100, Owen Rudge wrote: > Signed-off-by: Owen Rudge <orudge(a)codeweavers.com> > --- > dlls/wsdapi/Makefile.in | 3 +- > dlls/wsdapi/soap.c | 86 > +++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 88 insertions(+), 1 deletion(-) > create mode 100644 dlls/wsdapi/soap.c > > Unfortunately adding unused helper functions isn't the right way to split up the patches. You'll need to find a way to break up the functionality while incrementally adding these things. > diff --git a/dlls/wsdapi/soap.c b/dlls/wsdapi/soap.c > new file mode 100644 > index 0000000..9662203 > --- /dev/null > +++ b/dlls/wsdapi/soap.c > +static inline void free_xml_string(WS_XML_STRING *value) > +{ > + HANDLE heap; > + > + if (value == NULL) > + return; > + > + heap = GetProcessHeap(); > + > + if (value->bytes != NULL) > + HeapFree(heap, 0, value->bytes); > + > + HeapFree(heap, 0, value); > +} > GetProcessHeap() is cheap, you don't need to bother caching it. You may want to consider using the standardized heap_alloc() / heap_free() helpers. Huw.

1 0

Re: [PATCH 1/3] mshtml: Add a null parameter check in do_query_service().
by Jacek Caban Sept. 14, 2017

Sept. 14, 2017

Hi Zebediah, On 14.09.2017 20:42, Zebediah Figura wrote: > diff --git a/dlls/mshtml/main.c b/dlls/mshtml/main.c > index af716d1680..43270b9cc8 100644 > --- a/dlls/mshtml/main.c > +++ b/dlls/mshtml/main.c > @@ -190,6 +190,9 @@ HRESULT do_query_service(IUnknown *unk, REFGUID guid_service, REFIID riid, void > IServiceProvider *sp; > HRESULT hres; > > + if (!unk) > + return E_FAIL; I think it would be cleaner to add NULL check to caller. Thanks, Jacek

2 1

Re: [PATCH v2 2/2] secur32: Protect SSLRead/Write with cs on OSX
by Jacek Caban Sept. 14, 2017

Sept. 14, 2017

Hi Anton, On 14.09.2017 18:30, Anton Romanov wrote: > @@ -742,6 +743,9 @@ BOOL schan_imp_create_session(schan_imp_session *session, schan_credentials *cre > if (!s) > return FALSE; > > + InitializeCriticalSection(&s->cs); > + DEBUG_SET_CRITSEC_NAME(&s->cs, "schannel_session"); AFAICS this won't compile, this macro is not defined. If you don't have Mac to test, that's fine. I will fix it and test for you. Jacek

1 0

Re: [PATCH 1/2] secur32: Split push/pull schannel transports
by Jacek Caban Sept. 14, 2017

Sept. 14, 2017

On 13.09.2017 07:33, Anton Romanov wrote: > This makes schan_EncryptMessage/schan_DecryptMessage thread safe between > each other. > MSDN states that it's supposed to be thread safe. > Fixes Magic The Gathering: Online crash: > https://bugs.winehq.org/show_bug.cgi?id=43453 > > Signed-off-by: Anton Romanov <theli.ua(a)gmail.com> > --- > dlls/secur32/schannel.c | 33 +++++++++++++++++++-------------- > dlls/secur32/schannel_gnutls.c | 12 +++++++----- > dlls/secur32/schannel_macosx.c | 21 ++++++++++++--------- > dlls/secur32/secur32_priv.h | 5 +++-- > 4 files changed, 41 insertions(+), 30 deletions(-) > > diff --git a/dlls/secur32/schannel.c b/dlls/secur32/schannel.c > index 82374efd55..1f938e1a37 100644 > --- a/dlls/secur32/schannel.c > +++ b/dlls/secur32/schannel.c > @@ -59,6 +59,8 @@ struct schan_context > schan_imp_session session; > ULONG req_ctx_attr; > const CERT_CONTEXT *cert; > + struct schan_transport *push_transport; > + struct schan_transport *pull_transport; Do you really need separated transports? schan_transport already has separated in and out buffers. It seems to me that as long as you use only in buffers in reader and out buffers in writer (and it's already the case), there is no need for separated transport instances. Also, why do you use pointers here? Couldn't you just store the struct inside schan_context? > diff --git a/dlls/secur32/schannel_gnutls.c b/dlls/secur32/schannel_gnutls.c > index bc3bbaf67d..97b2c2d767 100644 > --- a/dlls/secur32/schannel_gnutls.c > +++ b/dlls/secur32/schannel_gnutls.c > @@ -73,7 +73,7 @@ MAKE_FUNCPTR(gnutls_record_send); > MAKE_FUNCPTR(gnutls_server_name_set); > MAKE_FUNCPTR(gnutls_transport_get_ptr); > MAKE_FUNCPTR(gnutls_transport_set_errno); > -MAKE_FUNCPTR(gnutls_transport_set_ptr); > +MAKE_FUNCPTR(gnutls_transport_set_ptr2); It's related to the previous comment, it seems to me that gnutls_transport_set_ptr should be enough. Thanks, Jacek

2 1

Re: [PATCH v2] riched20: Add more warnings for unhandled control words.
by Huw Davies Sept. 14, 2017

Sept. 14, 2017

On Thu, Sep 14, 2017 at 01:03:11AM -0500, Zebediah Figura wrote: > v2: use FIXME instead of WARN; use debugstr_a (thanks Huw) Unfortunately, these generate a lot of noise (especially in the tests). We probably want to deal with things like \rtf and \uc before this goes in. Huw.

1 0

Re: [PATCH] riched20: Add more warnings for unhandled control words.
by Huw Davies Sept. 14, 2017

Sept. 14, 2017

On Tue, Sep 12, 2017 at 09:43:10AM -0500, Zebediah Figura wrote: > Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> > --- > dlls/riched20/editor.c | 6 ++++++ > dlls/riched20/reader.c | 15 +++++++++++++-- > 2 files changed, 19 insertions(+), 2 deletions(-) > > diff --git a/dlls/riched20/editor.c b/dlls/riched20/editor.c > index a7b1bc748f..f6badb1a36 100644 > --- a/dlls/riched20/editor.c > +++ b/dlls/riched20/editor.c > @@ -533,6 +533,8 @@ void ME_RTFCharAttrHook(RTF_Info *info) > if (info->rtfParam != rtfNoParam) > fmt.yHeight = info->rtfParam*10; > break; > + default: > + WARN("unknown token \"%s\"\n", info->rtfTextBuf); These should be FIXME()s not WARN()s. It would also be better to use debugstr_a() on the string. > @@ -2450,7 +2454,10 @@ static void > Destination (RTF_Info *info) > { > if (!RTFGetDestinationCallback(info, info->rtfMinor)) > + { > + WARN("unknown token \"%s\"\n", info->rtfTextBuf); > RTFSkipGroup (info); > + } > } Now would be a good opportunity to get rid of the tabs from this function. Huw.

2 1

Re: [PATCH 5/7] msi: Perform the given action, or INSTALL, from MsiInstallProduct().
by Hans Leidekker Sept. 13, 2017

Sept. 13, 2017

On Tue, 2017-09-12 at 20:51 -0500, Zebediah Figura wrote: > The removed code to re-run all actions after InstallValidate is old > and not supported by tests. This makes the tests fail here: err:msi:ITERATE_Actions Execution halted, action L"Run C" returned 1603 Makefile:326: recipe for target 'install.ok' failed make: *** [install.ok] Error 1

2 2

Re: [PATCH 3/7] msi: More fully implement the ExecuteAction action.
by Hans Leidekker Sept. 13, 2017

Sept. 13, 2017

On Tue, 2017-09-12 at 20:51 -0500, Zebediah Figura wrote: > +static UINT iterate_properties(MSIRECORD *record, void *param) > +{ > + static const WCHAR prop_template[] = > + {'P','r','o','p','e','r','t','y','(','S',')',':',' ','[','1',']',' ','=',' ','[','2',']',0}; > + MSIRECORD *uirow; > + > + uirow = MSI_CloneRecord(record); > + if (!uirow) return ERROR_OUTOFMEMORY; > + MSI_RecordSetStringW(uirow, 0, prop_template); > + MSI_ProcessMessage((MSIPACKAGE *)param, INSTALLMESSAGE_INFO|MB_ICONHAND, uirow); You don't need this cast. > static UINT ACTION_ExecuteAction(MSIPACKAGE *package) > { > + static const WCHAR prop_query[] = > + {'S','E','L','E','C','T',' ','*',' ','F','R','O','M',' ','`','_','P','r','o','p','e','r','t','y','`',0}; > + WCHAR *productname; > + WCHAR *action = msi_dup_property(package->db, szEXECUTEACTION); > + MSIQUERY *view; > + MSIRECORD *uirow; > UINT rc; > > - package->script->InWhatSequence |= SEQUENCE_EXEC; > - rc = ACTION_ProcessExecSequence(package,FALSE); > + if (!action) action = msi_strdupW(szINSTALL, strlenW(szINSTALL)); action is leaked. > + /* Send COMMONDATA and INFO messages. */ > + /* FIXME: when should these messages be sent? [see also MsiOpenPackage()] */ > + uirow = MSI_CreateRecord(3); > + if (!uirow) return ERROR_OUTOFMEMORY; > + MSI_RecordSetStringW(uirow, 0, NULL); > + MSI_RecordSetInteger(uirow, 1, 0); > + MSI_RecordSetInteger(uirow, 2, package->num_langids ? package->langids[0] : 0); > + MSI_RecordSetInteger(uirow, 3, msi_get_string_table_codepage(package->db->strings)); > + MSI_ProcessMessageVerbatim(package, INSTALLMESSAGE_COMMONDATA, uirow); > + /* FIXME: send INSTALLMESSAGE_PROGRESS */ > + MSI_ProcessMessageVerbatim(package, INSTALLMESSAGE_COMMONDATA, uirow); > + MSI_ProcessMessage(package, INSTALLMESSAGE_COMMONDATA, uirow); > + > + productname = msi_dup_property(package->db, INSTALLPROPERTY_PRODUCTNAMEW); productname is leaked on error paths.

2 1

Re: [PATCH 4/4] mshtml: Expose IHTMLElement6 to scripts.
by Marvin Sept. 13, 2017

Sept. 13, 2017

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=33084 Your paranoid android. === w7u (32 bit htmldoc) === htmldoc.c:2682: Test failed: unexpected call GetOverrideKeyPath htmldoc.c:5092: Test failed: unexpected call {d9befc84-f21e-4166-874b-a8d3010dc764}

1 0

Re: [PATCH 5/5] wined3d: Don't take the upload code path if the destination surface is onscreen.
by Henri Verbeet Sept. 13, 2017

Sept. 13, 2017

On 12 September 2017 at 21:22, Matteo Bruni <mbruni(a)codeweavers.com> wrote: > Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com> > --- > dlls/wined3d/surface.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > Why would we want to avoid that?

2 2

Re: [v4 1/4] strmiids: Add evr interface GUIDS
by Alexandre Julliard Sept. 12, 2017

Sept. 12, 2017

Fabian Maurer <dark.shadow4(a)web.de> writes: > evr GUIDs should be defined in strmiids.lib This seems to be adding many GUIDs that are not supposed to be there. -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: [PATCH] comctl32/tests: Dump sequence correctly
by Nikolay Sivov Sept. 12, 2017

Sept. 12, 2017

On 09/12/2017 07:17 PM, Fabian Maurer wrote: > The "expected" pointer already got moved in the function, > make sure to use the full list of expected messages. Yes, that makes sense, thanks. Please make it look closer to how it's done in user32, see attached diff.

2 1

Re: [PATCH 1/2] comctl32/taskdialog: Initial support for callback procedure
by Fabian Maurer Sept. 12, 2017

Sept. 12, 2017

Hi, Sorry, but there's a bunch of issues I need to point out. Of course we could address them later, but it'll mean rewriting a bunch of code, and I'd like to have it correct from the beginning. In general, I think this implementation limits us too much. For example, I don't think it's a good idea to have the TASKDIALOGCONFIG structure being filled in run_test_, we need to be able to control it easily. Second, we also need to be able to return different values in the callback, the current approach doesn't account for that. And the most important part, the separation from messages to send and received messages to test against. It's better to send the next message only when the last message arrived, to make sure we process them in the correct order. I attached a patch to show the problem (apply on top of my in-review patch). When sending all messages together, the first message is skipped, and we don't want that to happen. Please reconsider my approach, I can try to simplify it if you want, but I think the complexity is justified to solve a complicated problem. Regards, Fabian Maurer

2 3

Re: [v4 2/4] evr: Add tests
by Marvin Sept. 12, 2017

Sept. 12, 2017

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=33066 Your paranoid android. === build (build) === Make failed

2 1

Re: [1/6] wsdapi: Add wsdapi_internal.h for shared internal function and macro definitions. [try 3]
by Owen Rudge Sept. 12, 2017

Sept. 12, 2017

Hi Alexandre, Is there anything else that needs improving for these patches to be accepted? Thanks, -- Owen Rudge http://www.owenrudge.net/

2 1

Re: [PATCH 7/7] mshtml: Added IHTMLDocument7::getElementsByClassName implementation.
by Marvin Sept. 12, 2017

Sept. 12, 2017

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=33062 Your paranoid android. === w7u (32 bit htmldoc) === htmldoc.c:2682: Test failed: unexpected call GetOverrideKeyPath

1 0

Re: [PATCH 6/7] mshtml: Added IHTMLDocument7::head property implementation.
by Marvin Sept. 12, 2017

Sept. 12, 2017

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=33061 Your paranoid android. === w7u (32 bit htmldoc) === htmldoc.c:2682: Test failed: unexpected call GetOverrideKeyPath htmldoc.c:5092: Test failed: unexpected call {d9befc84-f21e-4166-874b-a8d3010dc764}

1 0

Re: [PATCH 5/7] mshtml: Expose IHTMLDocument7 interface to scripts.
by Marvin Sept. 12, 2017

Sept. 12, 2017

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=33060 Your paranoid android. === w7u (32 bit htmldoc) === htmldoc.c:2682: Test failed: unexpected call GetOverrideKeyPath

1 0