September 2017 - Wine-Devel

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