- wine-devel - winehq.org

Re: [PATCH] secur32: Split push/pull schannel transports
by Anton Romanov 11 Sep '17

11 Sep '17

On Sat, Aug 5, 2017 at 11:27 AM, Anton Romanov <theli.ua(a)gmail.com> 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; > }; > > static struct schan_handle *schan_handle_table; > @@ -895,7 +897,7 @@ static SECURITY_STATUS SEC_ENTRY schan_InitializeSecurityContextW( > 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); > + schan_imp_set_session_transports(ctx->session, &transport, &transport); > > /* Perform the TLS handshake */ > ret = schan_imp_handshake(ctx->session); > @@ -927,6 +929,13 @@ static SECURITY_STATUS SEC_ENTRY schan_InitializeSecurityContextW( > if (ctx->req_ctx_attr & ISC_REQ_STREAM) > *pfContextAttr |= ISC_RET_STREAM; > > + ctx->push_transport = HeapAlloc(GetProcessHeap(), 0, sizeof(struct schan_transport)); > + ctx->push_transport->ctx = ctx; > + ctx->pull_transport = HeapAlloc(GetProcessHeap(), 0, sizeof(struct schan_transport)); > + ctx->pull_transport->ctx = ctx; > + schan_imp_set_session_transports(ctx->session, ctx->push_transport, > + ctx->pull_transport); > + > return ret; > } > > @@ -1205,7 +1214,6 @@ static int schan_encrypt_message_get_next_buffer_token(const struct schan_transp > static SECURITY_STATUS SEC_ENTRY schan_EncryptMessage(PCtxtHandle context_handle, > ULONG quality, PSecBufferDesc message, ULONG message_seq_no) > { > - struct schan_transport transport; > struct schan_context *ctx; > struct schan_buffers *b; > SECURITY_STATUS status; > @@ -1235,13 +1243,11 @@ static SECURITY_STATUS SEC_ENTRY schan_EncryptMessage(PCtxtHandle context_handle > data = HeapAlloc(GetProcessHeap(), 0, data_size); > memcpy(data, buffer->pvBuffer, data_size); > > - transport.ctx = ctx; > - init_schan_buffers(&transport.in, NULL, NULL); > + init_schan_buffers(&ctx->push_transport->in, NULL, NULL); > if (schan_find_sec_buffer_idx(message, 0, SECBUFFER_STREAM_HEADER) != -1) > - init_schan_buffers(&transport.out, message, schan_encrypt_message_get_next_buffer); > + init_schan_buffers(&ctx->push_transport->out, message, schan_encrypt_message_get_next_buffer); > else > - init_schan_buffers(&transport.out, message, schan_encrypt_message_get_next_buffer_token); > - schan_imp_set_session_transport(ctx->session, &transport); > + init_schan_buffers(&ctx->push_transport->out, message, schan_encrypt_message_get_next_buffer_token); > > length = data_size; > status = schan_imp_send(ctx->session, data, &length); > @@ -1251,7 +1257,7 @@ static SECURITY_STATUS SEC_ENTRY schan_EncryptMessage(PCtxtHandle context_handle > if (length != data_size) > status = SEC_E_INTERNAL_ERROR; > > - b = &transport.out; > + b = &ctx->push_transport->out; > b->desc->pBuffers[b->current_buffer_idx].cbBuffer = b->offset; > HeapFree(GetProcessHeap(), 0, data); > > @@ -1327,7 +1333,6 @@ static void schan_decrypt_fill_buffer(PSecBufferDesc message, ULONG buffer_type, > static SECURITY_STATUS SEC_ENTRY schan_DecryptMessage(PCtxtHandle context_handle, > PSecBufferDesc message, ULONG message_seq_no, PULONG quality) > { > - struct schan_transport transport; > struct schan_context *ctx; > SecBuffer *buffer; > SIZE_T data_size; > @@ -1371,11 +1376,9 @@ static SECURITY_STATUS SEC_ENTRY schan_DecryptMessage(PCtxtHandle context_handle > data_size = expected_size - 5; > data = HeapAlloc(GetProcessHeap(), 0, data_size); > > - transport.ctx = ctx; > - init_schan_buffers(&transport.in, message, schan_decrypt_message_get_next_buffer); > - transport.in.limit = expected_size; > - init_schan_buffers(&transport.out, NULL, NULL); > - schan_imp_set_session_transport(ctx->session, &transport); > + init_schan_buffers(&ctx->pull_transport->in, message, schan_decrypt_message_get_next_buffer); > + ctx->pull_transport->in.limit = expected_size; > + init_schan_buffers(&ctx->pull_transport->out, NULL, NULL); > > while (received < data_size) > { > @@ -1573,6 +1576,8 @@ void SECUR32_deinitSchannelSP(void) > { > struct schan_context *ctx = schan_free_handle(i, SCHAN_HANDLE_CTX); > schan_imp_dispose_session(ctx->session); > + HeapFree(GetProcessHeap(), 0, ctx->push_transport); > + HeapFree(GetProcessHeap(), 0, ctx->pull_transport); > HeapFree(GetProcessHeap(), 0, ctx); > } > } > 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); > MAKE_FUNCPTR(gnutls_transport_set_pull_function); > MAKE_FUNCPTR(gnutls_transport_set_push_function); > #undef MAKE_FUNCPTR > @@ -214,11 +214,13 @@ void schan_imp_dispose_session(schan_imp_session session) > pgnutls_deinit(s); > } > > -void schan_imp_set_session_transport(schan_imp_session session, > - struct schan_transport *t) > +void schan_imp_set_session_transports(schan_imp_session session, > + struct schan_transport *t_push, > + struct schan_transport *t_pull) > { > gnutls_session_t s = (gnutls_session_t)session; > - pgnutls_transport_set_ptr(s, (gnutls_transport_ptr_t)t); > + pgnutls_transport_set_ptr2(s, (gnutls_transport_ptr_t)t_pull, > + (gnutls_transport_ptr_t)t_push); > } > > void schan_imp_set_session_target(schan_imp_session session, const char *target) > @@ -568,7 +570,7 @@ BOOL schan_imp_init(void) > LOAD_FUNCPTR(gnutls_server_name_set) > LOAD_FUNCPTR(gnutls_transport_get_ptr) > LOAD_FUNCPTR(gnutls_transport_set_errno) > - LOAD_FUNCPTR(gnutls_transport_set_ptr) > + LOAD_FUNCPTR(gnutls_transport_set_ptr2) > LOAD_FUNCPTR(gnutls_transport_set_pull_function) > LOAD_FUNCPTR(gnutls_transport_set_push_function) > #undef LOAD_FUNCPTR > diff --git a/dlls/secur32/schannel_macosx.c b/dlls/secur32/schannel_macosx.c > index 7f38133b4b..4cd562411b 100644 > --- a/dlls/secur32/schannel_macosx.c > +++ b/dlls/secur32/schannel_macosx.c > @@ -184,7 +184,8 @@ enum { > > struct mac_session { > SSLContextRef context; > - struct schan_transport *transport; > + struct schan_transport *push_transport; > + struct schan_transport *pull_transport; > }; > > > @@ -631,9 +632,9 @@ static OSStatus schan_pull_adapter(SSLConnectionRef transport, void *buff, > int status; > OSStatus ret; > > - TRACE("(%p/%p, %p, %p/%lu)\n", s, s->transport, buff, buff_len, *buff_len); > + TRACE("(%p/%p, %p, %p/%lu)\n", s, s->pull_transport, buff, buff_len, *buff_len); > > - status = schan_pull(s->transport, buff, buff_len); > + status = schan_pull(s->pull_transport, buff, buff_len); > if (status == 0) > { > if (*buff_len == 0) > @@ -690,9 +691,9 @@ static OSStatus schan_push_adapter(SSLConnectionRef transport, const void *buff, > int status; > OSStatus ret; > > - TRACE("(%p/%p, %p, %p/%lu)\n", s, s->transport, buff, buff_len, *buff_len); > + TRACE("(%p/%p, %p, %p/%lu)\n", s, s->push_transport, buff, buff_len, *buff_len); > > - status = schan_push(s->transport, buff, buff_len); > + status = schan_push(s->push_transport, buff, buff_len); > if (status == 0) > { > TRACE("Pushed %lu bytes\n", *buff_len); > @@ -806,14 +807,16 @@ void schan_imp_dispose_session(schan_imp_session session) > HeapFree(GetProcessHeap(), 0, s); > } > > -void schan_imp_set_session_transport(schan_imp_session session, > - struct schan_transport *t) > +void schan_imp_set_session_transports(schan_imp_session session, > + struct schan_transport *t_push, > + struct schan_transport *t_pull) > { > struct mac_session *s = (struct mac_session*)session; > > - TRACE("(%p/%p, %p)\n", s, s->context, t); > + TRACE("(%p/%p, %p, %p)\n", s, s->context, t_push, t_pull); > > - s->transport = t; > + s->push_transport = t_push; > + s->pull_transport = t_pull; > } > > void schan_imp_set_session_target(schan_imp_session session, const char *target) > diff --git a/dlls/secur32/secur32_priv.h b/dlls/secur32/secur32_priv.h > index 9a60d65788..0bc8a57c1a 100644 > --- a/dlls/secur32/secur32_priv.h > +++ b/dlls/secur32/secur32_priv.h > @@ -246,8 +246,9 @@ extern schan_imp_session schan_session_for_transport(struct schan_transport* t) > /* schannel implementation interface */ > extern BOOL schan_imp_create_session(schan_imp_session *session, schan_credentials *cred) DECLSPEC_HIDDEN; > extern void schan_imp_dispose_session(schan_imp_session session) DECLSPEC_HIDDEN; > -extern void schan_imp_set_session_transport(schan_imp_session session, > - struct schan_transport *t) DECLSPEC_HIDDEN; > +extern void schan_imp_set_session_transports(schan_imp_session session, > + struct schan_transport *t_push, > + struct schan_transport *t_pull) DECLSPEC_HIDDEN; > extern void schan_imp_set_session_target(schan_imp_session session, const char *target) DECLSPEC_HIDDEN; > extern SECURITY_STATUS schan_imp_handshake(schan_imp_session session) DECLSPEC_HIDDEN; > extern unsigned int schan_imp_get_session_cipher_block_size(schan_imp_session session) DECLSPEC_HIDDEN; > -- > 2.11.0 > So, it's been a month now. Is there something I could do to have this accepted? Thanks.

3 6

Re: [PATCH] msvcrt: Protect onexit table with critical section
by Piotr Caban 11 Sep '17

11 Sep '17

Hi Daniel, Probably you shouldn't call the registered onexit function while holding the lock. Thanks, Piotr

1 0

Re: [PATCH resend] configure: Ensure that $includedir/wine is empty when deleted.
by Alexandre Julliard 11 Sep '17

11 Sep '17

Zebediah Figura <zfigura(a)codeweavers.com> writes: > Fixes a minor bug when running 'make uninstall'. > > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> > --- > configure.ac | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) This can't work: Makefile:24387: *** target file '__uninstall__' has both : and :: entries. Stop. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH v2 2/3] shell32: Implement FolderItems_Item.
by Alex Henrie 11 Sep '17

11 Sep '17

2017-09-10 22:43 GMT-06:00 Alex Henrie <alexhenrie24(a)gmail.com>: > v2: > - Changed E_OUTOFMEMORY to ERROR_BUFFER_OVERFLOW Forgot to mention: - Started rejecting paths with relative (non-canonical) components -Alex

1 0

Re: [PATCH 2/3] shell32: Implement FolderItems_Item.
by Nikolay Sivov 11 Sep '17

11 Sep '17

On 08.09.2017 8:29, Alex Henrie wrote: > typedef struct { > FolderItems3 FolderItems3_iface; > LONG ref; > + VARIANT dir; > + WCHAR **item_filenames; > + LONG item_count; > } FolderItemsImpl; If 'dir' is always a string maybe store it as such? > static HRESULT WINAPI FolderItemsImpl_Item(FolderItems3 *iface, VARIANT index, FolderItem **ppid) > { > - FIXME("(%p,%s,%p)\n", iface, debugstr_variant(&index), ppid); > + FolderItemsImpl *This = impl_from_FolderItems(iface); > + WCHAR path_str[MAX_PATH]; > + VARIANT path_var; > + HRESULT ret; > + > + TRACE("(%p,%s,%p)\n", iface, debugstr_variant(&index), ppid); > > *ppid = NULL; > - return E_NOTIMPL; > + > + if (!PathIsDirectoryW(V_BSTR(&This->dir))) > + return S_FALSE; How could it not be a directory? > + TRACE("(%p,%s,%p)\n", iface, debugstr_variant(&index), ppid); Could add some spaces in argument list? Most of the wine code is doing that I think. > + if (!PathCombineW(path_str, V_BSTR(&This->dir), This->item_filenames[V_I4(&index)])) > + return E_OUTOFMEMORY; Why E_OUTOFMEMORY? > + do > + { > + if (!strcmpW(file_info.cFileName, dot) || !strcmpW(file_info.cFileName, dot_dot)) > + continue; > + > + if (This->item_count >= item_size) > + { > + item_size *= 2; > + filenames = HeapReAlloc(GetProcessHeap(), 0, This->item_filenames, item_size * sizeof(WCHAR*)); > + if (!filenames) > + goto fail; > + This->item_filenames = filenames; > + } > + > + This->item_filenames[This->item_count] = strdupW(file_info.cFileName); > + if (!This->item_filenames[This->item_count]) > + goto fail; > + This->item_count++; > + } > + while (FindNextFileW(first_file, &file_info)); Do we have tests showing that it takes a snapshot like that? For example if new file is created after FolderItems was created, is it accessible with BSTR index for example?

2 1

Re: [PATCH 1/3] shell32/tests: Add tests for FolderItems_Item and FolderItems_get_Count.
by Nikolay Sivov 11 Sep '17

11 Sep '17

On 08.09.2017 8:29, Alex Henrie wrote: > + for (i = 0; i < sizeof(file_defs)/sizeof(file_defs[0]); i++) > + { > + switch (file_defs[i].type) > + { > + case DIRECTORY: > + r = CreateDirectoryA(file_defs[i].name, NULL); > + ok(r, "CreateDirectory failed: %08x\n", GetLastError()); > + PathCombineA(buf, file_defs[i].name, "foo.txt"); > + file = CreateFileA(buf, 0, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); > + ok(file != INVALID_HANDLE_VALUE, "CreateFile failed: %08x\n", GetLastError()); > + CloseHandle(file); > + break; > + > + case EMPTY: > + file = CreateFileA(file_defs[i].name, 0, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); > + ok(file != INVALID_HANDLE_VALUE, "CreateFile failed: %08x\n", GetLastError()); > + CloseHandle(file); > + break; > + > + case RANDOM: > + file = CreateFileA(file_defs[i].name, GENERIC_WRITE, 0, NULL, > + CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); > + ok(file != INVALID_HANDLE_VALUE, "CreateFile failed: %08x\n", GetLastError()); > + for (j = 0; j < sizeof(buf); j++) > + buf[j] = rand() % 256; > + r = WriteFile(file, buf, sizeof(buf), &dwcount, NULL); > + ok(r, "WriteFile failed: %08x\n", GetLastError()); > + CloseHandle(file); > + break; > + } > + } Why would it matter if file is empty or not? And if it does for some reason, why filling it with random content? > + MultiByteToWideChar(CP_ACP, 0, file_defs[0].name, -1, wstr, MAX_PATH); > + V_VT(&var) = VT_BSTR; > + V_BSTR(&var) = SysAllocString(wstr); I understand why you have test names as single byte strings, but this construct is used more than once, so maybe add a helper for that. Could be similar to _bstr_() in msxml test, but without global pointer array. > + for (i = -10; i < 10; i++) > + { > + V_VT(&var) = VT_ERROR; > + V_ERROR(&var) = i; VT_ERROR is used for optional arguments, what does [-10,10) range mean? I think it's enough to test some HRESULT-style failure code. > + ok(!!bstr, "bstr is null\n"); > + ok(!lstrcmpW(bstr, cur_dir), > + "expected %s, got %s\n", wine_dbgstr_w(cur_dir), wine_dbgstr_w(bstr)); First check seems redundant.

2 5

Re: [PATCH v2 6/7] evr: Add EnhancedVideoRenderer coclass
by Nikolay Sivov 10 Sep '17

10 Sep '17

On 10.09.2017 18:16, Fabian Maurer wrote: > +typedef struct > +{ > + BaseFilter basefilter; > + > + IUnknown IUnknown_inner; > + LONG ref; > +} evr_impl; I don't think you need a separate refcount field. Also it's not called "basefilter" in other quartz components. In fact you don't use it at all it seems in patch 6/7, and I think 7/7 is short enough to merge it with 6/7. > + ERR("(%p)->(%s,%p),not found\n", This, debugstr_guid(riid), ppv); > + return E_NOINTERFACE; I think this is usually FIXME or WARN. > ... Style-wise, I think naming could be improved, for example "impl" part does not add anything.

2 1

Re: [PATCH v2 4/7] evr: Add tests
by Marvin 10 Sep '17

10 Sep '17

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

2 1

Re: [PATCH] test.patch
by Fabian Maurer 10 Sep '17

10 Sep '17

Please ignore this patch, didn't mean to send that. Regards, Fabian Maurer

1 0

Re: [PATCH 4/7] evr: Add tests
by Marvin 10 Sep '17

10 Sep '17

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=33019 Your paranoid android. === build (build) === Patch failed to apply

1 0

Re: [PATCH] d2d1: Partially implement StrokeContainsPoint() for rectangles
by Henri Verbeet 08 Sep '17

08 Sep '17

On 8 September 2017 at 16:11, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > +static BOOL d2d_point_on_segment(const D2D1_POINT_2F *q, const D2D1_POINT_2F *p0, > + const D2D1_POINT_2F *p1, float tolerance) > +{ > + float dot, edge_length, distance; > + D2D1_POINT_2F v_p, v_q, n; > + > + d2d_point_subtract(&v_p, p1, p0); > + if ((edge_length = d2d_point_length(&v_p)) == 0.0f) > + return FALSE; > + > + /* Edge distance. */ > + d2d_point_subtract(&v_q, q, p0); > + > + n.x = -v_p.y; > + n.y = v_p.x; Unfortunately it's not that easy when "transform" is a world transformation instead of a geometry transformation. I.e., when you have for example a shear transformation, the "normal" is no longer necessarily perpendicular to the line segment. Put a different way, you're potentially testing against a parallelogram instead of a rectangle. In a sense you account for that with stroke_widths[], but see below. > + distance = fabsf(d2d_point_dot(&v_q, &n)) / edge_length; > + if (distance >= tolerance) > + return FALSE; > + > + /* Test if normal and edge intersect. */ > + dot = d2d_point_dot(&v_q, &v_p); > + return dot >= 0.0f && dot <= edge_length * edge_length; > +} Likewise, this doesn't do the right thing under all transformations. > + d2d_point_transform(&v_s[0], transform, stroke_width, 0.0f); > + d2d_point_transform(&v_s[1], transform, stroke_width, 0.0f); > + d2d_point_transform(&origin, transform, 0.0f, 0.0f); > + > + d2d_point_subtract(&v_s[0], &v_s[0], &origin); > + d2d_point_subtract(&v_s[1], &v_s[1], &origin); > + > + stroke_widths[0] = d2d_point_length(&v_s[0]); > + stroke_widths[1] = d2d_point_length(&v_s[1]); This ends up with the same value for stroke_widths[0] and stroke_widths[1]. > + d2d_point_subtract(&v, &point, &vertices[i]); > + > + if (d2d_point_dot(&v, &v) < tolerance * tolerance) > + { > + *contains = TRUE; > + break; > + } This tests around the corner of the joins, but note that there's potentially a space between that corner and the start of the line segment that this would miss. > + else > + { > + D2D1_POINT_2F d, s; > + > + s.x = rect->right - rect->left; > + s.y = rect->bottom - rect->top; > + d.x = fabsf((rect->right + rect->left) * 0.5f - point.x); > + d.y = fabsf((rect->bottom + rect->top) * 0.5f - point.y); > + > + /* Inside test. */ > + if (d.x <= (s.x - stroke_width) * 0.5f - tolerance && d.y <= (s.y - stroke_width) * 0.5f - tolerance) > + { > + *contains = FALSE; > + return S_OK; > + } > + > + if (tolerance == 0.0f) > + { > + *contains = d.x < (s.x + stroke_width) * 0.5f && d.y < (s.y + stroke_width) * 0.5f; > + } > + else > + { > + d.x = max(d.x - (s.x + stroke_width) * 0.5f, 0.0f); > + d.y = max(d.y - (s.y + stroke_width) * 0.5f, 0.0f); > + > + *contains = d2d_point_dot(&d, &d) < tolerance * tolerance; > + } > + } Ultimately it's not that important, but I think there's something to be said for handling the "!transform" case first as a special case. I.e., if (!transform) { ... return S_OK; } /* General case. */ > diff --git a/dlls/d2d1/tests/d2d1.c b/dlls/d2d1/tests/d2d1.c > index f6261295b6..95a010b13e 100644 > --- a/dlls/d2d1/tests/d2d1.c > +++ b/dlls/d2d1/tests/d2d1.c > @@ -76,6 +76,15 @@ struct expected_geometry_figure > const struct geometry_segment *segments; > }; > > +struct contains_point_test > +{ > + D2D1_MATRIX_3X2_F transform; > + D2D1_POINT_2F point; > + float tolerance; > + float stroke_width; > + BOOL contains; > +}; Unless these are reused between tests, we tend to keep them local to the function. > + static const struct contains_point_test stroke_contains[] = > + { > + {{0.0f, 0.0f, 0.0f, 1.0f}, {0.1f, 10.0f}, 0.0f, 1.0f, FALSE}, > + {{0.0f, 0.0f, 0.0f, 1.0f}, {5.0f, 10.0f}, 5.0f, 1.0f, FALSE}, > + {{0.0f, 0.0f, 0.0f, 1.0f}, {4.9f, 10.0f}, 5.0f, 1.0f, TRUE}, > + {{0.0f, 0.0f, 0.0f, 1.0f}, {5.0f, 10.0f}, -5.0f, 1.0f, FALSE}, > + {{0.0f, 0.0f, 0.0f, 1.0f}, {4.9f, 10.0f}, -5.0f, 1.0f, TRUE}, > + > + {{1.0f, 0.0f, 0.0f, 1.0f}, {0.0f, 10.0f}, 0.0f, 1.0f, TRUE}, > + {{1.0f, 0.0f, 0.0f, 1.0f}, {0.1f, 10.0f}, 0.0f, 1.0f, TRUE}, > + {{1.0f, 0.0f, 0.0f, 1.0f}, {0.5f, 10.0f}, 0.0f, 1.0f, FALSE}, > + {{1.0f, 0.0f, 0.0f, 1.0f}, {0.0f, 10.0f}, 1.0f, 1.0f, TRUE}, > + {{1.0f, 0.0f, 0.0f, 1.0f}, {0.59f, 10.0f}, 1.0f, 1.0f, TRUE}, > + {{1.0f, 0.0f, 0.0f, 1.0f}, {-0.59f, 10.0f}, 1.0f, 1.0f, TRUE}, > + {{1.0f, 0.0f, 0.0f, 1.0f}, {0.59f, 10.0f}, -1.0f, 1.0f, TRUE}, > + {{1.0f, 0.0f, 0.0f, 1.0f}, {-0.59f, 10.0f}, -1.0f, 1.0f, TRUE}, > + }; Rotations and shears are interesting. > @@ -2766,6 +2793,108 @@ static void test_rectangle_geometry(void) > ok(SUCCEEDED(hr), "FillContainsPoint() failed, hr %#x.\n", hr); > ok(!!contains, "Got wrong hit test result %d.\n", contains); > > + /* Stroked area hittesting. Edge. */ > + contains = FALSE; > + set_point(&point, 0.4f, 10.0f); > + hr = ID2D1RectangleGeometry_StrokeContainsPoint(geometry, point, 1.0f, NULL, NULL, 0.0f, &contains); > + ok(SUCCEEDED(hr), "StrokeContainsPoint() failed, hr %#x.\n", hr); > + ok(!!contains, "Got wrong hit test result %d.\n", contains); We tend to use %#x for BOOLs. And well, you might as well make these array based as well.

2 2

Re: [PATCH 1/2] msvcrt: Back up and restore the FPU state in setjmp/longjmp on arm
by Alexandre Julliard 08 Sep '17

08 Sep '17

Martin Storsjo <martin(a)martin.st> writes: > Signed-off-by: Martin Storsjo <martin(a)martin.st> > --- > dlls/msvcrt/except_arm.c | 22 ++++++++++++++++++++-- > 1 file changed, 20 insertions(+), 2 deletions(-) This doesn't build here with the Android NDK: arm-linux-androideabi-gcc -c -o except_arm.o ../../../wine/dlls/msvcrt/except_arm.c -I. -I../../../wine/dlls/msvcrt \ -I../../include -I../../../wine/include -D__WINESRC__ -D_MT -D_MSVCR_VER=0 -D_REENTRANT -fPIC \ -Wall -pipe -fno-strict-aliasing -Wdeclaration-after-statement -Wempty-body -Wignored-qualifiers \ -Wstrict-prototypes -Wtype-limits -Wunused-but-set-parameter -Wvla -Wwrite-strings -Wpointer-arith \ -Wlogical-op -gdwarf-2 -gstrict-dwarf -g -O2 -fno-diagnostics-show-caret -D__ANDROID_API__=26 -marm {standard input}: Assembler messages: {standard input}:44: Error: selected processor does not support ARM mode `vmrs r2,fpscr' {standard input}:46: Error: selected processor does not support ARM mode `vstr d8,[r0,#0x30]' {standard input}:47: Error: selected processor does not support ARM mode `vstr d9,[r0,#0x38]' {standard input}:48: Error: selected processor does not support ARM mode `vstr d10,[r0,#0x40]' {standard input}:49: Error: selected processor does not support ARM mode `vstr d11,[r0,#0x48]' {standard input}:50: Error: selected processor does not support ARM mode `vstr d12,[r0,#0x50]' {standard input}:51: Error: selected processor does not support ARM mode `vstr d13,[r0,#0x58]' {standard input}:52: Error: selected processor does not support ARM mode `vstr d14,[r0,#0x60]' {standard input}:53: Error: selected processor does not support ARM mode `vstr d15,[r0,#0x68]' {standard input}:75: Error: selected processor does not support ARM mode `vmsr fpscr,r3' {standard input}:76: Error: selected processor does not support ARM mode `vldr d8,[r0,#0x30]' {standard input}:77: Error: selected processor does not support ARM mode `vldr d9,[r0,#0x38]' {standard input}:78: Error: selected processor does not support ARM mode `vldr d10,[r0,#0x40]' {standard input}:79: Error: selected processor does not support ARM mode `vldr d11,[r0,#0x48]' {standard input}:80: Error: selected processor does not support ARM mode `vldr d12,[r0,#0x50]' {standard input}:81: Error: selected processor does not support ARM mode `vldr d13,[r0,#0x58]' {standard input}:82: Error: selected processor does not support ARM mode `vldr d14,[r0,#0x60]' {standard input}:83: Error: selected processor does not support ARM mode `vldr d15,[r0,#0x68]' Makefile:311: recipe for target 'except_arm.o' failed -- Alexandre Julliard julliard(a)winehq.org

2 4

Strange behaviour of CreateProcess with Linux binary.
by John Found 08 Sep '17

08 Sep '17

I have a program running under WINE that need to start some external programs using CreateProcess API function. Some of these programs are Linux ELF executables, some are Windows PE. But recently, after some updates of my Linux distribution (Manjaro 64bit) some of the Linux executables stopped to start this way. CreateProcess returns 0, GetLastError returns 0C1h (193) == ERROR_BAD_EXE_FORMAT The same executable runs fine when started from the Linux path. One example file that behaves such way is xfce4-terminal (but the problem is not limited to only this file). For now I am wrapping such executables in short bash script in order to workaround: #!/usr/bin/sh /usr/bin/xfce4-terminal "$@" Any ideas about what is the problem and how can be fixed? -- http://fresh.flatassembler.net http://asm32.info John Found <johnfound(a)asm32.info>

2 2

Re: [PATCH] comctl32/listview: Send NM_RELEASEDCAPTURE after the window focus has changed
by Nikolay Sivov 08 Sep '17

08 Sep '17

What use case does this fix? With this patch notification order is more broken than without. On Windows when clicking on item of unfocused control gives roughly: NM_RELEASEDCAPTURE LVN_ITEMCHANGING/LVN_ITEMCHANGED NM_CUSTOMDRAW NM_SETFOCUS NM_CLICK Without this patch we currently do: NM_RELEASEDCAPTURE LVN_ITEMCHANGIN/LVN_ITEMCHANGED NM_SETFOCUS NM_CLICK With the patch: LVN_ITEMCHANGING/LVN_ITEMCHANGED NM_SETFOCUS NM_RELEASECAPTURE NM_CLICK (in both cases on Wine sequence is interleaved with some NM_CUSTOMDRAW)

2 2

Re: webservices: Make alloc_float_text() static.
by Hans Leidekker 08 Sep '17

08 Sep '17

On Fri, 2017-09-08 at 02:24 +0200, Francois Gouget wrote: > dlls/webservices/reader.c | 2 +- > dlls/webservices/webservices_private.h | 1 - > 2 files changed, 1 insertion(+), 2 deletions(-) > > It was introduced in 82306e84 on 2017-07-20 and has not been used > outside reader.c since then. Ignore this patch if there are ongoing > plans to use it elsewhere. It will be used in the writer at some point.

1 0

Re: msi: Make MSI_ProcessMessageVerbatim() static.
by Hans Leidekker 08 Sep '17

08 Sep '17

On Fri, 2017-09-08 at 02:12 +0200, Francois Gouget wrote: > diff --git a/dlls/msi/msipriv.h b/dlls/msi/msipriv.h > index f316c0cce74..1bb0a849eed 100644 > --- a/dlls/msi/msipriv.h > +++ b/dlls/msi/msipriv.h > @@ -872,7 +872,6 @@ extern UINT MSI_SetInstallLevel( MSIPACKAGE *package, int iInstallLevel ) DECLSP > extern MSIPACKAGE *MSI_CreatePackage( MSIDATABASE *, LPCWSTR ) DECLSPEC_HIDDEN; > extern UINT MSI_OpenPackageW( LPCWSTR szPackage, MSIPACKAGE **pPackage ) DECLSPEC_HIDDEN; > extern UINT MSI_SetTargetPathW( MSIPACKAGE *, LPCWSTR, LPCWSTR ) DECLSPEC_HIDDEN; > -extern INT MSI_ProcessMessageVerbatim( MSIPACKAGE *, INSTALLMESSAGE, MSIRECORD * ) DECLSPEC_HIDDEN; > extern INT MSI_ProcessMessage( MSIPACKAGE *, INSTALLMESSAGE, MSIRECORD * ) DECLSPEC_HIDDEN; > extern MSICONDITION MSI_EvaluateConditionW( MSIPACKAGE *, LPCWSTR ) DECLSPEC_HIDDEN; > extern UINT MSI_GetComponentStateW( MSIPACKAGE *, LPCWSTR, INSTALLSTATE *, INSTALLSTATE * ) DECLSPEC_HIDDEN; > diff --git a/dlls/msi/package.c b/dlls/msi/package.c > index 47164613209..a88609a3dd8 100644 > --- a/dlls/msi/package.c > +++ b/dlls/msi/package.c > @@ -1444,6 +1444,8 @@ UINT msi_set_original_database_property( MSIDATABASE *db, const WCHAR *package ) > return r; > } > > +static INT MSI_ProcessMessageVerbatim(MSIPACKAGE*,INSTALLMESSAGE,MSIRECORD*); I'd prefer moving this function (and internal_ui_handler) up.

1 0

Re: [PATCH 1/3] d3dx9/tests: Add test for null shaders in effect.
by Matteo Bruni 07 Sep '17

07 Sep '17

2017-09-01 20:19 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > +static void test_effect_null_shader(void) > +{ > + D3DPRESENT_PARAMETERS present_parameters = {0}; > + IDirect3DDevice9 *device; > + ID3DXEffect *effect; > + D3DXPASS_DESC desc; > + IDirect3D9 *d3d; > + D3DXHANDLE pass; > + ULONG refcount; > + HWND window; > + HRESULT hr; > + > + /* Creating a fresh device because the existing device can have invalid render states > + from previous tests. If IDirect3DDevice9_ValidateDevice() returns certain error codes, > + native ValidateTechnique() fails. */ We generally want to be sure that each test is independent from the others so creating a separate device for each test function is a good idea regardless. Your point about ValidateDevice() is very interesting though, actually it makes a lot of sense and it probably means that ValidateTechnique() should delegate a good chunk of its work to ValidateDevice() (not surprising but somehow I forgot about it). It would be interesting to extend the d3d9 ValidateDevice() tests at some point.

1 0

Re: [1/2] ntdll: Implement set_cpu_context for ARM64
by Alexandre Julliard 07 Sep '17

07 Sep '17

André Hentschel <nerv(a)dawncrow.de> writes: > + "ldr x30, [x17, #0xf8]\n\t" > + "ldr x16, [x17, #0x100]\n\t" > + "mov sp, x16\n\t" > + "ldr x16, [x17, #0x108]\n\t" > + "br x16\n\t" You'd also need to restore x16 and x17, this is used in signal handlers. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [v5 PATCH] wmvcore/tests: add tests for WMReader methods
by Alexandre Julliard 06 Sep '17

06 Sep '17

Jefferson Carpenter <jeffersoncarpenter2(a)gmail.com> writes: > Can somebody explain why this one wasn't committed? I'm not convinced we need a large binary file for this, and it also seems to take a long time to run mainly because it's just a bunch of timeouts at this point. I'd like to see the implementation being fleshed out a bit first. -- Alexandre Julliard julliard(a)winehq.org

2 3

Re: [v2 PATCH] d2d1: Partially implement StrokeContainsPoint() for rectangles
by Henri Verbeet 06 Sep '17

06 Sep '17

On 6 September 2017 at 12:35, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > -static void d2d_point_normalise(D2D1_POINT_2F *p) > +static float d2d_point_normalise(D2D1_POINT_2F *p) > { > float l; > > if ((l = sqrtf(d2d_point_dot(p, p))) != 0.0f) > d2d_point_scale(p, 1.0f / l); > + > + return l; > } Maybe. I'm not entirely convinced you need this, see below. > +static BOOL d2d_rect_point_on(const D2D1_POINT_2F *point, float tolerance, const D2D1_POINT_2F *points) > +{ > + unsigned int i; > + float dot; > + > + for (i = 0; i < 4; i++) > + { It may improve readability to pull that loop out into the caller, and make this function just about testing whether a point is on a line segment. > + float edge_length, distance; > + D2D1_POINT_2F v, edge; > + > + v.x = point->x - points[i].x; > + v.y = point->y - points[i].y; d2d_point_subtract(&v, point, &points[i]); Or, alternatively, d2d_point_subtract(&v_q, q, p0); > + /* Point to vertex distance. */ > + if (d2d_point_dot(&v, &v) < tolerance * tolerance) > + return TRUE; Maybe. An argument could probably be made for testing joins separately. > + /* Point to edge distance. */ > + edge.x = points[(i + 1) % 4].x - points[i].x; > + edge.y = points[(i + 1) % 4].y - points[i].y; d2d_point_subtract(&edge, &points[(i + 1) % 4], &points[i]); Or, d2d_point_subtract(&v_p, p1, p0); > + if ((edge_length = sqrtf(d2d_point_dot(&edge, &edge))) == 0.0f) > + continue; I guess we'd want to introduce d2d_point_length(). > + distance = fabsf(edge.x * v.y - edge.y * v.x) / edge_length; n.x = -v_p.y; n.y = v_p.x; distance = fabsf(d2d_point_dot(&v_q, &n)) / edge_length; > + d2d_point_normalise(&edge); > + dot = d2d_point_dot(&edge, &v); > + if (dot >= 0.0f && dot <= edge_length) > + return TRUE; If you compare against "edge_length * edge_length", you can drop the normalisation. > +static BOOL d2d_rect_point_inside(const D2D1_POINT_2F *point, const D2D1_POINT_2F *points) > +{ > + D2D1_POINT_2F vec[2]; > + unsigned int i; > + float dot; > + > + for (i = 0; i < 2; i++) > + { > + float l; > + > + vec[0].x = points[i + 1].x - points[i].x; > + vec[0].y = points[i + 1].y - points[i].y; > + vec[1].x = point->x - points[i].x; > + vec[1].y = point->y - points[i].y; > + > + if ((l = d2d_point_normalise(&vec[0])) == 0.0f) > + return FALSE; > + > + if ((dot = d2d_point_dot(&vec[0], &vec[1])) < 0.0f || dot > l) > + return FALSE; > + } > + > + return TRUE; > +} Some of the comments on d2d_rect_point_on() apply here as well. Perhaps more importantly, does this work with shear transformations? > + if (transform) > + { > + D2D1_POINT_2F outer[4], inner[4]; > + > + stroke_width *= 0.5f; > + > + d2d_point_transform(&inner[0], transform, rect->left + stroke_width, rect->top + stroke_width); > + d2d_point_transform(&inner[1], transform, rect->left + stroke_width, rect->bottom - stroke_width); > + d2d_point_transform(&inner[2], transform, rect->right - stroke_width, rect->bottom - stroke_width); > + d2d_point_transform(&inner[3], transform, rect->right - stroke_width, rect->top + stroke_width); > + > + d2d_point_transform(&outer[0], transform, rect->left - stroke_width, rect->top - stroke_width); > + d2d_point_transform(&outer[1], transform, rect->left - stroke_width, rect->bottom + stroke_width); > + d2d_point_transform(&outer[2], transform, rect->right + stroke_width, rect->bottom + stroke_width); > + d2d_point_transform(&outer[3], transform, rect->right + stroke_width, rect->top - stroke_width); This seems suspicious. In general, the stroke width isn't affected by transformations on the geometry. Test coverage for that would be nice in either case.

1 0

Re: [PATCH 2/3] include: Add interfaces IMFMediaTypeHandler, IMFStreamDescriptor
by Henri Verbeet 06 Sep '17

06 Sep '17

On 6 September 2017 at 09:38, Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> wrote: > +[ > + object, > + uuid(e93dcf6c-4b07-4e1e-8123-aa16ed6eadf5), > +] > +interface IMFMediaTypeHandler : IUnknown > +{ ... > +}; Redundant ';'. Unfortunately we have that issue in a bunch of other IDLs as well, and the Microsoft IDLs don't necessarily always get it right either.

1 0

Re: [PATCH 3/5] wined3d: Store FLUSH flag in the query object.
by Henri Verbeet 06 Sep '17

06 Sep '17

On 6 September 2017 at 01:39, Matteo Bruni <mbruni(a)codeweavers.com> wrote: > So it can be used in the CS thread's query polling. Passing the > appropriate flag to glClientWaitSync() is potentially lighter on > performance than a separate, explicit glFlush(). Maybe. It would be debatable once there are multiple queries in-flight. > @@ -389,8 +389,8 @@ HRESULT CDECL wined3d_query_get_data(struct wined3d_query *query, > } > else if (query->counter_main != query->counter_retrieved) > { > - if (flags & WINED3DGETDATA_FLUSH) > - wined3d_cs_emit_flush(query->device->cs); > + if (flags & WINED3DGETDATA_FLUSH && !(query->flags & WINED3DGETDATA_FLUSH)) > + InterlockedExchange((LONG *)&query->flags, flags); Shouldn't the flag be cleared again when the query is restarted? How about the following: - wined3d_query_get_data() emits a flush when the counters don't match, "flags" contains WINED3DGETDATA_FLUSH, and query->flushed is FALSE. - wined3d_cs_exec_flush() sets query->flushed for all queries on cs->query_poll_list. - query->flushed is cleared when a query is removed from cs->query_poll_list. Would that work? It would leave a small window between wined3d_cs_emit_flush() and wined3d_cs_exec_flush() that could probably be avoided by checking in wined3d_query_get_data() if any queries were issued since the last flush. Not sure whether that's worth it. Alternatively, we could only do the latter. I.e., set cs->queries_flushed in wined3d_cs_emit_flush(), and clear it in wined3d_cs_emit_query_issue(). That wouldn't prevent all redundant flushes, but it would help for applications calling wined3d_query_get_data() in a tight loop.

1 0

Re: [PATCH 1/5] wined3d: Restore support for d3d9 "fake" formats.
by Henri Verbeet 06 Sep '17

06 Sep '17

On 6 September 2017 at 01:39, Matteo Bruni <mbruni(a)codeweavers.com> wrote: > @@ -4114,6 +4114,7 @@ extern enum wined3d_format_id pixelformat_for_depth(DWORD depth) DECLSPEC_HIDDEN > #define WINED3DFMT_FLAG_DEPTH 0x00000004 > #define WINED3DFMT_FLAG_STENCIL 0x00000008 > #define WINED3DFMT_FLAG_RENDERTARGET 0x00000010 > +#define WINED3DFMT_FLAG_FAKE 0x00000020 Could I suggest WINED3DFMT_FLAG_EXTENSION?

1 0

Re: [PATCH 01/10] atl80: Fix processorArchitecture in manifest
by Alexandre Julliard 06 Sep '17

06 Sep '17

Fabian Maurer <dark.shadow4(a)web.de> writes: > processorArchitecture should not be empty string I don't think that's correct for assemblies that can be imported. Note that the processorArchitecture is fixed up when creating the fake dll, so it doesn't end up empty in the resources. -- Alexandre Julliard julliard(a)winehq.org

2 5

Re: [PATCH] wined3d: wined3d_check_surface_capability return TRUE for surfaces with WINED3DFMT_FLAG_TEXTURE.
by Henri Verbeet 05 Sep '17

05 Sep '17

On 4 September 2017 at 18:26, lorenzo Skotos <lorenzofersteam(a)live.it> wrote: > - if (format->glInternal) > + if (format->glInternal || format->flags[WINED3D_GL_RES_TYPE_TEX_2D] & WINED3DFMT_FLAG_TEXTURE) That should fix the regression, but since the ambiguity of WINED3DFMT_FLAG_TEXTURE led to the issue in the first place, I'd rather not simply restore the check. Instead, I think it would be better to explicitly mark these special "extension" formats as such. In theory Matteo has a patch for that.

2 3

Re: [PATCH] ntdll: Make RtlDeregisterWaitEx(handle, INVALID_HANDLE_VALUE) thread safe.
by Stefan Dösinger 05 Sep '17

05 Sep '17

Hi, Is the test I was referring to in the description of the previous patch. It causes seemingly unrelated test failures like https://testbot.winehq.org/JobDetails.pl?Key=32956 (previous iteration of the test that used a different implementation) Stefan Am 2017-09-05 um 17:34 schrieb Stefan Dösinger: > Chromium signals the wait semaphore and calls DeregisterWaitEx with > CompletionHandle = INVALID_HANDLE_VALUE in close succession. Sometimes > the worker thread decides to run the callback, but before it sets > CallbackInProgress RtlDeregisterWaitEx decides that the callback is not > running and returns STATUS_SUCCESS. Chromium then releases resources > that the callback needs to run, resulting in random crashes. > > Signed-off-by: Stefan Dösinger <stefan(a)codeweavers.com> > > --- > > The tests show that we're only supposed to return STATUS_PENDING if the > callback is running. Note that the choice between waiting and returning > STATUS_SUCCESS and not waiting and returning STATUS_PENDING is still > racy, but we'll no longer return STATUS_SUCCESS and run the callback > afterwards. > > I tried to extend the tests to show what happens when both events are > available simultanously. The answer is that it is fairly random. > INVALID_HANDLE_VALUE blocks and runs the callback one last time, the > other ways of calling RtlDeregisterWait[Ex] randomly return STATUS_SUCCESS > and do not run the callback, or they return STATUS_PENDING and may or > may not run the callback. However, in no circumstance does the callback > run after STATUS_SUCCESS is returned. > > I am not including the tests because of the random outcome and because > it plays with SuspendThread and makes the random failures in follow up > tests worse. I'll send it to wine-devel for reference. > --- > dlls/ntdll/threadpool.c | 59 +++++++++++++++++++++++++++---------------------- > 1 file changed, 32 insertions(+), 27 deletions(-) > > diff --git a/dlls/ntdll/threadpool.c b/dlls/ntdll/threadpool.c > index 6063d51d9f..1b0546a781 100644 > --- a/dlls/ntdll/threadpool.c > +++ b/dlls/ntdll/threadpool.c > @@ -541,11 +541,13 @@ static DWORD CALLBACK wait_thread_proc(LPVOID Arg) > break; > } > > - completion_event = wait_work_item->CompletionEvent; > - if (completion_event) NtSetEvent( completion_event, NULL ); > > if (interlocked_inc( &wait_work_item->DeleteCount ) == 2 ) > + { > + completion_event = wait_work_item->CompletionEvent; > delete_wait_work_item( wait_work_item ); > + if (completion_event) NtSetEvent( completion_event, NULL ); > + } > > return 0; > } > @@ -633,44 +635,47 @@ NTSTATUS WINAPI RtlRegisterWait(PHANDLE NewWaitObject, HANDLE Object, > NTSTATUS WINAPI RtlDeregisterWaitEx(HANDLE WaitHandle, HANDLE CompletionEvent) > { > struct wait_work_item *wait_work_item = WaitHandle; > - NTSTATUS status = STATUS_SUCCESS; > + NTSTATUS status; > + HANDLE LocalEvent = NULL; > + BOOLEAN CallbackInProgress; > > - TRACE( "(%p)\n", WaitHandle ); > + TRACE( "(%p %p)\n", WaitHandle, CompletionEvent ); > > if (WaitHandle == NULL) > return STATUS_INVALID_HANDLE; > > - NtSetEvent( wait_work_item->CancelEvent, NULL ); > - if (wait_work_item->CallbackInProgress) > + CallbackInProgress = wait_work_item->CallbackInProgress; > + if (CompletionEvent == INVALID_HANDLE_VALUE || !CallbackInProgress) > { > - if (CompletionEvent != NULL) > - { > - if (CompletionEvent == INVALID_HANDLE_VALUE) > - { > - status = NtCreateEvent( &CompletionEvent, EVENT_ALL_ACCESS, NULL, NotificationEvent, FALSE ); > - if (status != STATUS_SUCCESS) > - return status; > - interlocked_xchg_ptr( &wait_work_item->CompletionEvent, CompletionEvent ); > - if (wait_work_item->CallbackInProgress) > - NtWaitForSingleObject( CompletionEvent, FALSE, NULL ); > - NtClose( CompletionEvent ); > - } > - else > - { > - interlocked_xchg_ptr( &wait_work_item->CompletionEvent, CompletionEvent ); > - if (wait_work_item->CallbackInProgress) > - status = STATUS_PENDING; > - } > - } > - else > - status = STATUS_PENDING; > + status = NtCreateEvent( &LocalEvent, EVENT_ALL_ACCESS, NULL, NotificationEvent, FALSE ); > + if (status != STATUS_SUCCESS) > + return status; > + interlocked_xchg_ptr( &wait_work_item->CompletionEvent, LocalEvent ); > + } > + else if (CompletionEvent != NULL) > + { > + interlocked_xchg_ptr( &wait_work_item->CompletionEvent, CompletionEvent ); > } > > + NtSetEvent( wait_work_item->CancelEvent, NULL ); > + > if (interlocked_inc( &wait_work_item->DeleteCount ) == 2 ) > { > status = STATUS_SUCCESS; > delete_wait_work_item( wait_work_item ); > } > + else if (LocalEvent) > + { > + NtWaitForSingleObject( LocalEvent, FALSE, NULL ); > + status = STATUS_SUCCESS; > + } > + else > + { > + status = STATUS_PENDING; > + } > + > + if (LocalEvent) > + NtClose( LocalEvent ); > > return status; > } >

1 0

Re: [PATCH] wbemprox: Fix CreateInstanceEnum/Async entries.
by Hans Leidekker 05 Sep '17

05 Sep '17

On Tue, 2017-09-05 at 15:19 +0300, Andrey Gusev wrote: > @@ -599,7 +599,7 @@ static HRESULT WINAPI wbem_services_CreateInstanceEnum( > > static HRESULT WINAPI wbem_services_CreateInstanceEnumAsync( > IWbemServices *iface, > - const BSTR strFilter, > + const BSTR strClass, > LONG lFlags, > IWbemContext *pCtx, > IWbemObjectSink *pResponseHandler ) I think the original name is fine.

1 0

Re: [2/2] ntdll: Implement RtlUnwind for ARM64
by Stefan Dösinger 05 Sep '17

05 Sep '17

Am 2017-09-04 um 20:46 schrieb André Hentschel: > + frame = NtCurrentTeb()->Tib.ExceptionList; > + while ((frame != (EXCEPTION_REGISTRATION_RECORD*)~0UL) && (frame != endframe)) Unless I am blind this handles only x86-style stack handlers, and not the function tables similar to amd64 that are likely to be present as well on arm64. Though I guess because of the lack of arm64 compilers, documentation and binaries from Microsoft we don't really know how Windows (will) handle arm64 exceptions.

1 0

Testing reading and writing of WMF files
by Jefferson Carpenter 04 Sep '17

04 Sep '17

I'm trying to imagine what would be a good way to test for the successful processing (reading or writing) of a Windows Media Format file. The WMReader interface has to parse the file correctly. I believe it would be impossible to test for this, without having a natural WMF (.wma, .wmv, or .asf) file to read. There are two ways of getting one: include one in the test suite, or create one as part of the test. The first option has certain disadvantages, such as A) the file size and B) inability to tweak the contents, or easily make a variety of similar files, should the need arise. Also, the WMWriter has to write the file correctly. I see only two ways of testing this: compare the output byte-for-byte with a file generated on a Windows PC, or attempt to read the generated file with a known good wmf implementation. The first way would assume the output is always the same byte-for-byte, which it may not be. For the second, we would need a conforming implementation to use. It seems to me, libav would be quite useful, as it can already decode and encode many if not most of the WMF formats. Do you think it's a valid assumption to make, that libav's codecs are a conforming implementation of the WMF formats? Jefferson Carpenter

1 0

Re: Re: [v5 PATCH] wmvcore/tests: add tests for WMReader methods
by Marvin 04 Sep '17

04 Sep '17

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=32942 Your paranoid android. === build (build) === Patch failed to apply

1 0

Re: PATCH] user32: Allow activating desktop only when clicked directly
by Fabian Maurer 02 Sep '17

02 Sep '17

Hello, just noticed this patch being marked as 'Superseded', but I don't see any new patch addressing this issue. Am I missing something here? Regards, Fabian Maurer

2 2

comctl v6
by Fabian Maurer 02 Sep '17

02 Sep '17

Hello wine-devel, While investigating Bug 43643, I learned that comboboxes behave differently when comctl32 v6 is requested by the program. Basically, when v6 is used, all comboboxes display 30 items maximum, regardless of height. This behavior is unimplemented by wine, so affected programs doesn't show any drop down list at all. This would be the correct behavior when not using v6, but it's wrong in this case. We probably need a second combobox class with different behavior (or one class for both behaviors). I don't know too much on how windows implements comboboxes, in wine the comboboxes are implemented in user32, but we probably need to split them between comctl32 v6 and lower. Do you already have plans on how to do that? Especially, how would we do the separation on the source level but still reuse as much code as possible? Is there something that could be done as a start to bring this into the right direction? Regards, Fabian Maurer

1 0

Re: [PATCH] winecfg: Remove processorArchitecture from manifest
by Fabian Maurer 02 Sep '17

02 Sep '17

It works with that patch, but maybe we want > processorArchitecture="*" instead? Regards, Fabian Maurer

1 0

Re: [v3 3/3] d3dx9: Return E_FAIL from ValidateTechnique() for techniques with unsupported shaders.
by Marvin 01 Sep '17

01 Sep '17

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=32937 Your paranoid android. === build (build) === Patch failed to apply

2 1

Re: [v3 3/3] d3dx9: Return E_FAIL from ValidateTechnique() for techniques with unsupported shaders.
by Paul Gofman 01 Sep '17

01 Sep '17

Please ignore this patch for now (along with the previous version). I have discovered that there is still the case when object_id can be nonzero, but the object is never created (not initialized array of shaders). So we should keep the creation_failed flag probably. I will send the test for null shaders and the new patch version in the next series. On 09/01/2017 01:56 PM, Paul Gofman wrote: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > v3: > - use object_id != 0 as the indication of non-null shader. > --- > dlls/d3dx9_36/effect.c | 45 +++++++++++++++++++++++++++++++++++++++++--- > dlls/d3dx9_36/tests/effect.c | 3 --- > 2 files changed, 42 insertions(+), 6 deletions(-) > > diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c > index 2cd0c22f64..ef772d99b5 100644 > --- a/dlls/d3dx9_36/effect.c > +++ b/dlls/d3dx9_36/effect.c > @@ -3941,11 +3941,50 @@ static D3DXHANDLE WINAPI ID3DXEffectImpl_GetCurrentTechnique(ID3DXEffect *iface) > > static HRESULT WINAPI ID3DXEffectImpl_ValidateTechnique(ID3DXEffect* iface, D3DXHANDLE technique) > { > - struct ID3DXEffectImpl *This = impl_from_ID3DXEffect(iface); > + struct ID3DXEffectImpl *effect = impl_from_ID3DXEffect(iface); > + struct d3dx9_base_effect *base = &effect->base_effect; > + struct d3dx_technique *tech = get_valid_technique(base, technique); > + HRESULT ret = D3D_OK; > + unsigned int i, j; > > - FIXME("(%p)->(%p): stub\n", This, technique); > + FIXME("iface %p, technique %p semi-stub.\n", iface, technique); > > - return D3D_OK; > + if (!tech) > + { > + ret = D3DERR_INVALIDCALL; > + goto done; > + } > + for (i = 0; i < tech->pass_count; ++i) > + { > + struct d3dx_pass *pass = &tech->passes[i]; > + > + for (j = 0; j < pass->state_count; ++j) > + { > + struct d3dx_state *state = &pass->states[j]; > + > + if (state_table[state->operation].class == SC_VERTEXSHADER > + || state_table[state->operation].class == SC_PIXELSHADER) > + { > + struct d3dx_parameter *param; > + void *param_value; > + BOOL param_dirty; > + HRESULT hr; > + > + if (FAILED(hr = d3dx9_get_param_value_ptr(pass, &pass->states[j], &param_value, &param, > + FALSE, &param_dirty))) > + return hr; > + > + if (param->object_id && !*(void **)param->data) > + { > + ret = E_FAIL; > + goto done; > + } > + } > + } > + } > +done: > + TRACE("Returning %#x.\n", ret); > + return ret; > } > > static HRESULT WINAPI ID3DXEffectImpl_FindNextValidTechnique(ID3DXEffect* iface, D3DXHANDLE technique, D3DXHANDLE* next_technique) > diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c > index bb26fe4e51..7260411c39 100644 > --- a/dlls/d3dx9_36/tests/effect.c > +++ b/dlls/d3dx9_36/tests/effect.c > @@ -7090,10 +7090,8 @@ static void test_effect_unsupported_shader(void) > ok(hr == D3D_OK, "Got result %#x.\n", hr); > > hr = effect->lpVtbl->ValidateTechnique(effect, "missing_technique"); > - todo_wine > ok(hr == D3DERR_INVALIDCALL, "Got result %#x.\n", hr); > hr = effect->lpVtbl->ValidateTechnique(effect, "tech0"); > - todo_wine > ok(hr == E_FAIL, "Got result %#x.\n", hr); > > hr = effect->lpVtbl->ValidateTechnique(effect, "tech1"); > @@ -7101,7 +7099,6 @@ static void test_effect_unsupported_shader(void) > effect->lpVtbl->SetInt(effect, "i", 1); > ok(hr == D3D_OK, "Got result %#x.\n", hr); > hr = effect->lpVtbl->ValidateTechnique(effect, "tech1"); > - todo_wine > ok(hr == E_FAIL, "Got result %#x.\n", hr); > effect->lpVtbl->SetInt(effect, "i", 0); > hr = effect->lpVtbl->ValidateTechnique(effect, "tech1");

1 0

Re: [v2 3/3] d3dx9: Return E_FAIL from ValidateTechnique() for techniques with unsupported shaders.
by Matteo Bruni 01 Sep '17

01 Sep '17

2017-08-31 20:56 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > + if (!param->object_id || base->objects[param->object_id].creation_failed) It should be possible to replace this with a check for !*(void **)param->data. Unless you mean to use the flag for something else later on?

2 1

Re: [PATCH] d2d1: Partially implement StrokeContainsPoint() for rectangle geometry
by Henri Verbeet 01 Sep '17

01 Sep '17

On 31 August 2017 at 14:00, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > - if (transform) > - { > - if (!d2d_matrix_invert(&g_i, transform)) > - return D2DERR_UNSUPPORTED_OPERATION; > - d2d_point_transform(&point, &g_i, point.x, point.y); > - } > + if (FAILED(hr = geometry_inv_transform_point(transform, &point))) > + return hr; That's fine in principle, but please do it in a separate commit, and please follow the existing conventions. E.g.: if (transform && FAILED(hr = d2d_point_transform_inverse(&point, transform, point.x, point.y))) return hr; > + stroke_width /= 2.0f; > + > + rect = geometry->u.rectangle.rect; > + rect.left -= stroke_width; > + rect.right += stroke_width; > + rect.top -= stroke_width; > + rect.bottom += stroke_width; > + > + dx = max(fabsf((rect.right + rect.left) / 2.0f - point.x) - (rect.right - rect.left) / 2.0f, 0.0f); > + dy = max(fabsf((rect.bottom + rect.top) / 2.0f - point.y) - (rect.bottom - rect.top) / 2.0f, 0.0f); > + > + if ((*contains = tolerance * tolerance > (dx * dx + dy * dy))) > + { > + rect = geometry->u.rectangle.rect; > + stroke_width += tolerance; > + rect.left += stroke_width; > + rect.right -= stroke_width; > + rect.top += stroke_width; > + rect.bottom -= stroke_width; > + > + *contains = rect.left >= point.x || rect.right <= point.x || rect.top >= point.y || rect.bottom <= point.y; > + } I think that looks a little more complicated than it needs to be. How about: D2D1_POINT_2F d, s; ... s.x = rect.right - rect.left; s.y = rect.bottom - rect.top; d.x = fabsf((rect.right + rect.left) * 0.5f - point.x); d.y = fabsf((rect.bottom + rect.top) * 0.5f - point.y); if (d.x < (s.x - stroke_width) * 0.5f - tolerance && d.y < (s.y - stroke_width) * 0.5f - tolerance) { *contains = FALSE; return S_OK; } d.x = max(d.x - (s.x + stroke_width) * 0.5f, 0.0f); d.y = max(d.y - (s.y + stroke_width) * 0.5f, 0.0f); *contains = (d.x * d.x + d.y * d.y) < tolerance * tolerance; I notice you don't have any tests with transformations, is that intentional? I think you'd need to transform the stroke width with the transformation matrix.

2 1

Re: [PATCH 3/3] d3dx9: Return E_FAIL from ValidateTechnique() for techniques with unsupported shaders.
by Matteo Bruni 31 Aug '17

31 Aug '17

2017-08-31 19:40 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > static HRESULT WINAPI ID3DXEffectImpl_ValidateTechnique(ID3DXEffect* iface, D3DXHANDLE technique) > { > - struct ID3DXEffectImpl *This = impl_from_ID3DXEffect(iface); > + struct ID3DXEffectImpl *effect = impl_from_ID3DXEffect(iface); > + struct d3dx9_base_effect *base = &effect->base_effect; > + struct d3dx_technique *tech = get_valid_technique(base, technique); > + HRESULT ret = D3D_OK; > + unsigned int i, j; > > - FIXME("(%p)->(%p): stub\n", This, technique); > + FIXME("iface %p, technique %p semi-stub.\n", iface, technique); Do you know what else is missing for ValidateTechnique()? Replying to myself: the return values mentioned in https://msdn.microsoft.com/en-us/library/windows/desktop/bb205835(v=vs.85).… give some ideas (although E_FAIL is not in the list, figures...)

2 1

Re: [PATCH 1/3] d3dx9/tests: Add test for effect with unsupported shader.
by Matteo Bruni 31 Aug '17

31 Aug '17

2017-08-31 19:40 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > dlls/d3dx9_36/tests/effect.c | 185 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 185 insertions(+) > > diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c > index 9658d2dc82..3e8df11d24 100644 > --- a/dlls/d3dx9_36/tests/effect.c > +++ b/dlls/d3dx9_36/tests/effect.c > @@ -6981,6 +6981,190 @@ static void test_effect_skip_constants(IDirect3DDevice9 *device) > effect->lpVtbl->Release(effect); > } > > +#if 0 > +vertexshader vs_arr[2] = > +{ > + asm > + { > + vs_3_0 > + def c0, 1, 1, 1, 1 > + dcl_position o0 > + mov o0, c0 > + }, > + > + asm > + { > + vs_3_sw > + def c256, 1, 1, 1, 1 > + dcl_position o0 > + mov o0, c256 > + }, > +}; > + > +int i; > + > +technique tech0 > +{ > + pass p0 > + { > + VertexShader = vs_arr[1]; > + } > +} > +technique tech1 > +{ > + pass p0 > + { > + VertexShader = vs_arr[i]; > + } > +} > +#endif > +static const DWORD test_effect_unsupported_shader_blob[] = > +{ > + 0xfeff0901, 0x000000ac, 0x00000000, 0x00000010, 0x00000004, 0x00000020, 0x00000000, 0x00000002, > + 0x00000001, 0x00000002, 0x00000007, 0x615f7376, 0x00007272, 0x00000002, 0x00000000, 0x0000004c, > + 0x00000000, 0x00000000, 0x00000001, 0x00000001, 0x00000000, 0x00000002, 0x00000069, 0x00000003, > + 0x00000010, 0x00000004, 0x00000000, 0x00000000, 0x00000000, 0x00000003, 0x00003070, 0x00000006, > + 0x68636574, 0x00000030, 0x00000004, 0x00000010, 0x00000004, 0x00000000, 0x00000000, 0x00000000, > + 0x00000003, 0x00003070, 0x00000006, 0x68636574, 0x00000031, 0x00000002, 0x00000002, 0x00000006, > + 0x00000005, 0x00000004, 0x00000018, 0x00000000, 0x00000000, 0x0000002c, 0x00000048, 0x00000000, > + 0x00000000, 0x00000074, 0x00000000, 0x00000001, 0x0000006c, 0x00000000, 0x00000001, 0x00000092, > + 0x00000000, 0x00000058, 0x00000054, 0x000000a0, 0x00000000, 0x00000001, 0x00000098, 0x00000000, > + 0x00000001, 0x00000092, 0x00000000, 0x00000084, 0x00000080, 0x00000002, 0x00000002, 0x00000001, > + 0x00000038, 0xfffe0300, 0x05000051, 0xa00f0000, 0x3f800000, 0x3f800000, 0x3f800000, 0x3f800000, > + 0x0200001f, 0x80000000, 0xe00f0000, 0x02000001, 0xe00f0000, 0xa0e40000, 0x0000ffff, 0x00000002, > + 0x00000038, 0xfffe0300, 0x05000051, 0xa00f0100, 0x3f800000, 0x3f800000, 0x3f800000, 0x3f800000, > + 0x0200001f, 0x80000000, 0xe00f0000, 0x02000001, 0xe00f0000, 0xa0e40100, 0x0000ffff, 0x00000001, > + 0x00000000, 0xffffffff, 0x00000000, 0x00000002, 0x000000e4, 0x00000008, 0x615f7376, 0x00007272, > + 0x46580200, 0x0023fffe, 0x42415443, 0x0000001c, 0x00000057, 0x46580200, 0x00000001, 0x0000001c, > + 0x00000100, 0x00000054, 0x00000030, 0x00000002, 0x00000001, 0x00000034, 0x00000044, 0xabab0069, > + 0x00020000, 0x00010001, 0x00000001, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, > + 0x4d007874, 0x6f726369, 0x74666f73, 0x29522820, 0x534c4820, 0x6853204c, 0x72656461, 0x6d6f4320, > + 0x656c6970, 0x2e392072, 0x392e3932, 0x332e3235, 0x00313131, 0x0002fffe, 0x54494c43, 0x00000000, > + 0x000cfffe, 0x434c5846, 0x00000001, 0x10000001, 0x00000001, 0x00000000, 0x00000002, 0x00000000, > + 0x00000000, 0x00000004, 0x00000000, 0xf0f0f0f0, 0x0f0f0f0f, 0x0000ffff, 0x00000000, 0x00000000, > + 0xffffffff, 0x00000000, 0x00000001, 0x0000000a, 0x615f7376, 0x315b7272, 0x0000005d, > +}; > + > +#define TEST_EFFECT_UNSUPPORTED_SHADER_VSHADER_POS 81 > +#define TEST_EFFECT_UNSUPPORTED_SHADER_VSHADER_LEN 14 > + > +static void test_effect_unsupported_shader(void) > +{ > + D3DPRESENT_PARAMETERS present_parameters = {0}; > + IDirect3DVertexShader9 *vshader; > + unsigned int passes_count; > + IDirect3DDevice9 *device; > + UINT byte_code_size; > + ID3DXEffect *effect; > + IDirect3D9 *d3d; > + void *byte_code; > + ULONG refcount; > + HWND window; > + HRESULT hr; > + > + if (!(window = CreateWindowA("static", "d3dx9_test", WS_OVERLAPPEDWINDOW, 0, 0, > + 640, 480, NULL, NULL, NULL, NULL))) > + { > + skip("Couldn't create application window\n"); > + return; > + } > + if (!(d3d = Direct3DCreate9(D3D_SDK_VERSION))) > + { > + skip("Couldn't create IDirect3D9 object\n"); > + DestroyWindow(window); > + return; > + } > + present_parameters.Windowed = TRUE; > + present_parameters.SwapEffect = D3DSWAPEFFECT_DISCARD; > + hr = IDirect3D9_CreateDevice(d3d, D3DADAPTER_DEFAULT, D3DDEVTYPE_HAL, window, > + D3DCREATE_HARDWARE_VERTEXPROCESSING, &present_parameters, &device); > + if (FAILED(hr)) { > + skip("Failed to create IDirect3DDevice9 object, hr %#x\n", hr); > + IDirect3D9_Release(d3d); > + DestroyWindow(window); > + return; > + } > + > + hr = D3DXCreateEffectEx(device, test_effect_unsupported_shader_blob, sizeof(test_effect_unsupported_shader_blob), > + NULL, NULL, NULL, 0, NULL, &effect, NULL); > + todo_wine > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + if (FAILED(hr)) > + { > + skip("Failed to create effect, skipping test.\n"); > + goto cleanup; > + } > + > + hr = effect->lpVtbl->ValidateTechnique(effect, "missing_technique"); > + ok(hr == D3DERR_INVALIDCALL, "Got result %#x.\n", hr); > + hr = effect->lpVtbl->ValidateTechnique(effect, "tech0"); > + ok(hr == E_FAIL, "Got result %#x.\n", hr); > + > + hr = effect->lpVtbl->ValidateTechnique(effect, "tech1"); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + effect->lpVtbl->SetInt(effect, "i", 1); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + hr = effect->lpVtbl->ValidateTechnique(effect, "tech1"); > + ok(hr == E_FAIL, "Got result %#x.\n", hr); > + effect->lpVtbl->SetInt(effect, "i", 0); > + hr = effect->lpVtbl->ValidateTechnique(effect, "tech1"); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + > + hr = effect->lpVtbl->SetTechnique(effect, "tech0"); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + hr = effect->lpVtbl->Begin(effect, &passes_count, 0); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + hr = effect->lpVtbl->BeginPass(effect, 0); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + > + hr = IDirect3DDevice9_GetVertexShader(device, &vshader); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr); > + ok(!vshader, "Got non NULL vshader.\n"); > + > + hr = effect->lpVtbl->EndPass(effect); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + hr = effect->lpVtbl->End(effect); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + > + hr = effect->lpVtbl->SetTechnique(effect, "tech1"); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + hr = effect->lpVtbl->Begin(effect, &passes_count, 0); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + hr = effect->lpVtbl->BeginPass(effect, 0); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + > + hr = IDirect3DDevice9_GetVertexShader(device, &vshader); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr); > + ok(!!vshader, "Got NULL vshader.\n"); > + hr = IDirect3DVertexShader9_GetFunction(vshader, NULL, &byte_code_size); > + ok(hr == D3D_OK, "Got result %x.\n", hr); > + byte_code = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, byte_code_size); > + hr = IDirect3DVertexShader9_GetFunction(vshader, byte_code, &byte_code_size); > + ok(hr == D3D_OK, "Got result %x.\n", hr); > + ok(byte_code_size == TEST_EFFECT_UNSUPPORTED_SHADER_VSHADER_LEN * sizeof(DWORD), > + "Got unexpected byte code size %u.\n", byte_code_size); > + ok(!memcmp(byte_code, > + &test_effect_unsupported_shader_blob[TEST_EFFECT_UNSUPPORTED_SHADER_VSHADER_POS], > + byte_code_size), "Incorrect shader selected.\n"); I was confused by this at first, I thought somehow the unsupported vs_3_sw shader was set. That's not the case, TEST_EFFECT_UNSUPPORTED_SHADER_VSHADER_POS is the position of the vs_3_0 shader, but the naming throw me off. Can you rename those defines to "BYTECODE_VS_3_0_POS" or something that makes it clear that it refers to the hardware shader in vs_arr[0]?

2 2

Re: [PATCH v2 10/11] progman: Use proper size for combobox controls
by Alexandre Julliard 31 Aug '17

31 Aug '17

Fabian Maurer <dark.shadow4(a)web.de> writes: > Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> > --- > programs/progman/progman.rc | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/programs/progman/progman.rc b/programs/progman/progman.rc > index 4804330aed..9f451a736b 100644 > --- a/programs/progman/progman.rc > +++ b/programs/progman/progman.rc > @@ -119,7 +119,7 @@ LTEXT "", PM_PROGRAM, 95, 5, 90, 15 > LTEXT "From group:", -1, 5, 13, 90, 15 > LTEXT "", PM_FROM_GROUP, 95, 13, 90, 15 > LTEXT "&To group:", PM_TO_GROUP_TXT, 5, 28, 140, 15 > -COMBOBOX PM_TO_GROUP, 5, 38, 140, 50, WS_TABSTOP | CBS_DROPDOWNLIST > +COMBOBOX PM_TO_GROUP, 5, 38, 140, 60, WS_TABSTOP | CBS_DROPDOWNLIST That kind of change doesn't seem necessary. We don't need all comboboxes to have the exact same height, only something reasonable. -- Alexandre Julliard julliard(a)winehq.org

3 2

Wine fails to compile in kernel32
by Fabian Maurer 30 Aug '17

30 Aug '17

Hello wine-devel, I'm running ArchLinux with gcc 7.2.0. Since commit e99811aee2dff88c41d6e2875401810a85325839 wine fails to build with > ../../../wine-git/dlls/kernel32/module.c: In function ‘load_library’: > ../../../wine-git/dlls/kernel32/module.c:987:44: error: initializer element is not constant > static const DWORD unsupported_flags = load_library_search_flags | Minimal test-program to show the error, just use something like "gcc main.c" to compile. ---------- static const int load_library_search_flags = 0; int main() { static const int unsupported_flags = load_library_search_flags; return 0; } ---------- Looks like an compiler error, but I'm not sure. Mind looking into that? Regards, Fabian Maurer

1 1

Re: [PATCH] winecfg: Use proper size for combobox controls
by Nikolay Sivov 30 Aug '17

30 Aug '17

On 29.08.2017 20:06, Fabian Maurer wrote: > Fixes Bug 43632. > winecfg used the broken behavior that was fixed > in f7f7b89e2e9117811c91269643868c6d063db5e9, > so we need to adjust it. > > Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> > --- > programs/winecfg/winecfg.rc | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > Interesting thing to test I think is to verify if current cross-built winecfg shows the same issue on Windows, before changing them all.

2 1

Re: [PATCH] winecfg: Use proper size for combobox controls
by Alexandre Julliard 30 Aug '17

30 Aug '17

Fabian Maurer <dark.shadow4(a)web.de> writes: > Fixes Bug 43632. > winecfg used the broken behavior that was fixed > in f7f7b89e2e9117811c91269643868c6d063db5e9, > so we need to adjust it. > > Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> > --- > programs/winecfg/winecfg.rc | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) There are resources in other modules that look like they would have the same problem, a grep over the codebase would probably be useful. -- Alexandre Julliard julliard(a)winehq.org

2 3

WINED3DFMT_RESZ in wined3d.
by lorenzo Skotos 30 Aug '17

30 Aug '17

Hi. I was investigating a regression that cause the CheckDeviceFormat function to return WINED3DERR_NOTAVAILABLE when quering for RESZ surfaces. (WINED3DFMT_RESZ). This is causing OblivionReloaded (that worked properly until wine 2.5) to not work properly (it detect an incompatible hardware in wine or NVIDIA in wine-staging). I tacked the regression to this commit : https://github.com/wine-mirror/wine/commit/8c98be4791f18f31b04a4a0f08d18979… [https://camo.githubusercontent.com/db3db2bf09d082a1f4244859890e83954d8410ab…]<https://github.com/wine-mirror/wine/commit/8c98be4791f18f31b04a4a0f08d18979…> wined3d: Allow all formats with "glInternal" set in CheckSurfaceCapab… · wine-mirror/wine@8c98be4<https://github.com/wine-mirror/wine/commit/8c98be4791f18f31b04a4a0f08d18979…> github.com …ility(). Rendertarget, depth/stencil and texturing restrictions are applied later in wined3d_check_device_format(), so to the extent this make a difference it mostly affects off-screen plain surf... I tried to fix this issue. The last patch I made in my local branch is adding a wined3d_format_texture_info in the format_texture_info array: + {WINED3DFMT_RESZ, GL_DEPTH24_STENCIL8, GL_DEPTH24_STENCIL8, 0, + GL_DEPTH_STENCIL, GL_UNSIGNED_INT, 0, + WINED3DFMT_FLAG_TEXTURE | WINED3DFMT_FLAG_DEPTH | WINED3DFMT_FLAG_STENCIL | WINED3DFMT_FLAG_RENDERTARGET | WINED3DFMT_FLAG_FBO_ATTACHABLE, + ARB_FRAMEBUFFER_OBJECT, NULL}, However I know almost nothing about the internal work of wined3d and opengl and don't know if this kind of addition is correct. OblivionReloaded works and can properly use the RESZ depth texture. I based this patch on the INTZ and RAWZ surfaces that are somewhat reported to be similar in scope in some docs. Also I don't know if the flags are correct or even meaningful in this scope. I could not find any documentation about the internal meaning of these flags. As the good Jon Snow I know nothing. Note I never did a patch in wine before (except adding some debug message). So please tell me if I'm in a completly wrong direction. Also note that I didn't run the test suite. Using make test is failing hard on an advapi32 test even without this patch applied. I attached the full patch file.

1 0

Re: [PATCH] user32: In GetMinMaxInfo, use the default X pos even if the Y pos changes
by Alexandre Julliard 30 Aug '17

30 Aug '17

Andrew Eikum <aeikum(a)codeweavers.com> writes: > This fixes maximizing to the wrong monitor on a Mac with multiple > monitors. Maximizing with multiple monitors continues to work correctly > on Linux for me. > > Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> > --- > dlls/user32/winpos.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) I'm not sure it makes sense to use only one coordinate of the monitor rectangle. What are the coordinate values that trigger this? -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 2/2] d3dx9: Get rid of constants modification bitmasks.
by Matteo Bruni 29 Aug '17

29 Aug '17

2017-08-10 12:02 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> The patch is fine overall, I only have a few questions / nitpicks. I'll skip the part about the delay with the review because it's old news by now... :/ > --- > dlls/d3dx9_36/d3dx9_private.h | 3 +- > dlls/d3dx9_36/preshader.c | 312 ++++++++++++++++++------------------------ > 2 files changed, 135 insertions(+), 180 deletions(-) > > diff --git a/dlls/d3dx9_36/d3dx9_private.h b/dlls/d3dx9_36/d3dx9_private.h > index 2727463a18..5ac5e63aeb 100644 > --- a/dlls/d3dx9_36/d3dx9_private.h > +++ b/dlls/d3dx9_36/d3dx9_private.h > @@ -230,7 +230,7 @@ enum pres_reg_tables > struct d3dx_const_param_eval_output > { > struct d3dx_parameter *param; > - unsigned int table; > + enum pres_reg_tables table; The change is good but (unrelated to this patch) the enum type should really use the singular form. > static void dump_bytecode(void *data, unsigned int size) > @@ -674,6 +614,40 @@ static HRESULT append_const_set(struct d3dx_const_tab *const_tab, struct d3dx_co > return D3D_OK; > } > > +static void append_pres_const_set(struct d3dx_const_tab *const_tab, struct d3dx_preshader *pres) Nitpick: this function appends all the struct d3dx_const_param_eval_output from the preshader so calling it "append_pres_const_sets" (or even something more explicit to that effect, e.g. clarifying that you're appending entries about the preshader outputs, which are shader inputs) would be preferable. > +{ > + unsigned int i; > + struct d3dx_const_param_eval_output const_set; > + > + memset(&const_set, 0, sizeof(const_set)); You could initialize const_set in the definition instead, like: struct d3dx_const_param_eval_output const_set = {NULL}; which looks slightly better to me. It doesn't matter much of course. > @@ -971,10 +946,30 @@ static HRESULT get_constants_desc(unsigned int *byte_code, struct d3dx_const_tab > if (FAILED(hr = init_set_constants_param(out, ctab, hc, inputs_param[index]))) > goto cleanup; > } > + if (pres) > + append_pres_const_set(out, pres); > if (out->const_set_count) > { > struct d3dx_const_param_eval_output *new_alloc; > > + qsort(out->const_set, out->const_set_count, sizeof(*out->const_set), compare_const_set); > + for (i = 0; i < out->const_set_count - 1; ++i) > + { > + if (out->const_set[i].constant_class == D3DXPC_FORCE_DWORD > + && out->const_set[i + 1].constant_class == D3DXPC_FORCE_DWORD > + && out->const_set[i].table == out->const_set[i + 1].table > + && out->const_set[i].register_index + out->const_set[i].register_count > + >= out->const_set[i + 1].register_index) > + { > + out->const_set[i].register_count = out->const_set[i + 1].register_index > + + out->const_set[i + 1].register_count - out->const_set[i].register_index; I think this might lose account of some registers. E.g. consider the case where const_set[i] has register_index == 0 and register_count == 4 while const_set[i + 1] has register_index == 1 and register_count == 1. This would seem to lose track of the last two components. I know that preshaders found in the wild seem not to overwrite already written output registers but I wouldn't feel safe with (silently) depending on it. > + memmove(&out->const_set[i + 1], &out->const_set[i + 2], sizeof(out->const_set[i]) > + * (out->const_set_count - i - 2)); > + --out->const_set_count; > + --i; Mostly matter of personal preference but rather than decrementing the for counter in the if when coalescing I'd write this as a while and increment "i" only in the (currently absent) else case. > @@ -1529,22 +1534,56 @@ static void set_constants(struct d3dx_regstore *rs, struct d3dx_const_tab *const > data += param->rows * param->columns; > } > start_offset = get_offset_reg(table, const_set->register_index); > - if (table_info[table].type == param_type) > - regstore_set_modified(rs, table, start_offset, > - get_offset_reg(table, const_set->register_count) * const_set->element_count); > - else > + if (table_info[table].type != param_type) > regstore_set_data(rs, table, start_offset, (unsigned int *)rs->tables[table] + start_offset, > get_offset_reg(table, const_set->register_count) * const_set->element_count, param_type); > } > + > + > const_tab->update_version = new_update_version; One blank line too many. > -} > + if (!update_device) > + return D3D_OK; > > + for (const_idx = 0; const_idx < const_tab->const_set_count; ++const_idx) > + { > + struct d3dx_const_param_eval_output *const_set = &const_tab->const_set[const_idx]; > + > + if (device_update_all || (const_set->param > + ? is_param_dirty(const_set->param, update_version) : pres_dirty)) > + { > + enum pres_reg_tables table = const_set->table; > + > + if (table == device_table && device_start + device_count == const_set->register_index) Again nitpicking: are there better names for those variables? Maybe use the "current_" prefix instead? It's pretty clear what's going on either way, so up to you.

2 6

Re: [PATCH] comctl32/taskdialog: Add callback and tests
by Nikolay Sivov 28 Aug '17

28 Aug '17

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.

2 1

Re: [PATCH] ws2_32: Also check the FQDN when injecting the local IP.
by Alexandre Julliard 28 Aug '17

28 Aug '17

Stefan Dösinger <stefan(a)codeweavers.com> writes: > @@ -6576,6 +6592,8 @@ int WINAPI WS_getaddrinfo(LPCSTR nodename, LPCSTR servname, const struct WS_addr > > hostname = get_hostname(); > if (!hostname) return WSA_NOT_ENOUGH_MEMORY; > + fqdn = get_fqdn(); > + if (!fqdn) return WSA_NOT_ENOUGH_MEMORY; It seems to me that you shouldn't need to retrieve both separately, the FQDN contains the hostname already. -- Alexandre Julliard julliard(a)winehq.org

1 0

Issue with suspended process
by Jon Doron 25 Aug '17

25 Aug '17

Hi all, I have an application that is creating a process suspended and then using detours modifies the imports of the new process prior to resuming it (the application is called Tracker.exe and is part of MS toolchain, it can be retrieved in MS EWDK package) The application does not work as it should on Wine, as the DLL its trying to inject to the new process is not getting loaded, I tracked down the problem here are the technical details: When a new process is created, after all the other set-ups its running the function: dlls/ntdll/server.c!server_init_process_done This function sets up the handlers for the different signals (signal_init_process) and makes a wine_server_call init_process_done. In the wine server function server/process.c!init_process_done a check is made to see if the thread of the process needs to be suspended (stop_thread_if_suspended) and indeed when a process is created with the CREATE_SUSPENDED flag, thread->suspend == 1, process->suspend == 0, and the process state is done at this point). This makes the wine_server call to stop_thread which will send a SIGUSR1 signal to the new process. The new process however still have the signals in a block state, thus not processing the signal and putting it into a pending state. The next thing that happens is LdrInitializeThunk continues and fixing up the EXE module imports, this is not the same behavior as Windows in which the Ldr only starts running after the thread has resumed. Only later signals are being unblocked and then the thread enters a suspended state, the function which unblocks the signals today is attach_process_dlls The interesting thing is that in the past Wine behavior was correct and the change that basically broke it is commit: commit 0f5fc117a2320bb7e21b9ae4d07d717bed413171 Author: Alexandre Julliard <julliard(a)winehq.org> Date: Mon Nov 19 14:27:07 2007 +0100 ntdll: Unblock signals in process init only after the dlls have been imported. Unfortunately though there I could not find any reason why the change was made or if there is any bug that it fixed back then (2007) . I was thinking that perhaps in order to resolve the problem and be closer in the behavior to Windows would be to unblock the signals just before server_init_process_done returns. Please let me know if you have any other questions, will be happy to resolve this now that we have all the information laid out. Thanks, -- Jon.

3 3

Re: [PATCH resend] msxml3: Remove CRs in domtext_put_data and add them in domtext_get_xml.
by Nikolay Sivov 25 Aug '17

25 Aug '17

On 03.08.2017 18:17, Alex Henrie wrote: > Fixes https://bugs.winehq.org/show_bug.cgi?id=42468 > > To be clear, I am not trying to fix all whitespace processing issues. I > am only trying to prevent "" from being erroneously inserted into > the output. > > Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> > --- > dlls/msxml3/tests/domdoc.c | 106 +++++++++++++++++++++++++++++++++++++-------- > dlls/msxml3/text.c | 42 +++++++++++++++--- > 2 files changed, 122 insertions(+), 26 deletions(-) > > diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c I think it needs more tests, for xml:space and put_preserveWhiteSpace(). > diff --git a/dlls/msxml3/text.c b/dlls/msxml3/text.c > index 3a12d4bf62..6814c1f817 100644 > --- a/dlls/msxml3/text.c > +++ b/dlls/msxml3/text.c > @@ -519,10 +519,15 @@ static HRESULT WINAPI domtext_get_xml( > BSTR* p) > { > domtext *This = impl_from_IXMLDOMText( iface ); > + HRESULT hr; > > TRACE("(%p)->(%p)\n", This, p); > > - return node_get_xml(&This->node, FALSE, p); > + hr = node_get_xml(&This->node, FALSE, p); > + if (hr == S_OK) > + *p = EnsureCorrectEOL(*p); > + > + return hr; Will node_get_xml(..., TRUE, ...) work? > } > > static HRESULT WINAPI domtext_transformNode( > @@ -616,15 +621,38 @@ static HRESULT WINAPI domtext_put_data( > BSTR data) > { > domtext *This = impl_from_IXMLDOMText( iface ); > - static const WCHAR rnW[] = {'\r','\n',0}; > + BSTR normalized_data = NULL; > + HRESULT hr; > + int i, j; > > TRACE("(%p)->(%s)\n", This, debugstr_w(data)); > > - if (data && !strcmpW(rnW, data)) > - This->node.node->name = xmlStringTextNoenc; > - else > - domtext_reset_noenc(This); > - return node_set_content(&This->node, data); > + if (data) > + { > + /* normalize line endings */ > + normalized_data = SysAllocStringLen(NULL, SysStringLen(data)); > + if (!normalized_data) return E_OUTOFMEMORY; > + for (i = 0, j = 0; data[i]; i++) > + { > + if (data[i] == '\r') > + { > + if (data[i + 1] == '\n') > + continue; /* change \r\n to just \n */ > + else > + normalized_data[j] = '\n'; /* change \r by itself to \n */ > + } > + else > + normalized_data[j] = data[i]; > + j++; > + } > + normalized_data[j] = 0; The only purpose for that is to inhibit libxml2 escaping, right? Could this be done the way it is now, but with something more elaborate than comparing to "\r\n"? > + } > + > + domtext_reset_noenc(This); > + hr = node_set_content(&This->node, normalized_data); > + > + SysFreeString(normalized_data); > + return hr; > } > > static HRESULT WINAPI domtext_get_length( >

2 1

Re: [v2] d3d9/tests: Add test for SW shader in MVP mode.
by Marvin 25 Aug '17

25 Aug '17

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=32837 Your paranoid android. === w8 (32 bit visual) === visual.c:7952: Test failed: Input test: Quad 3(2crd-wrongidx) returned color 0x00ff00ff, expected 0x00ff0080

2 1

Issue with suspended process
by Jonathan Doron 25 Aug '17

25 Aug '17

Hi all, I have an application that is creating a process suspended and then using detours modifies the imports of the new process prior to resuming it (the application is called Tracker.exe and is part of MS toolchain, it can be retrieved in MS EWDK package) The application does not work as it should on Wine, as the DLL its trying to inject to the new process is not getting loaded, I tracked down the problem here are the technical details: When a new process is created, after all the other set-ups its running the function: dlls/ntdll/server.c!server_init_process_done This function sets up the handlers for the different signals (signal_init_process) and makes a wine_server_call init_process_done. In the wine server function server/process.c!init_process_done a check is made to see if the thread of the process needs to be suspended (stop_thread_if_suspended) and indeed when a process is created with the CREATE_SUSPENDED flag, thread->suspend == 1, process->suspend == 0, and the process state is done at this point). This makes the wine_server call to stop_thread which will send a SIGUSR1 signal to the new process. The new process however still have the signals in a block state, thus not processing the signal and putting it into a pending state. The next thing that happens is LdrInitializeThunk continues and fixing up the EXE module imports, this is not the same behavior as Windows in which the Ldr only starts running after the thread has resumed. Only later signals are being unblocked and then the thread enters a suspended state, the function which unblocks the signals today is attach_process_dlls The interesting thing is that in the past Wine behavior was correct and the change that basically broke it is commit: commit 0f5fc117a2320bb7e21b9ae4d07d717bed413171 Author: Alexandre Julliard <julliard(a)winehq.org> Date: Mon Nov 19 14:27:07 2007 +0100 ntdll: Unblock signals in process init only after the dlls have been imported. Unfortunately though there I could not find any reason why the change was made or if there is any bug that it fixed back then (2007) . I was thinking that perhaps in order to resolve the problem and be closer in the behavior to Windows would be to unblock the signals just before server_init_process_done returns. Please let me know if you have any other questions, will be happy to resolve this now that we have all the information laid out. Thanks, -- Jon.

2 1

Re: [PATCH 3/3] d3d9/tests: Add test for SW shader in MVP mode.
by Henri Verbeet 24 Aug '17

24 Aug '17

On 23 August 2017 at 16:17, Paul Gofman <gofmanp(a)gmail.com> wrote: > -static IDirect3DDevice9 *create_device(IDirect3D9 *d3d, HWND device_window, HWND focus_window, BOOL windowed) > +static IDirect3DDevice9 *create_device_behavior(IDirect3D9 *d3d, HWND device_window, HWND focus_window, > + BOOL windowed, DWORD behavior_flags) It's perhaps a bit of an uncomfortable change to make due to the number of tests in visual.c, but ideally this would match the version of create_device() in device.c. Lacking that, there are a few tests in visual.c that just call IDirect3D9_CreateDevice() directly. In either case, we'd like device.c and visual.c to converge instead of diverge. > + hr = IDirect3DDevice9_SetVertexShader(device, NULL); > + ok(SUCCEEDED(hr), "Got unexpected hr %#x.\n", hr); > + hr = IDirect3DDevice9_SetVertexShader(device, pure_sw_shader); > + ok(SUCCEEDED(hr), "Got unexpected hr %#x.\n", hr); Is resetting the shader to NULL really needed here? > + hr = IDirect3DDevice9_DrawPrimitiveUP(device, D3DPT_TRIANGLESTRIP, 2, quad, sizeof(*quad)); > + todo_wine > + ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#x.\n", hr); > + hr = IDirect3DDevice9_DrawPrimitiveUP(device, D3DPT_TRIANGLESTRIP, 2, quad, sizeof(*quad)); > + ok(SUCCEEDED(hr), "Got unexpected hr %#x.\n", hr); Odd. > + /* wined3d does not guarantee 0 for out of bounds constant access by default. */ IIRC neither does Windows, in general. Matteo may have a better recollection of the details, but IIRC while it mostly works out in practice on current NVIDIA hardware, it doesn't necessarily everywhere else, and is essentially hardware/driver specific behaviour.

2 2

Re: [PATCH 2/2] mshtml: Properly expose IHTMLDocument6 interface to scripts.
by Marvin 24 Aug '17

24 Aug '17

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

1 0

Issue with suspended process
by Jonathan Doron 24 Aug '17

24 Aug '17

Hi all, I have an application that is creating a process suspended and then using detours modifies the imports of the new process prior to resuming it (the application is called Tracker.exe and is part of MS toolchain, it can be retrieved in MS EWDK package) The application does not work as it should on Wine, as the DLL its trying to inject to the new process is not getting loaded, I tracked down the problem here are the technical details: When a new process is created, after all the other set-ups its running the function: dlls/ntdll/server.c!server_init_process_done This function sets up the handlers for the different signals (signal_init_process) and makes a wine_server_call init_process_done. In the wine server function server/process.c!init_process_done a check is made to see if the thread of the process needs to be suspended (stop_thread_if_suspended) and indeed when a process is created with the CREATE_SUSPENDED flag, thread->suspend == 1, process->suspend == 0, and the process state is done at this point). This makes the wine_server call to stop_thread which will send a SIGUSR1 signal to the new process. The new process however still have the signals in a block state, thus not processing the signal and putting it into a pending state. The next thing that happens is LdrInitializeThunk continues and fixing up the EXE module imports, this is not the same behavior as Windows in which the Ldr only starts running after the thread has resumed. Only later signals are being unblocked and then the thread enters a suspended state, the function which unblocks the signals today is attach_process_dlls The interesting thing is that in the past Wine behavior was correct and the change that basically broke it is commit: commit 0f5fc117a2320bb7e21b9ae4d07d717bed413171 Author: Alexandre Julliard <julliard(a)winehq.org> Date: Mon Nov 19 14:27:07 2007 +0100 ntdll: Unblock signals in process init only after the dlls have been imported. Unfortunately though there I could not find any reason why the change was made or if there is any bug that it fixed back then (2007) . I was thinking that perhaps in order to resolve the problem and be closer in the behavior to Windows would be to unblock the signals just before server_init_process_done returns. Please let me know if you have any other questions, will be happy to resolve this now that we have all the information laid out. Thanks, -- Jon.

1 0

Filling in ARM64 ABI structs
by Martin Storsjö 23 Aug '17

23 Aug '17

Hi, I've noticed that some of the ABI specific structs for ARM64 are incomplete or missing - namely _JUMP_BUFFER in include/msvcrt/setjmp.h (not yet available) and _CONTEXT (not missing but only has got made-up content so far) and associated CONTEXT_* defines, and structs like RUNTIME_FUNCTION and UNWIND_HISTORY_TABLE in include/winnt.h. As far as I know, these structs should be available in the win10 sdk. (MSDN doesn't have them but just says to refer to winnt.h for the CONTEXT struct at least). In order not to accidentally make myself uneligible for contributing code on these matters, I've tried to not look at the sdk. Can someone help out with completeing these data structures according to whatever clean-room guidelines are required? When looking at existing code with git blame, they mostly appear as such without any further explanation on how they were derived, so I'm taking the more cautious route here. // Martin

2 2

Re: [PATCH 3/3] d3d9/tests: Add test for SW shader in MVP mode.
by Marvin 23 Aug '17

23 Aug '17

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=32778 Your paranoid android. === w8 (32 bit visual) === visual.c:7975: Test failed: Input test: Quad 3(2crd-wrongidx) returned color 0x00ff00ff, expected 0x00ff0080 === w1064 (64 bit visual) === visual.c:8023: Test failed: Got unexpected color 0x00007580 for quad 2 (different colors).

2 1

JACK driver for WINE
by Jeremy Banks 23 Aug '17

23 Aug '17

Hello all, I am wanting to output the audio from various applications run in WINE to the JACK server on my computer. I would prefer that WINE do this natively instead of routing across various wrapper setups with ALSA or PulseAudio. I have looked on the WINE wiki page for sound[1] and see that there was a JACK driver at one point, but it was removed when the driver architecture was changed. I am interested in trying to implement a driver to get WINE talking to JACK natively. The wiki says to ask on the mailing list before starting such a project, and so here I am. I have downloaded a local copy of the source repo and have started browsing through its contents. Being unfamiliar with the WINE codebase, which portions of the source should I start studying first? I am interested in studying the drivers for ALSA, OSS, and Pulse for comparison; do those live in a particular place in the source tree? I'm presently looking on the Wiki for other helpful information, but is there some particular documentation I should be studying for this endeavor? Regards, Jeremy Banks 1. https://wiki.winehq.org/Sound#Backend_information

3 4

Killing https://www.winehq.org/docs/wineusr-guide
by Austin English 22 Aug '17

22 Aug '17

Hi Rosanne, I saw on https://bugs.winehq.org/show_bug.cgi?id=43525 that wineusr-guide is deprecated (which I realized), but I didn't realize it was still up. Would you (or anyone else) be opposed to completely removing it? IMO it's doing more harm than good, especially without a deprecated notice on it. -- -Austin GPG: 14FB D7EA A041 937B

3 3

Re: [PATCH 4/4] mshtml: Added delayed DispatchEx init support and use it to expose IHTMLDOMNode3 from document node.
by Marvin 21 Aug '17

21 Aug '17

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

1 0

Re: [PATCH] evr: Add stubs for EnhancedVideoRenderer
by Alistair Leslie-Hughes 18 Aug '17

18 Aug '17

Hi Fabian, On 18/08/17 11:25, Fabian Maurer wrote: > + if (IsEqualGUID(riid, &IID_IUnknown) > + || IsEqualGUID(riid, &IID_IBaseFilter)) > + { Your missing some IID checks here. Alistair.

2 3

Re: [PATCH] evr: Add stubs for EnhancedVideoRenderer
by Nikolay Sivov 18 Aug '17

18 Aug '17

On 18.08.2017 4:25, Fabian Maurer wrote: > +static const IBaseFilterVtbl IBaseFilter_Vtbl = > +{ > + /*** IUnknown methods ***/ > + EnhancedVideoRendererImpl_QueryInterface, > + EnhancedVideoRendererImpl_AddRef, > + EnhancedVideoRendererImpl_Release, > + /*** IPersist methods ***/ > + EnhancedVideoRendererImpl_GetClassID, > + /*** IMediaFilter methods ***/ > + EnhancedVideoRendererImpl_Stop, > + EnhancedVideoRendererImpl_Pause, > + EnhancedVideoRendererImpl_Run, > + EnhancedVideoRendererImpl_GetState, > + EnhancedVideoRendererImpl_SetSyncSource, > + EnhancedVideoRendererImpl_GetSyncSource, > + /*** IBaseFilter methods ***/ > + EnhancedVideoRendererImpl_EnumPins, > + EnhancedVideoRendererImpl_FindPin, > + EnhancedVideoRendererImpl_QueryFilterInfo, > + EnhancedVideoRendererImpl_JoinFilterGraph, > + EnhancedVideoRendererImpl_QueryVendorInfo > +}; Shouldn't it use strmbase? > +HRESULT EnhancedVideoRendererImpl_Create(IUnknown *pUnkOuter, void **ppObj) > +{ > + EnhancedVideoRendererImpl *object; > + > + TRACE("(%p,%p)\n", pUnkOuter, ppObj); > + > + object = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(EnhancedVideoRendererImpl)); > + if (!object) > + return E_OUTOFMEMORY; > + > + object->IBaseFilter_iface.lpVtbl = &IBaseFilter_Vtbl; > + object->ref = 1; > + > + *ppObj = &object->IBaseFilter_iface; > + > + return S_OK; > +} This needs at test for aggregation support. > +typedef struct { > + IBaseFilter IBaseFilter_iface; > + LONG ref; > +} EnhancedVideoRendererImpl; Does this need to be exposed? > + TRACE("(0x%p, %d, %p)\n", instance, reason, reserved); > + Format looks wrong. > - case DLL_WINE_PREATTACH: > - return FALSE; /* prefer native version */ It still doesn't do anything. > +static HRESULT WINAPI XFCF_QueryInterface(LPCLASSFACTORY iface,REFIID riid, void **ppobj) > +{ What does XF prefix stand for? Also please use consistent type names and formatting. > +/******************************************************************************* > + * Retrieves class object from a DLL object > + * > + * NOTES > + * Docs say returns STDAPI > + * > + * PARAMS > + * rclsid [I] CLSID for the class object > + * riid [I] Reference to identifier of interface for class object > + * ppv [O] Address of variable to receive interface pointer for riid > + * > + * RETURNS > + * Success: S_OK > + * Failure: CLASS_E_CLASSNOTAVAILABLE, E_OUTOFMEMORY, E_INVALIDARG, > + * E_UNEXPECTED > + */ > +HRESULT WINAPI DllGetClassObject(REFCLSID rclsid, REFIID riid, void **ppv) I personally don't think this export needs a documentation header. > + if ( !IsEqualGUID( &IID_IClassFactory, riid ) > + && ! IsEqualGUID( &IID_IUnknown, riid) ) > + return E_NOINTERFACE; I think it's better to let QueryInterface handle this. > + for (i=0; i < sizeof(object_creation)/sizeof(object_creation[0]); i++) > + { > + if (IsEqualGUID(object_creation[i].clsid, rclsid)) > + break; > + } > + > + if (i == sizeof(object_creation)/sizeof(object_creation[0])) > + { > + FIXME("%s: no class found.\n", debugstr_guid(rclsid)); > + return CLASS_E_CLASSNOTAVAILABLE; > + } Does it support any other CLSIDs? This seems like too much. Static factory instance will work too I suppose.

2 3

Re: [1/2] user32/tests: Test DialogBoxParam using a dialog template with invalid control class.
by Sebastian Lackner 18 Aug '17

18 Aug '17

On 18.08.2017 13:41, Dmitry Timoshkov wrote: > Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> > Signed-off-by: Sebastian Lackner <sebastian(a)fds-team.de> > --- > dlls/user32/tests/dialog.c | 6 ++++++ > dlls/user32/tests/resource.rc | 6 ++++++ > 2 files changed, 12 insertions(+) > There is no need to resubmit, the patches are exactly identical to what Andrew submitted. Also, why the hell is there my sign-off? It is apparently not fine for you when Andrew wants to take responsiblity for your patches, but you expect me to take responsibility for it, without confirming that I'm fine with it first? For other people I would now start to explain that Wine Staging also accepts workarounds and imperfect patches, and that the addition to Wine Staging does not imply my sign-off, but I'm pretty sure you are well aware of it...

2 1

Re: [1/2] user32/tests: Test DialogBoxParam using a dialog template with invalid control class.
by Marvin 18 Aug '17

18 Aug '17

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=32738 Your paranoid android. === w7u (32 bit msg) === msg.c:5084: Test failed: ShowWindow(SW_RESTORE):overlapped: 18: the msg sequence is not complete: expected 0000 - actual 0088 msg.c:5207: Test failed: SetWindowPos:show_popup_first_show_window: 11: the msg 0x001c was expected, but got msg 0x0047 instead msg.c:5207: Test failed: SetWindowPos:show_popup_first_show_window: 12: the msg 0x0086 was expected, but got msg 0x001c instead msg.c:5207: Test failed: SetWindowPos:show_popup_first_show_window: 13: the msg 0x0006 was expected, but got msg 0x0086 instead msg.c:5207: Test failed: SetWindowPos:show_popup_first_show_window: 18: the msg 0x0009 was expected, but got msg 0x0006 instead msg.c:5207: Test failed: SetWindowPos:show_popup_first_show_window: 19: the msg 0x0007 was expected, but got msg 0x0009 instead msg.c:5207: Test failed: SetWindowPos:show_popup_first_show_window: 20: the msg 0x0047 was expected, but got msg 0x0007 instead msg.c:5207: Test failed: SetWindowPos:show_popup_first_show_window: 21: the msg 0x0003 was expected, but got msg 0x0047 instead msg.c:5207: Test failed: SetWindowPos:show_popup_first_show_window: 22: the msg sequence is not complete: expected 0000 - actual 0003 === wvistau64 (32 bit listbox) === listbox.c:706: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY, *) filled with 7 entries, expected > 7 listbox.c:941: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY|DDL_EXCLUSIVE, *) failed err 18 === wvistau64 (64 bit listbox) === listbox.c:706: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY, *) filled with 7 entries, expected > 7 listbox.c:941: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY|DDL_EXCLUSIVE, *) failed err 18 === w864 (32 bit broadcast) === The task timed out === w8 (32 bit sysparams) === sysparams.c:2519: Test failed: Waiting for the WM_DISPLAYCHANGE message timed out sysparams.c:2530: Test failed: Set bpp 32, but WM_DISPLAYCHANGE reported bpp -1 === w1064 (32 bit sysparams) === sysparams.c:206: Test failed: Unexpected WM_DISPLAYCHANGE message === w864 (64 bit sysparams) === The task timed out

1 0

Re: [v5 PATCH 1/3] dwrite: Protect cached fontface list when accessed from multiple threads
by Anton Romanov 18 Aug '17

18 Aug '17

On Tue, Aug 15, 2017 at 4:40 AM, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > --- > dlls/dwrite/dwrite_private.h | 15 ++++++++--- > dlls/dwrite/font.c | 30 +++++++++++++-------- > dlls/dwrite/main.c | 63 +++++++++++++++++++++++++++++++------------- > 3 files changed, 74 insertions(+), 34 deletions(-) > Since apps seem to freely use fontface interfaces from multiple threads - Isn't the following race possible? +----------------+------------------------+-------------------------+ | T1 | T2 | T3 | +----------------+------------------------+-------------------------+ | Release | | | | Decrement | | | | == 0 -> True | | | | | Query_Interface/AddRef | | | | Increment | | | | | Release | | | | Decrement | | free(cached) | | | | factory_unlock | | | | | | use after free (cached) | +----------------+------------------------+-------------------------+

3 7

Re: [PATCH] comctl32/listview: Keep the active item selected after hwndEdit is destroyed
by Nikolay Sivov 17 Aug '17

17 Aug '17

On 10.08.2017 7:54, Hugh McMaster wrote: > Signed-off-by: Hugh McMaster <hugh.mcmaster(a)outlook.com> > --- > dlls/comctl32/listview.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c > index 73eceff509..05bd375571 100644 > --- a/dlls/comctl32/listview.c > +++ b/dlls/comctl32/listview.c > @@ -10216,6 +10216,7 @@ static LRESULT LISTVIEW_LButtonDown(LISTVIEW_INFO *infoPtr, WORD wKey, INT x, IN > static BOOL bGroupSelect = TRUE; > POINT pt = { x, y }; > INT nItem; > + BOOL editing = FALSE; > > TRACE("(key=%hu, X=%u, Y=%u)\n", wKey, x, y); > > @@ -10309,11 +10310,20 @@ static LRESULT LISTVIEW_LButtonDown(LISTVIEW_INFO *infoPtr, WORD wKey, INT x, IN > else > { > if (!infoPtr->bFocus) > + { > + editing = IsWindow(infoPtr->hwndEdit); > SetFocus(infoPtr->hwndSelf); > + } > > - /* remove all selections */ > + /* keep the active item selected after hwndEdit is destroyed, or deselect all items */ > if (!(wKey & MK_CONTROL) && !(wKey & MK_SHIFT)) > - LISTVIEW_DeselectAll(infoPtr); > + { > + if (editing) > + LISTVIEW_DeselectAllSkipItem(infoPtr, infoPtr->nFocusedItem); > + else > + LISTVIEW_DeselectAll(infoPtr); > + } > + Maybe this could be reduced to a single unconditional call: LISTVIEW_DeselectAllSkipItem(infoPtr, infoPtr->nEditLabelItem); > ReleaseCapture(); > } > >

1 0

Re: [PATCH 1/2] user32/tests: Test DialogBoxParam using a dialog template with invalid control class.
by Marvin 17 Aug '17

17 Aug '17

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=32717 Your paranoid android. === wvistau64 (32 bit listbox) === listbox.c:706: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY, *) filled with 7 entries, expected > 7 listbox.c:941: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY|DDL_EXCLUSIVE, *) failed err 18 === wvistau64 (64 bit listbox) === listbox.c:706: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY, *) filled with 7 entries, expected > 7 listbox.c:941: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY|DDL_EXCLUSIVE, *) failed err 18 === w864 (64 bit broadcast) === The task timed out === w8 (32 bit sysparams) === sysparams.c:2519: Test failed: Waiting for the WM_DISPLAYCHANGE message timed out sysparams.c:2530: Test failed: Set bpp 32, but WM_DISPLAYCHANGE reported bpp -1 === w864 (32 bit sysparams) === sysparams.c:2519: Test failed: Waiting for the WM_DISPLAYCHANGE message timed out sysparams.c:2530: Test failed: Set bpp 32, but WM_DISPLAYCHANGE reported bpp -1

2 1

Re: [PATCH 1/2] user32/tests: Test DialogBoxParam using a dialog template with invalid control class.
by Dmitry Timoshkov 17 Aug '17

17 Aug '17

Andrew Eikum <aeikum(a)codeweavers.com> wrote: > From: Dmitry Timoshkov <dmitry(a)baikal.ru> > > Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> > --- > dlls/user32/tests/dialog.c | 6 ++++++ > dlls/user32/tests/resource.rc | 6 ++++++ > 2 files changed, 12 insertions(+) I think that I've expressed my opinion clear enough in my response to your private e-mail that I don't allow you to send my patches included in the wine-staging tree to winehq, and explained well enough my reasoning. The only person who has my permission is Sebastian Lackner - wine-staging maintainer. I'm too sometimes sending the patches, but I have to admit that's pretty rare cases these days, and usually that's not my personal decision. Please try to find your own areas to work on, and stop stealing patches created by somebody else, especially when directly asked not to. -- Dmitry.

3 12

Re: [v5 PATCH 1/3] dwrite: Protect cached fontface list when accessed from multiple threads
by Sebastian Lackner 17 Aug '17

17 Aug '17

On 15.08.2017 13:40, Nikolay Sivov wrote: > if (cached_key_size == key_size && !memcmp(cached_key, key, key_size)) { > + if (FAILED(hr = IDWriteFontFace4_QueryInterface(cached->fontface, riid, obj))) > + WARN("Failed to get %s from fontface, hr %#x.\n", debugstr_guid(riid), hr); > + > TRACE("returning cached fontface %p\n", cached->fontface); > - *font_face = (IDWriteFontFace*)cached->fontface; > - return S_OK; > + break; > } > } There is still a minor problem (no race condition though). Lets assume a fontface is just about to be destroyed, then the code above will abort enumeration after reaching the fontface with refcount == 0. Lets assume the newly created fontface is added after the old one, then all following attempts to get the fontface from the cache will fail, until the old one has finally be removed. This issue can be avoided by doing "continue;" instead of "break;" for E_FAIL.

1 0

Re: [PATCH] kernel32/nls: Added LOCALE_SNAN entries
by Henri Verbeet 17 Aug '17

17 Aug '17

On 17 August 2017 at 00:28, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > diff --git a/dlls/kernel32/nls/far.nls b/dlls/kernel32/nls/far.nls > index 0f5b17068b..2baf289223 100644 > --- a/dlls/kernel32/nls/far.nls > +++ b/dlls/kernel32/nls/far.nls > @@ -124,6 +124,7 @@ STRINGTABLE LANGUAGE LANG_PERSIAN, SUBLANG_DEFAULT > LOCALE_SMONTHNAME13 "" > LOCALE_SMONTHOUSANDSEP "٬" > LOCALE_SNAME "fa-IR" > + LOCALE_SNAN "ﻥﺎﻋﺩﺩ" That doesn't look right. According to http://www.unicode.org/cldr/charts/latest/by_type/numbers.symbols.html#2a6d… "ناعدد" is correct, but the above is using the Presentation Forms-B characters.

2 1

Re: [PATCH] msi: Print error number when GetProcAddress fails
by Hans Leidekker 17 Aug '17

17 Aug '17

On Wed, 2017-08-16 at 17:20 +0100, Carlos Palminha wrote: > diff --git a/dlls/msi/custom.c b/dlls/msi/custom.c > index 922927e8903..cfdfe852efb 100644 > --- a/dlls/msi/custom.c > +++ b/dlls/msi/custom.c > @@ -608,7 +608,7 @@ static DWORD ACTION_CallDllFunction( const GUID *guid ) > ERR("failed to create handle for %p\n", remote_package ); > } > else > - ERR("GetProcAddress(%s) failed\n", debugstr_w( function ) ); > + ERR("GetProcAddress(%s) failed: %d\n", debugstr_w( function ), GetLastError() ); > > FreeLibrary(hModule); This doesn't appear to add any useful information. Our implementation of GetProcAddress sets either ERROR_PROC_NOT_FOUND or ERROR_MOD_NOT_FOUND and LoadLibrary must have succeeded before reaching this code.

3 2

Re: [PATCH v3] user32/combo: Set listview popup height correctly and add tests
by Nikolay Sivov 16 Aug '17

16 Aug '17

It's a listbox, not listview.

2 1

D3D8 Software Vertex Processing
by Jan Havran 16 Aug '17

16 Aug '17

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

2 6

Re: [PATCH] gdiplus: Implement transform matrix for line gradient brushes
by Vincent Povirk 16 Aug '17

16 Aug '17

I wrote a test program (attached), and it doesn't work when the start point isn't 0,0. > + if (line->startpoint.X == line->endpoint.X) > + t = line->startpoint.Y > line->endpoint.Y ? 3.f * M_PI / 2.f : M_PI / 2.f; > + else > + t = atanf((line->endpoint.Y - line->startpoint.Y) / (line->endpoint.X - line->startpoint.X)); > + > + t_cos = cosf(t); > + t_sin = sinf(t); This seems like an oddly complicated and fragile way to normalize a vector.

2 1

Re: [PATCH] avrt: Add AvQuerySystemResponsiveness stub.
by Huw Davies 16 Aug '17

16 Aug '17

On Wed, Aug 16, 2017 at 02:22:13PM +0300, Andrey Gusev wrote: > Signed-off-by: Andrey Gusev <andrey.goosev(a)gmail.com> > --- > dlls/avrt/avrt.spec | 2 +- > dlls/avrt/main.c | 6 ++++++ > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/dlls/avrt/avrt.spec b/dlls/avrt/avrt.spec > index 640a12ac22..a7dfc42879 100644 > --- a/dlls/avrt/avrt.spec > +++ b/dlls/avrt/avrt.spec > @@ -1,4 +1,4 @@ > -@ stub AvQuerySystemResponsiveness > +@ stdcall AvQuerySystemResponsiveness(long ptr) > @ stdcall AvRevertMmThreadCharacteristics(long) > @ stub AvRtCreateThreadOrderingGroup > @ stub AvRtCreateThreadOrderingGroupExA > diff --git a/dlls/avrt/main.c b/dlls/avrt/main.c > index 8a110250ae..68e44d6098 100644 > --- a/dlls/avrt/main.c > +++ b/dlls/avrt/main.c > @@ -83,6 +83,12 @@ HANDLE WINAPI AvSetMmThreadCharacteristicsW(LPCWSTR TaskName, LPDWORD TaskIndex) > return (HANDLE)0x12345678; > } > > +BOOL WINAPI AvQuerySystemResponsiveness(HANDLE AvrtHandle, PULONG SystemResponsivenessValue) > +{ > + FIXME("(%p, %p): stub\n", AvrtHandle, SystemResponsivenessValue); > + return TRUE; > +} > + If you're going to return TRUE, you need to initialize *SystemResponsivenessValue to something meaningful. While you're at it, we generally prefer 'ULONG *' to 'PULONG', although I realise there are several of the latter form in this file. Huw.

2 3

Re: [PATCH v2] server: Use queue_irp() also for synchronous IRPs.
by Alexandre Julliard 16 Aug '17

16 Aug '17

Zebediah Figura <z.figura12(a)gmail.com> writes: > Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> > --- > server/device.c | 46 ++++++++++++++++++++-------------------------- > 1 file changed, 20 insertions(+), 26 deletions(-) It's probably a matter of taste, but this doesn't look to me like an improvement. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [v4 5/5] kernel32/tests: Add tests for LoadLibrary with LIBRARY_SEARCH_* flags
by Marvin 16 Aug '17

16 Aug '17

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=32694 Your paranoid android. === wvistau64 (32 bit module) === module.c:479: Test failed: Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got 8 === w2008s64 (32 bit module) === module.c:479: Test failed: Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got 8 === wvistau64 (64 bit module) === module.c:479: Test failed: Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got 8 === w2008s64 (64 bit module) === module.c:479: Test failed: Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got 8

1 0

Re: [PATCH] usp10: fall back to 'dflt' language if shaping language tag not found
by Henri Verbeet 15 Aug '17

15 Aug '17

On 14 August 2017 at 20:04, Aric Stewart <aric(a)codeweavers.com> wrote: > + script = get_opentype_script(hdc,psa,psc,FALSE); Does that do anything? If I'm reading the code correctly that will set it to the same value it already was. (In which case most of this would simplify to "if (!script) script = MS_MAKE_TAG('l','a','t','n');")

2 2

Re: [PATCH v3 5/5] ieframe: Add stub implementation of IInternetExplorerManager.
by Jacek Caban 15 Aug '17

15 Aug '17

On 10.08.2017 20:52, Zebediah Figura wrote: > v2: register IE and IEM separately, and the latter with SINGLEUSE > (thanks Jacek) > > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> > --- > dlls/ieframe/ieframe.h | 5 +- > dlls/ieframe/ieframe_main.c | 35 ++++--------- > dlls/ieframe/iexplore.c | 111 ++++++++++++++++++++++++++++++++++++++--- > dlls/ieframe/tests/ie.c | 21 ++++++++ > programs/iexplore/iexplore.inf | 3 ++ > 5 files changed, 142 insertions(+), 33 deletions(-) > > diff --git a/dlls/ieframe/ieframe.h b/dlls/ieframe/ieframe.h > index d1f1c5daf24..c690d5c6878 100644 > --- a/dlls/ieframe/ieframe.h > +++ b/dlls/ieframe/ieframe.h > @@ -312,13 +312,16 @@ TID_LIST > } tid_t; > > HRESULT get_typeinfo(tid_t,ITypeInfo**) DECLSPEC_HIDDEN; > -HRESULT register_class_object(BOOL) DECLSPEC_HIDDEN; > > HRESULT WINAPI CUrlHistory_Create(IClassFactory*,IUnknown*,REFIID,void**) DECLSPEC_HIDDEN; > HRESULT WINAPI InternetExplorer_Create(IClassFactory*,IUnknown*,REFIID,void**) DECLSPEC_HIDDEN; > HRESULT WINAPI InternetShortcut_Create(IClassFactory*,IUnknown*,REFIID,void**) DECLSPEC_HIDDEN; > HRESULT WINAPI WebBrowser_Create(IClassFactory*,IUnknown*,REFIID,void**) DECLSPEC_HIDDEN; > HRESULT WINAPI WebBrowserV1_Create(IClassFactory*,IUnknown*,REFIID,void**) DECLSPEC_HIDDEN; > +HRESULT WINAPI InternetExplorerManager_Create(IClassFactory*,IUnknown*,REFIID,void**) DECLSPEC_HIDDEN; > + > +extern IClassFactory InternetExplorerFactory; > +extern IClassFactory InternetExplorerManagerFactory; This is missing DECLSPEC_HIDDEN. > > +/****************************************************************** > + * IInternetExplorerManager implementation > + */ > +struct internet_explorer_manager { > + IInternetExplorerManager IInternetExplorerManager_iface; > + LONG ref; > +}; Please use the type from public declaration (coclass). See how InternetExplorer object is declared. > +HRESULT WINAPI InternetExplorerManager_Create(IClassFactory *iface, IUnknown *pOuter, REFIID riid, void **ppv) > +{ > + struct internet_explorer_manager *ret; > + > + 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; > + return IInternetExplorerManager_QueryInterface(&ret->IInternetExplorerManager_iface, riid, ppv); > +} This leaks the object if QueryInterface fails. The usual way to handle is to set initial ref to 1 and release the object after QI() call. > +static void test_InternetExplorerManager(void) > +{ > + IUnknown *unk; > + ULONG ref; > + HRESULT hres; > + > + hres = CoCreateInstance(&CLSID_InternetExplorerManager, NULL, CLSCTX_LOCAL_SERVER, > + &IID_IInternetExplorerManager, (void**)&unk); > + ok(hres == S_OK || hres == REGDB_E_CLASSNOTREG, "Could not create InternetExplorerManager instance: %08x\n", hres); Use broken() for failure with older versions. > + > + if(hres != S_OK) > + return; This could use a win_skip(). Thanks, Jacek

1 0

Re: [PATCH v3 4/5] ieframe: Parse multiple command-line arguments.
by Jacek Caban 15 Aug '17

15 Aug '17

Hi Zebediah, On 10.08.2017 20:52, Zebediah Figura wrote: > + if (!strncmpiW(cmdline, embeddingW, length)) > + embedding = TRUE; > + else if (!strncmpiW(cmdline, nohomeW, length)) > + nohome = TRUE; > + else if (*cmdline == '-') > + FIXME("ignoring unknown option %s\n", debugstr_wn(cmdline, length)); It's probably not very important, but with native IE unknown options seem to be treated as URLs (iexplore.exe -test tries to open "-test" URL). Thanks, Jacek

1 0

Re: [PATCH 2/3] include: Add CryptProtectMemory and CryptUnprotectMemory.
by Michael Stefaniuc 15 Aug '17

15 Aug '17

On 08/15/2017 05:00 PM, Alex Henrie wrote: > Needed for PuTTY to compile with Winelib. > > Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> > --- > include/wincrypt.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/include/wincrypt.h b/include/wincrypt.h > index abbe99f245..058798c4f4 100644 > --- a/include/wincrypt.h > +++ b/include/wincrypt.h > @@ -4428,6 +4428,9 @@ BOOL WINAPI CryptUnprotectData( DATA_BLOB* pDataIn, LPWSTR* ppszDataDescr, > DATA_BLOB* pOptionalEntropy, PVOID pvReserved, > CRYPTPROTECT_PROMPTSTRUCT* pPromptStruct, DWORD dwFlags, DATA_BLOB* pDataOut ); > > +BOOL WINAPI CryptProtectMemory(void *pData, DWORD cbData, DWORD dwFlags); > +BOOL WINAPI CryptUnrotectMemory(void *pData, DWORD cbData, DWORD dwFlags); Isn't that missing a p^ ? bye michael > + > DWORD WINAPI CertGetNameStringA(PCCERT_CONTEXT pCertContext, DWORD dwType, > DWORD dwFlags, void *pvTypePara, LPSTR pszNameString, DWORD cchNameString); > DWORD WINAPI CertGetNameStringW(PCCERT_CONTEXT pCertContext, DWORD dwType, >

2 1

Re: [3/8] bcrypt: Add a stub implementation of BCryptEncrypt.
by Jacek Caban 15 Aug '17

15 Aug '17

Hi Hans, On 15.08.2017 14:51, Hans Leidekker wrote: > diff --git a/dlls/bcrypt/bcrypt_main.c b/dlls/bcrypt/bcrypt_main.c > index 3d70c15e7c..ccab676c1a 100644 > --- a/dlls/bcrypt/bcrypt_main.c > +++ b/dlls/bcrypt/bcrypt_main.c > @@ -515,7 +515,7 @@ NTSTATUS WINAPI BCryptGetProperty( BCRYPT_HANDLE handle, LPCWSTR prop, UCHAR *bu > NTSTATUS WINAPI BCryptSetProperty( BCRYPT_HANDLE handle, const WCHAR *prop, UCHAR *value, ULONG size, ULONG flags ) > { > FIXME( "%p, %s, %p, %u, %08x\n", handle, debugstr_w(prop), value, size, flags ); > - return STATUS_SUCCESS; > + return STATUS_NOT_IMPLEMENTED; I think this part was meant to be in patch 2/8. Thanks, Jacek

1 0

Re: [PATCH] user32/combo: Set listview popup height correctly and add tests
by Marvin 14 Aug '17

14 Aug '17

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=32672 Your paranoid android. === wxppro (32 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 00000000 === w2003std (32 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 00000000 === wvistau64 (32 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 00000000 === w2008s64 (32 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 00000000 === w7u (32 bit combo) === combo.c:786: Test failed: Test 2, expected list height to be 16, got 0 combo.c:786: Test failed: Test 3, expected list height to be 16, got 18 combo.c:786: Test failed: Test 4, expected list height to be 32, got 18 combo.c:786: Test failed: Test 5, expected list height to be 32, got 36 combo.c:786: Test failed: Test 6, expected list height to be 32, got 36 combo.c:786: Test failed: Test 7, expected list height to be 32, got 36 combo.c:786: Test failed: Test 8, expected list height to be 32, got 36 combo.c:786: Test failed: Test 11, expected list height to be 16, got 0 combo.c:786: Test failed: Test 12, expected list height to be 16, got 18 combo.c:786: Test failed: Test 13, expected list height to be 32, got 18 combo.c:786: Test failed: Test 14, expected list height to be 48, got 36 combo.c:781: Test failed: Test 15, expected list height to be 48, got 54 combo.c:786: Test failed: Test 16, expected list height to be 64, got 54 combo.c:786: Test failed: Test 17, expected list height to be 64, got 72 combo.c:315: Test failed: Focus not on Main Window, instead on 00000000 === w7pro64 (32 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 00000000 === w8 (32 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 00000000 === w864 (32 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 00000000 === w1064 (32 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 00000000 === wvistau64 (64 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 0000000000000000 === w2008s64 (64 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 0000000000000000 === w7pro64 (64 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 0000000000000000 === w864 (64 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 0000000000000000 === w1064 (64 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 0000000000000000

2 1

Re: [1/3] shell32: Implement SHCreateItemFromRelativeName.
by Huw Davies 14 Aug '17

14 Aug '17

On Sat, Aug 12, 2017 at 11:42:40PM +0800, Jactry Zeng wrote: > +HRESULT WINAPI SHCreateItemFromRelativeName(IShellItem *parent, PCWSTR name, IBindCtx *pbc, > + REFIID riid, void **ppv) > +{ > + LPITEMIDLIST pidl_folder = NULL, pidl = NULL; > + IShellFolder *desktop = NULL, *folder = NULL; > + HRESULT hr; > + > + TRACE("(%p, %p, %p, %s, %p)\n", parent, name, pbc, debugstr_guid(riid), ppv); Use debugstr_w() to trace 'name'. > + > + if(!ppv) > + return E_INVALIDARG; > + *ppv = NULL; > + if(!name) > + return E_INVALIDARG; > + > + hr = SHGetIDListFromObject((IUnknown*)parent, &pidl_folder); > + if(hr != S_OK) > + return hr; > + > + hr = SHGetDesktopFolder(&desktop); > + if(hr != S_OK) > + goto cleanup; > + > + if(!_ILIsDesktop(pidl_folder)) > + { > + hr = IShellFolder_BindToObject(desktop, pidl_folder, pbc, &IID_IShellFolder, > + (void**)&folder); You should pass a NULL IBindCtx here. pbc is associated with binding the name. > + if(hr != S_OK) > + goto cleanup; > + } > + > + hr = IShellFolder_ParseDisplayName(folder ? folder : desktop, NULL, pbc, (LPWSTR)name, > + NULL, &pidl, NULL); > + if(hr != S_OK) > + goto cleanup; > + hr = SHCreateItemFromIDList(pidl, riid, ppv); > + > diff --git a/dlls/shell32/tests/shlfolder.c b/dlls/shell32/tests/shlfolder.c > index 9e7127dfea..cbab88035a 100644 > --- a/dlls/shell32/tests/shlfolder.c > +++ b/dlls/shell32/tests/shlfolder.c > @@ -2519,6 +2523,86 @@ static void test_SHCreateShellItem(void) > else > win_skip("No SHCreateItemFromIDList\n"); > > + /* SHCreateItemFromRelativeName */ > + if(pSHCreateItemFromRelativeName && pSHGetKnownFolderPath) > + { > + IShellItem *shellitem_desktop = NULL; > + WCHAR *desktop_path, *displayname; > + WCHAR testfile_path[MAX_PATH] = {0}; > + HANDLE file; > + LPITEMIDLIST pidl_desktop_testfile = NULL; > + int order; > + > + ret = pSHCreateShellItem(NULL, NULL, pidl_desktop, &shellitem_desktop); > + ok(ret == S_OK, "SHCreateShellItem failed: 0x%08x.\n", ret); > + > + shellitem = (void*)0xdeadbeef; > + ret = pSHCreateItemFromRelativeName(shellitem_desktop, NULL, NULL, &IID_IShellItem, > + (void**)&shellitem); > + ok(ret == E_INVALIDARG, "Expected 0x%08x but SHCreateItemFromRelativeName return: 0x%08x.\n", > + E_INVALIDARG, ret); > + ok(shellitem == NULL, "shellitem was %p.\n", shellitem); > + > + /* Test with a non-existent file */ > + shellitem = (void*)0xdeadbeef; > + ret = pSHCreateItemFromRelativeName(shellitem_desktop, testfileW, NULL, &IID_IShellItem, > + (void**)&shellitem); > + ok(ret == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND), > + "Expected 0x%08x but SHCreateItemFromRelativeName return: 0x%08x.\n", > + HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND), ret); > + ok(shellitem == NULL, "shellitem was %p.\n", shellitem); > + > + /* Create a file for testing in desktop folder */ > + pSHGetKnownFolderPath(&FOLDERID_Desktop, 0, NULL, &desktop_path); > + lstrcatW(testfile_path, desktop_path); > + myPathAddBackslashW(testfile_path); > + lstrcatW(testfile_path, testfileW); > + file = CreateFileW(testfile_path, GENERIC_WRITE, 0, NULL, CREATE_NEW, 0, NULL); > + ok(file != INVALID_HANDLE_VALUE, "CreateFileW failed! Last error: 0x%08x.\n", GetLastError()); > + CloseHandle(file); > + > + shellitem = (void*)0xdeadbeef; > + ret = pSHCreateItemFromRelativeName(shellitem_desktop, testfileW, NULL, &IID_IShellItem, > + (void**)&shellitem); > + ok(ret == S_OK, "SHCreateItemFromRelativeName failed: 0x%08x.\n", ret); > + ok(shellitem != NULL, "shellitem was %p.\n", shellitem); > + if(SUCCEEDED(ret)) > + { > + ret = IShellItem_GetDisplayName(shellitem, 0, &displayname); > + ok(ret == S_OK, "IShellItem_GetDisplayName failed: 0x%08x.\n", ret); > + ok(!lstrcmpW(displayname, testfileW), "got wrong display name: %s.\n", wine_dbgstr_w(displayname)); > + > + shellitem2 = (void*)0xdeadbeef; > + ret = pSHCreateItemFromRelativeName(shellitem_desktop, testfileW, NULL, &IID_IShellItem, > + (void**)&shellitem2); > + ok(ret == S_OK, "SHCreateItemFromRelativeName failed: 0x%08x.\n", 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); > + > + shellitem2 = (void*)0xdeadbeef; > + ret = IShellFolder_ParseDisplayName(desktopfolder, NULL, NULL, testfileW, NULL, > + &pidl_desktop_testfile, NULL); > + ok(ret == S_OK, "ParseDisplayName failed 0x%08x.\n", ret); > + ret = pSHCreateItemFromIDList(pidl_desktop_testfile, &IID_IShellItem, (void**)&shellitem2); > + ret = IShellItem_Compare(shellitem, shellitem2, 0, &order); > + ok(ret == S_OK, "IShellItem_Compare fail: 0x%08x.\n", ret); > + ok(!order, "order got wrong value: %d.\n", order); > + pILFree(pidl_desktop_testfile); > + IShellItem_Release(shellitem2); > + > + IShellItem_Release(shellitem); > + } > + > + DeleteFileW(testfile_path); > + CoTaskMemFree(desktop_path); > + CoTaskMemFree(displayname); dipslayname is uninitialized if the 'if' block isn't executed. Either move this inside the 'if' block, or just get rid of the 'if' condition. We expect the condition to succeed in all cases anyway. Huw.

2 1

Re: [2/3 v4] shell32: Implement SHCreateItemInKnownFolder.
by Huw Davies 14 Aug '17

14 Aug '17

On Sat, Aug 12, 2017 at 11:42:43PM +0800, Jactry Zeng wrote: > Superseded patch 136380. > > ChangLog: > v4: > - Using SHCreateItemFromRelativeName() for implementing > SHCreateItemInKnownFolder(); > - Fix a logic error in tests. > v3: > - Load SHGetKnownFolderPath() dynamically. > v2: > - Using SHGetKnownFolderIDList() instead of SHGetKnownFolderPath() and > IShellFolder_ParseDisplayName(); > - Add more tests. > > Signed-off-by: Jactry Zeng <jzen g(a)codeweavers.com> > --- > dlls/shell32/shell32.spec | 1 + > dlls/shell32/shellitem.c | 35 +++++++++++++ > dlls/shell32/tests/shlfolder.c | 111 > +++++++++++++++++++++++++++++++++++++++++ > include/shobjidl.idl | 1 + > 4 files changed, 148 insertions(+) > This and patch 3/3 look good to me, so just fix up 1/3 and resend the series. Huw.

1 0

Re: [PATCH] mfplat: Add MFTEnumEx stub.
by Alistair Leslie-Hughes 14 Aug '17

14 Aug '17

Hi Andrey, On 14/08/17 19:59, Andrey Gusev wrote: > Signed-off-by: Andrey Gusev <andrey.goosev(a)gmail.com> > --- > + * MFTEnumEx (mfplat.@) > + */ > +HRESULT WINAPI MFTEnumEx(GUID category, UINT32 flags, const MFT_REGISTER_TYPE_INFO *input_type, > + const MFT_REGISTER_TYPE_INFO *output_type, IMFActivate ***activate, > + UINT32 *pcount) > +{ > + FIXME("(%s, %x, %p, %p, %p, %p): stub\n", debugstr_guid(&category), flags, input_type, > + output_type, activate, pcount); > + return S_OK; > +} If you must return S_OK, please set pcount to 0. (maybe rename pcount to count at the same time). Alistair.

1 0

Re: [PATCH] wined3d: Reduce timeout value to avoid GL_TIMEOUT_EXPIRED on macOS
by Henri Verbeet 12 Aug '17

12 Aug '17

On 12 August 2017 at 11:06, <nneonneo(a)gmail.com> wrote: > - /* Apple seems to be into arbitrary limits, and timeouts larger than > - * 0xfffffffffffffbff immediately return GL_TIMEOUT_EXPIRED. We don't > - * really care and can live with waiting a few μs less. (OS X 10.7.4). */ > + /* Timeouts near 0xffffffffffffffff may immediately return GL_TIMEOUT_EXPIRED, > + * possibly because macOS internally adds some slop to the timer. To avoid this, > + * we use a large number that isn't near the point of overflow (macOS 10.12.5). > + */ > GLenum gl_ret = GL_EXTCALL(glClientWaitSync(fence->object.sync, > - GL_SYNC_FLUSH_COMMANDS_BIT, ~(GLuint64)0xffff)); > + GL_SYNC_FLUSH_COMMANDS_BIT, (GLuint64)((1ULL << 62ULL) - 0xffff))); Adjusting the workaround is probably fine, but ULL literals are not portable. Does something along the lines of "~(GLuint64)0 >> 1" work as well? I don't think we care about the low bits, but note that the patch as it is doesn't actually preserve them.

2 2

Re: [PATCH resend 1/2] krnl386.exe16: Use toupper() instead of RtlUpperChar().
by Alexandre Julliard 12 Aug '17

12 Aug '17

Zebediah Figura <z.figura12(a)gmail.com> writes: > Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> > --- > dlls/krnl386.exe16/ne_module.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) I don't think that's a good idea. Why do you need that? -- Alexandre Julliard julliard(a)winehq.org

2 1

Re: [PATCH v3 2/5] ieproxy: Add stub DLL.
by Jacek Caban 11 Aug '17

11 Aug '17

On 10.08.2017 20:52, Zebediah Figura wrote: > --- a/loader/wine.inf.in > +++ b/loader/wine.inf.in > @@ -2559,6 +2559,7 @@ HKLM,%CurrentVersion%\Telephony\Country List\998,"SameAreaRule",,"G" > 11,,msxml6.dll > 11,,shdocvw.dll > 16422,Internet Explorer,iexplore.exe > +16422,Internet Explorer,ieproxy.dll > 16422,Windows Media Player,wmplayer.exe > 16422,Windows NT\Accessories,wordpad.exe > 11,,* > @@ -2613,6 +2614,7 @@ HKLM,%CurrentVersion%\Telephony\Country List\998,"SameAreaRule",,"G" > 16410,Microsoft, > 16412,Microsoft, > 16422,Internet Explorer,iexplore.exe > +16422,Internet Explorer,ieproxy.dll Are you sure about it? It's in regular system32 (and syswow64) on my Win10. Jacek

2 2

Re: [PATCH v5 1/5] include: Add initial ieautomation.idl
by Jacek Caban 11 Aug '17

11 Aug '17

On 11.08.2017 16:10, Zebediah Figura wrote: > +cpp_quote("#define INTERNETEXPLORERCONFIGURATION_HOST 0x00000001") This should be a value of INTERNETEXPLORERCONFIGURATION enum. Jacek

1 0

Re: [PATCH v4 1/5] include: Add initial ieautomation.idl
by Jacek Caban 11 Aug '17

11 Aug '17

On 11.08.2017 15:54, Zebediah Figura wrote: > + */ > + > +import "unknwn.idl"; One more thing I didn't notice earlier: this should be ocidl.idl. Jacek

1 0

Re: [v3 5/5] kernel32/tests: Add tests for LoadLibrary with LIBRARY_SEARCH_* flags
by Carlos Palminha 11 Aug '17

11 Aug '17

Hi, My patch with test is producing a strange result in Test Bot. I'm getting a ERROR_NOT_ENOUGH_MEMORY when calling LoadLibraryExA only in Vista and Win2008 64bit. Any hint? Regards, C.Palminha On 11-08-2017 11:39, Carlos Palminha wrote: > Test LIBRARY_SEARCH_* flags when loading libraries. > Uses helper from path.c to create the test dll file. > > Signed-off-by: Carlos Palminha <palminha(a)synopsys.com> > --- > v3: > - Improved check for Win2003 cases > > v2: > - Split the tests affecting different files > - Tests checks for Win2003 cases > > dlls/kernel32/tests/module.c | 132 ++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 131 insertions(+), 1 deletion(-) > > diff --git a/dlls/kernel32/tests/module.c b/dlls/kernel32/tests/module.c > index bb22c24316c..96dd3277c1b 100644 > --- a/dlls/kernel32/tests/module.c > +++ b/dlls/kernel32/tests/module.c > @@ -407,6 +407,136 @@ static void testLoadLibraryEx(void) > FreeLibrary(hmodule); > } > > +/* use some functions and strcutures from loader.c */ > +extern DWORD create_test_dll( const IMAGE_DOS_HEADER *dos_header, UINT dos_size, > + const IMAGE_NT_HEADERS *nt_header, const char *dll_name ); > +extern const IMAGE_NT_HEADERS nt_header_template; > + > +static void testLoadLibraryExSearchFlags(void) > +{ > + HMODULE hmodule; > + char tmpPath[MAX_PATH]; > + char dllName[] = "testFlags.dll"; > + char tmpFullFile[MAX_PATH]; > + char sysFullFile[MAX_PATH]; > + IMAGE_DOS_HEADER dos_header; > + IMAGE_NT_HEADERS nt_header; > + ULONG file_size; > + char fullPath[MAX_PATH]; > + DWORD version; > + BOOL is_win2003 = FALSE; > + > + GetTempPathA(MAX_PATH, tmpPath); > + lstrcpyA(tmpFullFile, tmpPath); > + lstrcatA(tmpFullFile, dllName); > + > + /* check for required function */ > + if (!pSetDllDirectoryA) > + { > + win_skip("SetDllDirectoryA not available\n"); > + return; > + } > + > + /* check if we are in win2003... flags are not supported */ > + version = GetVersion(); > + if ((LOBYTE(LOWORD(version)) == 5) && (HIBYTE(LOWORD(version))==2)) > + is_win2003 = TRUE; > + > + /* try to load test dll from test path before creating it*/ > + SetLastError(0xdeadbeef); > + pSetDllDirectoryA(tmpPath); > + hmodule = LoadLibraryExA(dllName, NULL, LOAD_LIBRARY_SEARCH_USER_DIRS); > + ok (hmodule == NULL, "Test library %s should not be loaded from %s\n", dllName, tmpPath); > + ok( GetLastError() == ERROR_MOD_NOT_FOUND || GetLastError() == ERROR_DLL_NOT_FOUND || GetLastError() == ERROR_INVALID_PARAMETER, > + "Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got %d\n", GetLastError()); > + FreeLibrary(hmodule); > + pSetDllDirectoryA(NULL); > + > + /* create test library */ > + lstrcpyA(fullPath, tmpPath); > + lstrcatA(fullPath, dllName); > + > + dos_header.e_magic = IMAGE_DOS_SIGNATURE; > + dos_header.e_lfanew = sizeof(dos_header); > + > + nt_header = nt_header_template; > + > + nt_header.FileHeader.NumberOfSections = 1; > + nt_header.FileHeader.SizeOfOptionalHeader = sizeof(IMAGE_OPTIONAL_HEADER); > + nt_header.OptionalHeader.SectionAlignment = 0x1000; > + nt_header.OptionalHeader.FileAlignment = 0x1000; > + nt_header.OptionalHeader.SizeOfImage = sizeof(dos_header) + sizeof(nt_header_template) + sizeof(IMAGE_SECTION_HEADER) + 0x1000; > + nt_header.OptionalHeader.SizeOfHeaders = sizeof(dos_header) + sizeof(nt_header_template) + sizeof(IMAGE_SECTION_HEADER); > + > + file_size = create_test_dll(&dos_header, sizeof(dos_header), &nt_header, fullPath); > + ok( file_size, "could not create '%s'\n",fullPath); > + > + /* try to load test dll without specifying the test path */ > + SetLastError(0xdeadbeef); > + pSetDllDirectoryA(NULL); > + hmodule = LoadLibraryExA(dllName, NULL, LOAD_LIBRARY_SEARCH_USER_DIRS); > + ok (hmodule == NULL, "Test library should not be loaded from 'NULL' path\n"); > + ok( GetLastError() == ERROR_MOD_NOT_FOUND || GetLastError() == ERROR_DLL_NOT_FOUND || GetLastError() == ERROR_INVALID_PARAMETER, > + "Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got %d\n", GetLastError()); > + FreeLibrary(hmodule); > + > + /* try to load test dll from test path */ > + SetLastError(0xdeadbeef); > + pSetDllDirectoryA(tmpPath); > + hmodule = LoadLibraryExA(dllName, NULL, LOAD_LIBRARY_SEARCH_USER_DIRS); > + if(is_win2003) > + ok (hmodule == NULL, "Test library should not be loaded from %s in Win2003 (flag not supported)\n", tmpPath); > + else > + ok (hmodule != NULL, "Could not load test library %s from %s\n", dllName, tmpPath); > + ok(GetLastError() == 0xdeadbeef || GetLastError() == ERROR_SUCCESS || GetLastError() == ERROR_INVALID_PARAMETER, > + "Expected 0xdeadbeef or ERROR_SUCCESS or ERROR_INVALID_PARAMETER (win2003), got %d\n", GetLastError()); > + FreeLibrary(hmodule); > + pSetDllDirectoryA(NULL); > + > + /* load non-existing test dll from system32 */ > + SetLastError(0xdeadbeef); > + hmodule = LoadLibraryExA(dllName, NULL, LOAD_LIBRARY_SEARCH_SYSTEM32); > + ok(hmodule == NULL, "%s should not be loaded\n", dllName); > + ok( GetLastError() == ERROR_MOD_NOT_FOUND || GetLastError() == ERROR_DLL_NOT_FOUND || GetLastError() == ERROR_INVALID_PARAMETER, > + "Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got %d\n", GetLastError()); > + FreeLibrary(hmodule); > + > + /* move library from temp path to system32 and delete from old path */ > + GetWindowsDirectoryA(sysFullFile,MAX_PATH); > + lstrcatA(sysFullFile,"\\system32\\"); > + lstrcatA(sysFullFile, dllName); > + MoveFileA(tmpFullFile,sysFullFile); > + DeleteFileA(tmpFullFile); > + > + /* try to load dll using more than one flag */ > + pSetDllDirectoryA(tmpPath); > + SetLastError(0xdeadbeef); > + hmodule = LoadLibraryExA(dllName, NULL, LOAD_LIBRARY_SEARCH_USER_DIRS | LOAD_LIBRARY_SEARCH_SYSTEM32); > + if(is_win2003) > + ok (hmodule == NULL, "Test library should not be loaded from 'system32' in Win2003 (flag not supported)\n"); > + else > + ok (hmodule != NULL, "Library %s was expected to be loaded from 'system32'\n", dllName); > + ok(GetLastError() == 0xdeadbeef || GetLastError() == ERROR_SUCCESS || GetLastError() == ERROR_INVALID_PARAMETER, > + "Expected 0xdeadbeef or ERROR_SUCCESS or ERROR_INVALID_PARAMETER (win2003), got %d\n", GetLastError()); > + FreeLibrary(hmodule); > + pSetDllDirectoryA(NULL); > + > + /* try to load test dll from system32 */ > + SetLastError(0xdeadbeef); > + hmodule = LoadLibraryExA(dllName, NULL, LOAD_LIBRARY_SEARCH_SYSTEM32); > + if(is_win2003) > + ok (hmodule == NULL, "Test library should not be loaded from 'system32' in Win2003 (flag not supported)\n"); > + else > + ok (hmodule != NULL, "Library %s was expected to be loaded from 'system32'\n", dllName); > + ok(GetLastError() == 0xdeadbeef || GetLastError() == ERROR_SUCCESS || GetLastError() == ERROR_INVALID_PARAMETER, > + "Expected 0xdeadbeef or ERROR_SUCCESS or ERROR_INVALID_PARAMETER (win2003), got %d\n", GetLastError()); > + FreeLibrary(hmodule); > + > + /* delete test dll from system32 directory */ > + DeleteFileA(sysFullFile); > + > +} > + > static void testGetDllDirectory(void) > { > CHAR bufferA[MAX_PATH]; > @@ -744,7 +874,6 @@ START_TEST(module) > WCHAR filenameW[MAX_PATH]; > > /* Test if we can use GetModuleFileNameW */ > - > SetLastError(0xdeadbeef); > GetModuleFileNameW(NULL, filenameW, MAX_PATH); > if (GetLastError() == ERROR_CALL_NOT_IMPLEMENTED) > @@ -766,6 +895,7 @@ START_TEST(module) > testLoadLibraryA_Wrong(); > testGetProcAddress_Wrong(); > testLoadLibraryEx(); > + testLoadLibraryExSearchFlags(); > testGetModuleHandleEx(); > testK32GetModuleInformation(); > } >

1 0

Re: [PATCH v3 1/5] include: Add initial ieautomation.idl
by Jacek Caban 11 Aug '17

11 Aug '17

On 10.08.2017 20:52, Zebediah Figura wrote: > + > +cpp_quote("DEFINE_GUID(CLSID_InternetExplorerManager, 0xdf4fcc34, 0x067a, 0x4e0a, 0x83,0x52, 0x4a,0x1a,0x50,0x95,0x34,0x6e);") Please use coclass, like in PSDK, instead. Jacek

1 0

Re: [v3 5/5] kernel32/tests: Add tests for LoadLibrary with LIBRARY_SEARCH_* flags
by Marvin 11 Aug '17

11 Aug '17

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=32633 Your paranoid android. === wvistau64 (32 bit module) === module.c:479: Test failed: Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got 8 === w2008s64 (32 bit module) === module.c:479: Test failed: Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got 8 === wvistau64 (64 bit module) === module.c:479: Test failed: Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got 8 === w2008s64 (64 bit module) === module.c:479: Test failed: Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got 8

1 0

Re: [PATCH] comctl32/listview: Keep the active item selected after hwndEdit is destroyed
by Nikolay Sivov 11 Aug '17

11 Aug '17

Could you describe how should I be testing this?

2 1

Re: [PATCH v2 3/5] ieframe: Add stub implementation of IInternetExplorerManager.
by Jacek Caban 10 Aug '17

10 Aug '17

Hi Zebediah, On 08.08.2017 20:04, Zebediah Figura wrote: > HRESULT register_class_object(BOOL do_reg) > { > HRESULT hres; > > - static DWORD cookie; > + static DWORD ie_cookie, iem_cookie; > > if(do_reg) { > hres = CoRegisterClassObject(&CLSID_InternetExplorer, > (IUnknown*)&InternetExplorerFactory, CLSCTX_SERVER, > - REGCLS_MULTIPLEUSE|REGCLS_SUSPENDED, &cookie); > - if (FAILED(hres)) { > + REGCLS_MULTIPLEUSE, &ie_cookie); > + if (FAILED(hres)) > + ERR("failed to register object %08x\n", hres); > + hres = CoRegisterClassObject(&CLSID_InternetExplorerManager, > + (IUnknown*)&InternetExplorerManagerFactory, CLSCTX_SERVER, > + REGCLS_MULTIPLEUSE, &iem_cookie); > + if (FAILED(hres)) > ERR("failed to register object %08x\n", hres); > - return hres; > - } > - > - hres = CoResumeClassObjects(); > - if(SUCCEEDED(hres)) > - return hres; > - > - ERR("failed to resume object %08x\n", hres); > } > - > - return CoRevokeClassObject(cookie); > + else > + { > + hres = CoRevokeClassObject(ie_cookie); > + if (FAILED(hres)) > + ERR("failed to register object %08x\n", hres); > + hres = CoRevokeClassObject(iem_cookie); > + if (FAILED(hres)) > + ERR("failed to register object %08x\n", hres); > + } > + return hres; > } Are you sure that's how it's supposed to work? I didn't test it myself, but given how it's registered and documented, I'd expect that CLSID_InternetExplorerManager should live in a different process from CLSID_InternetExplorer. It should be easy to distinguish those processes with -startmanager command line argument. > +static HRESULT WINAPI InternetExplorerManager_CreateObject(IInternetExplorerManager *iface, DWORD config, LPCWSTR url, REFIID riid, void **ppv) > +{ > + FIXME("(%p)->(0x%x, %s, %s, %p) semi-stub\n", iface, config, debugstr_w(url), debugstr_guid(riid), ppv); > + > + return CoCreateInstance(&CLSID_InternetExplorer, NULL, CLSCTX_LOCAL_SERVER, riid, ppv); > +} Using CoCreateInstance is very questionable here (eg. due to the comment above). Also, a patch adding a stub should just return E_NOTIMPL. Implementation deserves a separated patch. I'd expect that it should just use internal ieframe functions to create a new IE instance. Also, I'd suggest to reorder patches to something like: 2, 4, 5, 1 (likely replaced by support for -startmanager command line argument), 3 and then a new patch implementing CreateObject. Thanks, Jacek

2 6

Re: [v3 PATCH 1/2] dwrite: Protect cached fontface list when accessed from multiple threads
by Sebastian Lackner 10 Aug '17

10 Aug '17

On 10.08.2017 08:45, Nikolay Sivov wrote: > Based on patch by Anton Romanov <theli.ua(a)gmail.com> > > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > --- > dlls/dwrite/dwrite_private.h | 6 ++++-- > dlls/dwrite/font.c | 24 ++++++++++++--------- > dlls/dwrite/main.c | 50 +++++++++++++++++++++++++++++++++----------- > 3 files changed, 56 insertions(+), 24 deletions(-) > Isn't it also necessary to protect factory_cache_fontface()? By the way: An alternative method to implement proper synchronization (which does not require locking critical sections before InterlockedDecrement) would be to do something like: In the destructor: --- snip --- if (!ref) { EnterCriticalSection(...); list_remove(...); LeaveCriticalSection(...); ... } --- snip --- And when querying cached font faces: --- snip --- EnterCriticalSection(...); LIST_FOR_EACH_ENTRY(...) { ... /* the following code could also be hidden in QueryInterface */ if (InterlockedIncrement( &obj->refcount ) == 1) { InterlockedDecrement( &obj->refcount ); continue; } /* return it */ } LeaveCriticalSection(...); --- snip --- This is also what is done in Wines threadpool implementation [1], and has the advantage that the CS is not needed for every Release, only when the object is really destroyed. But in the end it is a matter of taste, for rpcrt4 Jacek also picked the "hold CS during InterlockedDecrement" method. [1] https://source.winehq.org/git/wine.git/blob/HEAD:/dlls/ntdll/threadpool.c#l… Best regards, Sebastian

3 2

Re: [PATCH] gdiplus: Support GdipSetClipRegion in metafiles
by Vincent Povirk 10 Aug '17

10 Aug '17

+ stat = GdipGetRegionData(region, (BYTE*)object_record + FIELD_OFFSET(EmfPlusObject, Header.Size), size, NULL); + object_record->Header.DataSize = object_record->Header.Size; + object_record->Header.Size = record_size; This is a little too clever. I'd rather have a solution similar to write_path_data.

1 0

Re: [v4 PATCH 1/2] dwrite: Protect cached fontface list when accessed from multiple threads
by Sebastian Lackner 10 Aug '17

10 Aug '17

On 10.08.2017 15:41, Nikolay Sivov wrote: > @@ -861,6 +877,10 @@ HRESULT factory_get_cached_fontface(IDWriteFactory5 *iface, IDWriteFontFile * co > const void *cached_key; > IDWriteFontFile *file; > > + if (IDWriteFontFace4_AddRef(cached->fontface) == 1) > + continue; > + IDWriteFontFace4_Release(cached->fontface); This will not work as expected. The refcount also matters when the object is about to be destroyed. There could be multiple threads running factory_get_cached_fontface() before _Release() can finally remove the fontface from the list. Please note that always calling _Release() also wouldn't be correct because it would run the destructor twice. It is not really necessary to access the private structs from other parts of dwrite, but you should either: * change AddRef to use something like interlocked_dec_if_nonzero (see ntdll/sync.c) * implement the check in QueryInterface (manually run InterlockedDecrement to decrement the refcount without going through the destructor) There are probably also other ways, for example using a flag to make sure the destructor is not called twice. Best regards, Sebastian

1 0

Re: [v2 PATCH 2/2] dwrite: Added a helper to check for supported characters
by Dmitry Timoshkov 10 Aug '17

10 Aug '17

Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > - TRACE("(%p)->(0x%08x)\n", This, ch); > + TRACE("(%p)->(0x%#x)\n", This, ch); This will cause printing of the 0x prefix twice. Same applies to other places. -- Dmitry.

1 0

Re: wineandroid: Add suffix to lib directory and wine binary on 64-bit
by Alexandre Julliard 10 Aug '17

10 Aug '17

André Hentschel <nerv(a)dawncrow.de> writes: > This fixes at least arm64-v8a builds, where you have lib64/libwine.so and bin/wine64 > > Signed-off-by: André Hentschel <nerv(a)dawncrow.de> > --- > > This and "winebuild: Improved position independent code generation" enable Wine on ARM64 Android phones (e.g. most Android devices). > The check for 64 in the CPU_ABI can be discussed, but I think it's reliable and fast. > > dlls/wineandroid.drv/WineActivity.java | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) We already prefix with the ABI, so I think plain "lib" is better. -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: [PATCH v2 2/5] include: Add initial ieautomation.idl
by Nikolay Sivov 09 Aug '17

09 Aug '17

On 08.08.2017 21:04, Zebediah Figura wrote: > +[ > + object, > + uuid(acc84351-04ff-44f9-b23f-655ed168c6d5), > + oleautomation > +] > +interface IInternetExplorerManager : IUnknown > +{ > + HRESULT CreateObject([in] DWORD config, [in, unique] LPCWSTR url, [in] REFIID riid, [out, iid_is(riid)] void **ppv); > +} This is not how it's defined in SDK - oleautomation is not used there and "url" argument has different attributes.

3 2

Re: [PATCH v2 5/5] ieproxy: Add proxy/stub for IInternetExplorerManager.
by Alistair Leslie-Hughes 09 Aug '17

09 Aug '17

Hi Zebediah, On 09/08/17 04:04, Zebediah Figura wrote: > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> > interface IInternetExplorerManager : IUnknown > { > - HRESULT CreateObject([in] DWORD config, [in, unique] LPCWSTR url, [in] REFIID riid, [out, iid_is(riid)] void **ppv); > + [local] HRESULT CreateObject([in] DWORD config, [in, unique] LPCWSTR url, [in] REFIID riid, [out, iid_is(riid)] void **ppv); > + [call_as(CreateObject)] HRESULT RemoteCreateObject([in] DWORD config, [in, unique] LPCWSTR url, [in] REFIID riid, [out, iid_is(riid)] void **ppv); The public API doesn't have the RemoteCreateObject function. Best Regards Alistair.

2 1

Re: [PATCH 3/5] setupapi: Fix compiler warnings when converting between pointers and integers on 64-bit systems
by Alexandre Julliard 09 Aug '17

09 Aug '17

Hugh McMaster <hugh.mcmaster(a)outlook.com> writes: > Signed-off-by: Hugh McMaster <hugh.mcmaster(a)outlook.com> > --- > dlls/setupapi/devinst.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) That's only hiding the bug. -- Alexandre Julliard julliard(a)winehq.org

4 3

Re: [PATCH v2 5/5] ieproxy: Add proxy/stub for IInternetExplorerManager.
by Alistair Leslie-Hughes 09 Aug '17

09 Aug '17

Hi Zebediah, On 09/08/17 04:04, Zebediah Figura wrote: > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> > interface IInternetExplorerManager : IUnknown > { > - HRESULT CreateObject([in] DWORD config, [in, unique] LPCWSTR url, [in] REFIID riid, [out, iid_is(riid)] void **ppv); > + [local] HRESULT CreateObject([in] DWORD config, [in, unique] LPCWSTR url, [in] REFIID riid, [out, iid_is(riid)] void **ppv); > + [call_as(CreateObject)] HRESULT RemoteCreateObject([in] DWORD config, [in, unique] LPCWSTR url, [in] REFIID riid, [out, iid_is(riid)] void **ppv); The public API doesn't have the RemoteCreateObject function. Best Regards Alistair.

1 0

Re: [PATCH] kernel32: Uncomment Win7 stubs in specfile.
by Sebastian Lackner 08 Aug '17

08 Aug '17

On 08.08.2017 21:38, Zebediah Figura wrote: > These were originally commented since the default Windows version > was still XP. As I see you want to increase your regression count. ;) Whats the point of advertising all those functions when Wine has no implementation yet?

3 3

Re: [PATCH 5/5] ieproxy: Add proxy/stub for IInternetExplorerManager.
by Marvin 08 Aug '17

08 Aug '17

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=32620 Your paranoid android. === build (build) === Make in include dir failed

1 0

Re: shell32: Wait for process to finish when using flag SEE_MASK_NOASYNC
by Huw Davies 08 Aug '17

08 Aug '17

On 8 Aug 2017, at 16:50, Carlos Palminha <CARLOS.PALMINHA(a)synopsys.com> wrote: > > Support for SEE_MASK_NOASYNC (same value as SEE_MASK_FLAG_DDEWAIT). > Waits for process execution to finish before returning. > > Signed-off-by: Carlos Palminha <palminha(a)synopsys.com> > --- > dlls/shell32/shlexec.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/dlls/shell32/shlexec.c b/dlls/shell32/shlexec.c > index c0ef53ab8e7..fe3eeeb5743 100644 > --- a/dlls/shell32/shlexec.c > +++ b/dlls/shell32/shlexec.c > @@ -344,6 +344,11 @@ static UINT_PTR SHELL_ExecuteW(const WCHAR *lpCmd, WCHAR *env, BOOL shWait, > dwCreationFlags = CREATE_UNICODE_ENVIRONMENT; > if (!(psei->fMask & SEE_MASK_NO_CONSOLE)) > dwCreationFlags |= CREATE_NEW_CONSOLE; > + > + if(psei->fMask & SEE_MASK_NOASYNC) { > + dwCreationFlags |= SEE_MASK_NOASYNC; > + } > + > if (CreateProcessW(NULL, (LPWSTR)lpCmd, NULL, NULL, FALSE, dwCreationFlags, env, > lpDirectory, &startup, &info)); Passing SEE_MASK_NOASYNC to CreateProcessW doesn’t look right… Huw.

2 2

Invitations to WineConf
by Jeremy White 08 Aug '17

08 Aug '17

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

6 6

Re: [v2 5/5] kernel32/tests: Add tests for LoadLibrary with LIBRARY_SEARCH_* flags
by Marvin 08 Aug '17

08 Aug '17

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=32619 Your paranoid android. === w2003std (32 bit module) === module.c:471: Test failed: Could not load test library testFlags.dll from C:\DOCUME~1\ADMINI~1\LOCALS~1\Temp\ module.c:496: Test failed: Library testFlags.dll was expected to be loaded from 'system32' module.c:505: Test failed: Library testFlags.dll was expected to be loaded from 'system32'

1 0

Re: [PATCH] netapi32: Return ERROR_SUCCESS in DsGetDcNameA/W.
by Huw Davies 08 Aug '17

08 Aug '17

On Fri, Aug 04, 2017 at 04:17:14PM +0300, Andrey Gusev wrote: > Signed-off-by: Andrey Gusev <andrey.goosev(a)gmail.com> > --- > dlls/netapi32/netapi32.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/dlls/netapi32/netapi32.c b/dlls/netapi32/netapi32.c > index 278d4528b0..0236a2ad53 100644 > --- a/dlls/netapi32/netapi32.c > +++ b/dlls/netapi32/netapi32.c > @@ -3024,7 +3024,7 @@ DWORD WINAPI DsGetDcNameW(LPCWSTR ComputerName, LPCWSTR AvoidDCName, > FIXME("(%s, %s, %s, %s, %08x, %p): stub\n", debugstr_w(ComputerName), > debugstr_w(AvoidDCName), debugstr_guid(DomainGuid), > debugstr_w(SiteName), Flags, DomainControllerInfo); > - return ERROR_CALL_NOT_IMPLEMENTED; > + return ERROR_SUCCESS; > } > > DWORD WINAPI DsGetDcNameA(LPCSTR ComputerName, LPCSTR AvoidDCName, > @@ -3034,7 +3034,7 @@ DWORD WINAPI DsGetDcNameA(LPCSTR ComputerName, LPCSTR AvoidDCName, > FIXME("(%s, %s, %s, %s, %08x, %p): stub\n", debugstr_a(ComputerName), > debugstr_a(AvoidDCName), debugstr_guid(DomainGuid), > debugstr_a(SiteName), Flags, DomainControllerInfo); > - return ERROR_CALL_NOT_IMPLEMENTED; > + return ERROR_SUCCESS; > } This can't be right. These functions are supposed to return a ptr to an allocated DOMAIN_CONTROLLER_INFO in the final parameter. If you return ERROR_SUCCESS, the app will likely dereference that uninitialized pointer. Huw.

1 0

Re: [PATCH v3] shell32: Implement SHCreateItemInKnownFolder.
by Alistair Leslie-Hughes 08 Aug '17

08 Aug '17

Hi Jactry, On 08/08/17 17:04, Jactry Zeng wrote: > + /* SHCreateItemInKnownFolder */ > + if(pSHCreateItemInKnownFolder || pSHGetKnownFolderPath) If one of these is NULL then the test will crash. Best Regards Alistair

2 1

Re: [PATCH v3] shell32: Implement SHCreateItemInKnownFolder.
by Huw Davies 08 Aug '17

08 Aug '17

On Tue, Aug 08, 2017 at 03:04:21PM +0800, Jactry Zeng wrote: > diff --git a/dlls/shell32/shellitem.c b/dlls/shell32/shellitem.c > index 1e19f7c72e..0bfb4a7f6f 100644 > --- a/dlls/shell32/shellitem.c > +++ b/dlls/shell32/shellitem.c > @@ -662,6 +662,67 @@ HRESULT WINAPI SHCreateItemFromIDList(PCIDLIST_ABSOLUTE pidl, REFIID riid, void > return ret; > } > > +HRESULT WINAPI SHCreateItemInKnownFolder(REFKNOWNFOLDERID rfid, DWORD flags, > + LPWSTR filename, REFIID riid, void **ppv) > +{ > + HRESULT hr; > + WCHAR *pathW = NULL; > + IShellFolder *desktop = NULL, *folder = NULL; > + LPITEMIDLIST pidl_folder = NULL, pidl = NULL; > + > + TRACE("%p, %x, %s, %s, %p\n", rfid, flags, wine_dbgstr_w(filename), > + debugstr_guid(riid), ppv); > + > + if(!rfid || !ppv) > + return E_INVALIDARG; > + > + if(flags) > + FIXME("flags 0x%08x not supported\n", flags); > + > + *ppv = NULL; > + hr = SHGetKnownFolderIDList(rfid, 0, NULL, &pidl_folder); flags should just be passed as the second parameter here and then you can ditch the FIXME(). > + if(hr != S_OK) > + goto cleanup; > + > + hr = SHGetDesktopFolder(&desktop); > + if(hr != S_OK) > + goto cleanup; > + > + if(!IsEqualIID(rfid, &FOLDERID_Desktop)) > + { > + hr = IShellFolder_BindToObject(desktop, pidl_folder, NULL, &IID_IShellFolder, > + (void**)&folder); > + if(hr != S_OK) > + goto cleanup; > + } > + > + if(filename) > + { > + hr = IShellFolder_ParseDisplayName(folder ? folder : desktop, NULL, NULL, > + filename, NULL, &pidl, NULL); > + if(hr != S_OK) > + goto cleanup; > + hr = SHCreateItemFromIDList(pidl, riid, ppv); > + if(hr != S_OK) > + goto cleanup; > + } > + else > + hr = SHCreateItemFromIDList(pidl_folder, riid, ppv); Thinking about this some more, how about implementing SHCreateItemFromRelativeName() first? Then this function becomes something like: SHGetKnownFolderIDList( rfid, flags, NULL, &pidl ); SHCreateItemFromIDList( pidl, &IID_IShellItem, &parent ); if (name) SHCreateItemFromRelativeName( parent, name, NULL, iid, ppv ); else IShellItem_QueryInterface( parent, iid, ppv ); IShellItem_Release( parent ); ILFree( pidl ); > diff --git a/dlls/shell32/tests/shlfolder.c b/dlls/shell32/tests/shlfolder.c > index 9e7127dfea..461f8cea95 100644 > --- a/dlls/shell32/tests/shlfolder.c > +++ b/dlls/shell32/tests/shlfolder.c > > + /* SHCreateItemInKnownFolder */ > + if(pSHCreateItemInKnownFolder || pSHGetKnownFolderPath) I think you mean && here (although in practice it doesn't matter). > + 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); > + } I'm not a big fan of this sort of thing, just leave it out. Huw.

1 0

Re: [PATCH v2] shell32: Implement SHCreateItemInKnownFolder.
by Huw Davies 08 Aug '17

08 Aug '17

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.

2 1

Re: [PATCH v2] ntdll: Fix wrong string size alloc when loading libraries
by Nikolay Sivov 07 Aug '17

07 Aug '17

On 07.08.2017 20:41, Carlos Palminha wrote: > Fixed some nasty unhandled exceptions when loading libraries > that don't have .dll extension. > > Problem was caused by wrong string size calculation. > > v2: Now properly takes into account strlen * sizeof WCHAR > > Signed-off-by: Carlos Palminha <palminha(a)synopsys.com> > --- > dlls/ntdll/loader.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c > index dd1f74c0fc..83dd6d378e 100644 > --- a/dlls/ntdll/loader.c > +++ b/dlls/ntdll/loader.c > @@ -2141,7 +2141,7 @@ static NTSTATUS find_dll_file( const WCHAR *load_path, const WCHAR *libname, > if (!(ext = strrchrW( libname, '.')) || strchrW( ext, '/' ) || strchrW( ext, '\\')) > { > if (!(dllname = RtlAllocateHeap( GetProcessHeap(), 0, > - (strlenW(libname) * sizeof(WCHAR)) + sizeof(dllW) ))) > + (strlenW(libname) * sizeof(WCHAR)) + (strlenW(dllW) * sizeof(WCHAR)) ))) > return STATUS_NO_MEMORY; > strcpyW( dllname, libname ); > strcatW( dllname, dllW ); > This is still shorter than it is now: sizeof(dllW) is 5 * sizeof(WCHAR) vs strlenW(dllW) * sizeof(WCHAR) == 4 * sizeof(WCHAR).

2 1

Re: [PATCH] ws2_32: Don't return synchronously if the connection is refused.
by Huw Davies 07 Aug '17

07 Aug '17

On Mon, Aug 07, 2017 at 03:39:14PM +0100, Huw Davies wrote: > Signed-off-by: Huw Davies <huw(a)codeweavers.com> > --- > dlls/ws2_32/socket.c | 5 +++++ > dlls/ws2_32/tests/sock.c | 20 ++++++-------------- > 2 files changed, 11 insertions(+), 14 deletions(-) Please ignore this for now, it needs more work. Huw.

1 0

Re: [PATCH] ntdll: Fix wrong string size alloc when loading libraries
by Józef Kucia 07 Aug '17

07 Aug '17

On Mon, Aug 7, 2017 at 6:48 PM, Carlos Palminha <CARLOS.PALMINHA(a)synopsys.com> wrote: > Fixed some nasty unhandled exceptions when loading libraries > that don't have .dll extension. > > Problem was caused by wrong string size calculation. > > Signed-off-by: Carlos Palminha <palminha(a)synopsys.com> > --- > dlls/ntdll/loader.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c > index dd1f74c0fc..8fc464bcd7 100644 > --- a/dlls/ntdll/loader.c > +++ b/dlls/ntdll/loader.c > @@ -2141,7 +2141,7 @@ static NTSTATUS find_dll_file( const WCHAR *load_path, const WCHAR *libname, > if (!(ext = strrchrW( libname, '.')) || strchrW( ext, '/' ) || strchrW( ext, '\\')) > { > if (!(dllname = RtlAllocateHeap( GetProcessHeap(), 0, > - (strlenW(libname) * sizeof(WCHAR)) + sizeof(dllW) ))) > + (strlenW(libname) * sizeof(WCHAR)) + strlenW(dllW) ))) This doesn't look right. After the change it allocates less memory than before. Notice that strlenW(dllW) == 4 and sizeof(dllW) == 10. > return STATUS_NO_MEMORY; > strcpyW( dllname, libname ); > strcatW( dllname, dllW ); > -- > 2.11.0 > > >

2 1

Re: [PATCH 2/2] wined3d: do not call queue_is_empty in wined3d_cs_mt_finish.
by Henri Verbeet 07 Aug '17

07 Aug '17

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.

1 0

Re: winebuild: Improved position independent code generation
by Martin Storsjö 06 Aug '17

06 Aug '17

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

1 0

Re: [PATCH] msvcrt: Fix strcpy implementation so it works on overlapping buffers
by Ken Thomases 05 Aug '17

05 Aug '17

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

2 1

Re: [4/6] wsdapi: Implement "Hello" message creation support. [try 3]
by Vincent Povirk 04 Aug '17

04 Aug '17

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

2 2

Re: shell32: Implement SHCreateItemInKnownFolder.
by Huw Davies 04 Aug '17

04 Aug '17

On Sun, Jul 30, 2017 at 05:37:02PM +0800, Jactry Zeng wrote: > diff --git a/dlls/shell32/shellitem.c b/dlls/shell32/shellitem.c > index 1e19f7c72e..57fb849f16 100644 > --- a/dlls/shell32/shellitem.c > +++ b/dlls/shell32/shellitem.c > @@ -662,6 +662,66 @@ HRESULT WINAPI SHCreateItemFromIDList(PCIDLIST_ABSOLUTE pidl, REFIID riid, void > return ret; > } > > +HRESULT WINAPI SHCreateItemInKnownFolder(REFKNOWNFOLDERID rfid, DWORD flags, > + LPWSTR filename, REFIID riid, void **ppv) > +{ > + HRESULT hr; > + WCHAR *pathW = NULL; > + IShellFolder *desktop = NULL, *folder = NULL; > + LPITEMIDLIST pidl_folder = NULL, pidl = NULL; > + > + TRACE("%p, %x, %s, %s, %p\n", rfid, flags, wine_dbgstr_w(filename), > + debugstr_guid(riid), ppv); > + > + if(!rfid || !ppv) > + return E_INVALIDARG; > + > + if(flags) > + FIXME("flags 0x%08x not supported\n", flags); > + > + *ppv = NULL; > + hr = SHGetDesktopFolder(&desktop); > + if(hr != S_OK) return hr; > + > + hr = SHGetKnownFolderPath(rfid, 0, NULL, &pathW); > + if(hr != S_OK) return hr; > + > + hr = IShellFolder_ParseDisplayName(desktop, NULL, NULL, pathW, NULL, &pidl_folder, NULL); > + if(hr != S_OK) > + goto cleanup; You probably want to use SHGetKnownFolderIDList() instead of these last two calls. Huw.

2 3

Re: [4/4] kernel32/tests: Add tests for LoadLibrary with LIBRARY_SEARCH_* flags
by Marvin 03 Aug '17

03 Aug '17

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

1 0

Re: [PATCH 2/2] winebrowser: Open the default browser if called without an argument.
by Alexandre Julliard 03 Aug '17

03 Aug '17

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

3 5

Re: [1/2] msvcrt/tests: Add tests for mbstowcs and wcstombs with empty strings.
by Piotr Caban 03 Aug '17

03 Aug '17

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

1 0

Re: ntdll: Support receiving server reply data to memory with write watch.
by Alexandre Julliard 03 Aug '17

03 Aug '17

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

2 2

Re: [PATCH v3] ole32: Store proxy/stub CLSIDs per process, not per apartment.
by Huw Davies 03 Aug '17

03 Aug '17

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.

1 0

Re: [4/6] wsdapi: Implement "Hello" message creation support. [try 2]
by Vincent Povirk 02 Aug '17

02 Aug '17

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

2 1

Re: [2/6] wsdapi: Add code to create SOAP envelope from header and body elements. [try 2]
by Vincent Povirk 02 Aug '17

02 Aug '17

+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.

2 1

Re: [PATCH v2] ole32: Store proxy/stub CLSIDs per process, not per apartment.
by Huw Davies 02 Aug '17

02 Aug '17

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.

2 1

Re: [PATCHv2] msvcrt: Export -f suffixed float functions functions on arm64
by Piotr Caban 02 Aug '17

02 Aug '17

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

2 1

Re: [PATCH 2/3] msvcrt: Set errno through the _matherr function.
by Piotr Caban 02 Aug '17

02 Aug '17

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

2 2

Re: [5/6] wsdapi: Add support for sending messages via UDP multicast.
by Vincent Povirk 01 Aug '17

01 Aug '17

+ delay = rand() % (UDP_MAX_DELAY - UDP_MIN_DELAY + 1) + UDP_MIN_DELAY; Using rand() for this doesn't seem very random, and it could interfere with an application's use of rand(). + /* Get size of buffer for adapters */ + adapterAddresses = (IP_ADAPTER_ADDRESSES *) HeapAlloc(heap, 0, bufferSize); Was this comment meant to be earlier? Sending messages to every adapter in sequence, with a delay for each, doesn't seem right. +void terminate_networking(IWSDiscoveryPublisherImpl *impl) +{ + impl->publisherStarted = FALSE; + + WSACleanup(); +} It seems like this will call WSACleanup even when freeing an object that was never used.

2 1

Re: [PATCH] ole32: Store proxy/stub CLSIDs per process, not per apartment.
by Sebastian Lackner 01 Aug '17

01 Aug '17

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.

1 0

Re: [2/6] wsdapi: Add code to create SOAP envelope from header and body elements.
by Vincent Povirk 01 Aug '17

01 Aug '17

+ wideStringLength = lstrlenW(wideString); + sizeNeeded = WideCharToMultiByte(CP_UTF8, 0, wideString, wideStringLength, NULL, 0, NULL, NULL); + + if (sizeNeeded < 0) + { + return NULL; + } + + newString = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeNeeded + 1); + WideCharToMultiByte(CP_UTF8, 0, wideString, wideStringLength, newString, sizeNeeded, NULL, NULL); This would be much simpler if you passed in -1 as the wide string length. Using HEAP_ZERO_MEMORY to null-terminate the string is a bit weird. + else if (curNode->Type == TextType) + { + textNode = WSDAllocateLinkedMemory(parent, sizeof(WSDXML_TEXT)); + if (textNode == NULL) goto cleanup; + + textNode->Node.Next = NULL; + textNode->Node.Parent = parent; + textNode->Node.Type = TextType; + textNode->Text = duplicate_string(textNode, ((WSDXML_TEXT *)curNode)->Text); + if (textNode->Text == NULL) goto cleanup; + } I'm not sure, but I think you need to add textNode as a child of newElement? + /* SequenceID attribute */ + if (header->AppSequence->SequenceId != NULL) + { + if (!add_string_attribute(xmlContext, appSequenceElement, discoveryNsUri, sequenceIdString, header->AppSequence->SequenceId)) goto cleanup; + } Do we need the namespace for SequenceID?

2 3

Re: [PATCH 1/2] winebrowser: Try other browsers on non-zero exit code.
by Vincent Povirk 01 Aug '17

01 Aug '17

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.

2 1

Re: [PATCH 5/6 v3] ws2_32: Added support for overlapped GetAddrInfoExW.
by Sebastian Lackner 01 Aug '17

01 Aug '17

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.

1 0

Re: [PATCH 5/6] ws2_32: Added support for overlapped GetAddrInfoExW.
by Sebastian Lackner 01 Aug '17

01 Aug '17

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);

2 1

Re: [PATCH 6/6] ws2_32/tests: Added GetAddrInfoExW tests.
by Alexandre Julliard 01 Aug '17

01 Aug '17

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

2 1

Re: [PATCH v2 2/3] include: Add initial IDL for cmnquery.h
by Alexandre Julliard 01 Aug '17

01 Aug '17

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

1 0

Re: [PATCH v2 2/3] include: Add initial IDL for cmnquery.h
by Alistair Leslie-Hughes 01 Aug '17

01 Aug '17

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

1 0

Re: [PATCH v2 2/3] include: Add initial IDL for cmnquery.h
by Alistair Leslie-Hughes 01 Aug '17

01 Aug '17

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

1 0

Re: [PATCH] kernel32: Load library search in system32 directory.
by Carlos Palminha 01 Aug '17

01 Aug '17

On 31-07-2017 18:50, Alexandre Julliard wrote: > Carlos Palminha <CARLOS.PALMINHA(a)synopsys.com> writes: > >> On 31-07-2017 17:54, Alexandre Julliard wrote: >>> Carlos Palminha <CARLOS.PALMINHA(a)synopsys.com> writes: >>> >>>> Hi Alexandre, >>>> >>>> I tested with iTunes installation that was crashing when calling load_libraryin my system. After the patch it was ok. Probably not enough indicator that was working properly in all systems. >>>> >>>> I will send a second version calling GetWindowsDirectory instead of using %windows%. >>>> >>>> Regarding the other cases, when using the flag >>>> LOAD_LIBRARY_SEARCH_SYSTEM32, directories in the standard search path >>>> are not searched and the value cannot be combined with >>>> LOAD_WITH_ALTERED_SEARCH_PATH. The search path is reduced to >>>> windows\system32, right? Or probably i need to append >>>> %windows&\system32 to the existing path... >>> >>> If you only specify SEARCH_SYSTEM32 the path is reduced to that, but you >>> could also have other flags like SEARCH_APPLICATION_DIR, >>> SEARCH_DLL_LOAD_DIR, etc. You can't handle just one flag and ignore the >>> others. >>> >> >> All the other cases SEARCH_APPLICATION_DIR, SEARCH_DEFAULT_DIRS, SEARCH_DLL_LOAD_DIR and SEARCH_USER_DIRS are not yet supported (unsopported flags). >> So at this moment LOAD_LIBRARY_SEARCH_* are reduced to ALTERED_SEARCH_PATH or SEARCH_SYSTEM32. > > They are not supported now, but if you add one you have to add the > others, otherwise dlls are going to fail to load because you are > searching only system32. > 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? Regards, C.Palminha

2 3

Re: [PATCH 3/3] ucrtbase/tests: Add tests for math function errors.
by Marvin 01 Aug '17

01 Aug '17

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

1 0

Re: [PATCH] opengl32: Update OpenGL Registry files URLs.
by Austin English 01 Aug '17

01 Aug '17

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

1 0

Re: [4/6] wsdapi: Implement "Hello" message creation support.
by Vincent Povirk 31 Jul '17

31 Jul '17

- obj = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*obj)); + heap = GetProcessHeap(); + obj = HeapAlloc(heap, HEAP_ZERO_MEMORY, sizeof(*obj)); I really hope we don't have to start optimizing for fewer GetProcessHeap calls. + /* Calculate space needed, including colon and potential trailing space */ + memoryNeeded = sizeof(WCHAR) * (lstrlenW(cur->Element->LocalName) + lstrlenW(cur->Element->Space->PreferredPrefix) + 2); This doesn't account for the terminating NULL wsprintfW will add. + discoveredNamespaces = WSDAllocateLinkedMemory(NULL, sizeof(struct list)); + if (!discoveredNamespaces) goto cleanup; Why not allocate this on the stack?

2 1

Re: [PATCHv2] msvcrt: Export -f suffixed float functions functions on arm64
by Piotr Caban 31 Jul '17

31 Jul '17

Hi Why do you need it? Is there native arm64 version of the dlls (I've seen some announcements but is it already available)? Or is it only for winelib applications? Thanks, Piotr

2 1

Re: [PATCH] kernel32: Load library search in system32 directory.
by Alexandre Julliard 31 Jul '17

31 Jul '17

Carlos Palminha <CARLOS.PALMINHA(a)synopsys.com> writes: > Use %windows%\system32 directory to search for the DLL. > Standard search path is not used. > > Signed-off-by: Carlos Palminha <palminha(a)synopsys.com> > --- > dlls/kernel32/module.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) %windows% cannot possibly work, environment variables are not expanded, and even if they were, there's no such variable. How did you test this? Also it's possible to specify multiple flags, this needs to be integrated properly in the code that builds the search path. -- Alexandre Julliard julliard(a)winehq.org

2 3

[PATCH] kernel32: Load library search in system32 directory.
by Carlos Palminha 31 Jul '17

31 Jul '17

Use %windows%\system32 directory to search for the DLL. Standard search path is not used. Signed-off-by: Carlos Palminha <palminha(a)synopsys.com> --- dlls/kernel32/module.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/dlls/kernel32/module.c b/dlls/kernel32/module.c index badfe1d175..aff5097664 100644 --- a/dlls/kernel32/module.c +++ b/dlls/kernel32/module.c @@ -915,13 +915,19 @@ static HMODULE load_library( const UNICODE_STRING *libname, DWORD flags ) LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR | LOAD_LIBRARY_SEARCH_APPLICATION_DIR | LOAD_LIBRARY_SEARCH_USER_DIRS | - LOAD_LIBRARY_SEARCH_SYSTEM32 | LOAD_LIBRARY_SEARCH_DEFAULT_DIRS; if( flags & unsupported_flags) FIXME("unsupported flag(s) used (flags: 0x%08x)\n", flags); - load_path = MODULE_get_dll_load_path( flags & LOAD_WITH_ALTERED_SEARCH_PATH ? libname->Buffer : NULL ); + if (flags & LOAD_LIBRARY_SEARCH_SYSTEM32) { + WCHAR spath[] = {'%','w','i','n','d','o','w','s','%','\\','s','y','s','t','e','m','3','2',0}; + int slen = (strlenW(spath) + 1) * sizeof(WCHAR); + load_path = HeapAlloc(GetProcessHeap(), 0, slen); + memcpy (load_path, spath, slen); + } else { + load_path = MODULE_get_dll_load_path( flags & LOAD_WITH_ALTERED_SEARCH_PATH ? libname->Buffer : NULL ); + } if (flags & LOAD_LIBRARY_AS_DATAFILE) { -- 2.11.0

1 0

Re: [PATCH] wined3d: Don't enable GL_EXT_geometry_shader4 on OpenGL >3.1
by Henri Verbeet 31 Jul '17

31 Jul '17

On 31 July 2017 at 08:13, Zach Mertes <techiezach(a)gmail.com> wrote: > GL_EXT_geometry_shader4's features are intrinsically enabled on OpenGL > 3.1 and newer. > Enabling it past 3.1 causes an annoying GLSL warning. > Against what version of Wine is this written? While it's correct that we don't (ever) want to use EXT_geometry_shader4, Wine has never used that extension. We initially used ARB_geometry_shader4 and later dropped it in favour of the 3.2 core profile.

1 0

UIRibbon
by Fabian Maurer 30 Jul '17

30 Jul '17

Hi Wine-Devel, I'm currently working on a patchset to get UIRibbon to work, or at least to not crash the program. I noticed that the ribbon xml files need to be compiled with a program called UICC.exe from the windows SDK, the resulting resources have some proprietary binary format. If we want to load and display a ribbon, we'll have to understand it. Does anyone know about information on that, or is the only way to reverse engineer it? Regards, Fabian Maurer

2 2

Re: [PATCH v4 2/3] uiribbon: Add interface definitions
by Alistair Leslie-Hughes 30 Jul '17

30 Jul '17

Hi Fabian, Besides the below issues, the patches look good. On 30/07/17 09:29, Fabian Maurer wrote: > Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> > +[ > + object, > + uuid(D428903C-729A-491d-910D-682A08FF2522), Please make the uuid lowercase ... > +[ > + object, > + uuid(F4F0385D-6872-43a8-AD09-4C339CB3F5C5), > + local, > + pointer_default(unique) Please make the uuid lowercase Also, could you please get rid of the LPVOID in the third patch. Best Regards Alistair Leslie-Hughes

1 0

Re: [PATCH v3 2/3] uiribbon: Add interface definitions
by Alistair Leslie-Hughes 29 Jul '17

29 Jul '17

Hi Fabian, On 30/07/17 06:36, Fabian Maurer wrote: > Code taken and adapted from d3dxof. > > Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> > --- > dlls/uiribbon/Makefile.in | 2 + > dlls/uiribbon/uiribbon_classes.idl | 21 +++++ These changes don't make any sense for this patch. Move them to the third patch in which the interfaces are registered. Besides this, everything looks ok. Best Regards Alistair

1 0

Re: [PATCH 1/3] uiribbon: Add DLL
by Alistair Leslie-Hughes 28 Jul '17

28 Jul '17

Hi Fabian, This is about the series of uiribbon. >+BOOL WINAPI DllMain(HINSTANCE hInstDLL, DWORD fdwReason, LPVOID lpv) Since nothing is implemented in this dll, please return check for DLL_WINE_PREATTACH. (example dlls/authz/authz.c). In this patch you could also add the private functions. patch 2. > + uiribbon.idl \ ... >interface IUIFramework : IUnknown >+{ >+ >+} >+ If your going to add a idl file, please fill in methods available for each interface. Please also avoid the forward declare for IUIApplication include it in the idl as well. >+ >+HRESULT UIRibbonFrameworkImpl_Create(IUnknown* pUnkOuter, LPVOID* ppObj)l; Move this function to after +static const IUIFrameworkVtbl IUIFramework_Vtbl = then you wont need the forward declare. I would have the following patches. 1. Add new dll (DLLMain and private functions) 2. Add idl to include (Including all the interfaces required) 3. Add the stubbed interface. 4. Tests? (maybe not possible at this stage). Best Regards Alistair Leslie-Hughes

2 3

[PATCH v7 00/26] x86: Enable User-Mode Instruction Prevention
by Ricardo Neri 28 Jul '17

28 Jul '17

This is v7 of this series. The six previous submissions can be found here [1], here [2], here[3], here[4], here[5] and here[6]. This version addresses the comments received in v6 plus improvements of the handling of exceptions unrelated to UMIP as well as corner cases in virtual-8086 mode. Please see details in the change log. === What is UMIP? User-Mode Instruction Prevention (UMIP) is a security feature present in new Intel Processors. If enabled, it prevents the execution of certain instructions if the Current Privilege Level (CPL) is greater than 0. If these instructions were executed while in CPL > 0, user space applications could have access to system-wide settings such as the global and local descriptor tables, the segment selectors to the current task state and the local descriptor table. Hiding these system resources reduces the tools available to craft privilege escalation attacks such as [7]. These are the instructions covered by UMIP: * SGDT - Store Global Descriptor Table * SIDT - Store Interrupt Descriptor Table * SLDT - Store Local Descriptor Table * SMSW - Store Machine Status Word * STR - Store Task Register If any of these instructions is executed with CPL > 0, a general protection exception is issued when UMIP is enabled. === How does it impact applications? We want to have UMIP enabled by default. However, UMIP will change the behavior that certain applications expect from the operating system. For instance, programs running on WineHQ and DOSEMU2 rely on some of these instructions to function. Stas Sergeev found that Microsoft Windows 3.1 and dos4gw use the instruction SMSW when running in virtual-8086 mode[8]. SGDT and SIDT can also run on virtual-8086 mode. In order to not change the behavior of the system. This patchset emulates the SGDT, SIDT and SMSW. This should be sufficient to not break the applications mentioned above. Regarding the two remaining instructions, STR and SLDT, the WineHQ team has shown interest catching the general protection fault and use it as a vehicle to fix broken applications[9]. Furthermore, STR and SLDT can only run in protected and long modes. DOSEMU2 emulates virtual-8086 mode via KVM. No applications will be broken unless DOSEMU2 decides to enable the CR4.UMIP bit in platforms that support it. Also, this should not pose a security risk as no system resouces would be revealed. Instead, code running inside the KVM would only see the KVM's GDT, IDT and MSW. Please note that UMIP is always enabled for both 64-bit and 32-bit Linux builds. However, emulation of the UMIP-protected instructions is not done for 64-bit processes. 64-bit user space applications will receive the SIGSEGV signal when UMIP instructions causes a general protection fault. === How are UMIP-protected instructions emulated? This version keeps UMIP enabled at all times and by default. If a general protection fault caused by the instructions protected by UMIP is detected, such fault will be fixed-up by returning dummy values as follows: * SGDT and SIDT return hard-coded dummy values as the base of the global descriptor and interrupt descriptor tables. These hard-coded values correspond to memory addresses that are near the end of the kernel memory map. This is also the case for virtual-8086 mode tasks. In all my experiments in x86_32, the base of GDT and IDT was always a 4-byte address, even for 16-bit operands. Thus, my emulation code does the same. In all cases, the limit of the table is set to 0. * SMSW returns the value with which the CR0 register is programmed in head_32/64.S at boot time. This is, the following bits are enabled: CR0.0 for Protection Enable, CR.1 for Monitor Coprocessor, CR.4 for Extension Type, which will always be 1 in recent processors with UMIP; CR.5 for Numeric Error, CR0.16 for Write Protect, CR0.18 for Alignment Mask. As per the Intel 64 and IA-32 Architectures Software Developer's Manual, SMSW returns a 16-bit results for memory operands. However, when the operand is a register, the results can be up to CR0[63:0]. Since the emulation code only kicks-in in x86_32, we return up to CR[31:0]. * The proposed emulation code is handles faults that happens in both protected and virtual-8086 mode. * Again, STR and SLDT are not emulated. === How is this series laid out? ++ Preparatory work As per suggestions from Andy Lutormirsky and Borislav Petkov, I moved the x86 page fault error codes to a header. Also, I made user_64bit_mode available to x86_32 builds. This helps to reuse code and reduce the number of #ifdef's in these patches. ++ Fix bugs in MPX address evaluator I found very useful the code for Intel MPX (Memory Protection Extensions) used to parse opcodes and the memory locations contained in the general purpose registers when used as operands. I put some of this code in a separate library file that both MPX and UMIP can access and avoid code duplication. Before creating the new library, I fixed a couple of bugs that I found in how MPX determines the address contained in the instruction and operands. ++ Provide a new x86 instruction evaluating library With bugs fixed, the MPX evaluating code is relocated in a new insn-eval.c library. The basic functionality of this library is extended to obtain the segment descriptor selected by either segment override prefixes or the default segment by the involved registers in the calculation of the effective address. It was also extended to obtain the default address and operand sizes as well as the segment base address. Also, support to process 16-bit address encodings. Armed with this arsenal, it is now possible to determine the linear address onto which the emulated results shall be copied. This code supports Normal 32-bit and 64-bit (i.e., __USER32_CS and/or __USER_CS) protected mode, virtual-8086 mode, 16-bit protected mode with 32-bit base address. ++ Emulate UMIP instructions A new fixup_umip_exception functions inspect the instruction at the instruction pointer. If it is an UMIP-protected instruction, it executes the emulation code. This uses all the address-computing code of the previous section. ++ Add self-tests Lastly, self-tests are added to entry_from_v86.c to exercise the most typical use cases of UMIP-protected instructions in a virtual-8086 mode. ++ Extensive tests Extensive tests were performed to test all the combinations of ModRM, SiB and displacements for 16-bit and 32-bit encodings for the ss, ds, es, fs and gs segments. Tests also include a 64-bit program that uses segmentation via fs and gs. For this purpose, I temporarily enabled UMIP support for 64-bit process. This change is not part of this patchset. The intention is to test the computations of linear addresses in 64-bit mode, including the extra R8-R15 registers. Extensive test is also implemented for virtual-8086 tasks. Code of these tests can be found here [10] and here [11]. ++ Merging this series? Am I any close to see these patches merged? :) [1]. https://lwn.net/Articles/705877/ [2]. https://lkml.org/lkml/2016/12/23/265 [3]. https://lkml.org/lkml/2017/1/25/622 [4]. https://lkml.org/lkml/2017/2/23/40 [5]. https://lkml.org/lkml/2017/3/3/678 [6]. https://lkml.org/lkml/2017/3/7/866 [7]. http://timetobleed.com/a-closer-look-at-a-recent-privilege-escalation-bug-i… [8]. https://www.winehq.org/pipermail/wine-devel/2017-April/117159.html [10]. https://github.com/01org/luv-yocto/tree/rneri/umip/meta-luv/recipes-core/um… [11]. https://github.com/01org/luv-yocto/commit/a72a7fe7d68693c0f4100ad86de6ecabd… Thanks and BR, Ricardo Changes since V6: *Reworded and addded more details on the special cases of ModRM and SIB bytes. To avoid confusion, I ommited mentioning the involved registers (EBP and ESP). *Replaced BUG() with printk_ratelimited in function get_reg_offset of insn-eval.c *Removed unused utility functions that obtain a register value from pt_regs given a SIB base and index. *Clarified nomenclature to call CS, DS, ES, FS, GS and SS segment registers and their values segment selectors. *Reworked function resolve_seg_register to issue an error when more than one segment overrides prefixes are used in the instruction. *Added logic in resolve_seg_register to ignore segment register when in long mode and not using FS or GS. *Added logic to ensure the effective address is within the limits of the segment in protected mode. *Added logic to ensure segment override prefixes are ignored when resolving the segment of EIP and EDI with string instructions. *Added code to make user_64bit_mode() available in CONFIG_X86_32... and make it return false, of course. *Merged the two functions that obtain the default address and operand size of a code segment into one as they are always used together. *Corrected logic of displacement-only addressing in long mode to make the displacement relative to the RIP of the next instruction. *Reworked logic to sign-extend 32-bit memory offsets into 64-bit signed memory offsets. This include more checks and putting all together in an utility function. *Removed the 'unlikely' of conditional statements as we are not in a critical path. *In virtual-8086 mode, ensure that effective addresses are always less than 0x10000, even when address override prefixes are used. Also, ensure that linear addresses have a size of 20-bits. Changes since V5: * Relocate the page fault error code enumerations to traps.h Changes since V4: * Audited patches to use braces in all the branches of conditional. statements, except those in which the conditional action only takes one line. * Implemented support in 64-builds for both 32-bit and 64-bit tasks in the instruction evaluating library. * Split segment selector function in the instruction evaluating library into two functions to resolve the segment type by instruction override or default and a separate function to actually read the segment selector. * Fixed a bug when evaluating 32-bit effective addresses with 64-bit kernels. * Split patches further for for easier review. * Use signed variables for computation of effective address. * Fixed issue with a spurious static modifier in function insn_get_addr_ref found by kbuild test bot. * Removed comparison between true and fixup_umip_exception. * Reworked check logic when identifying erroneous vs invalid values of the SiB base and index. Changes since V3: * Limited emulation to 32-bit and 16-bit modes. For 64-bit mode, a general protection fault is still issued when UMIP-protected instructions are executed with CPL > 0. * Expanded instruction-evaluating code to obtain segment descriptor along with their attributes such as base address and default address and operand sizes. Also, support for 16-bit encodings in protected mode was implemented. * When getting a segment descriptor, this include support to obtain those of a local descriptor table. * Now the instruction-evaluating code returns -EDOM when the value of registers should not be used in calculating the effective address. The value -EINVAL is left for errors. * Incorporate the value of the segment base address in the computation of linear addresses. * Renamed new instruction evaluation library from insn-kernel.c to insn-eval.c * Exported functions insn_get_reg_offset_* to obtain the register offset by ModRM r/m, SiB base and SiB index. * Improved documentation of functions. * Split patches further for easier review. Changes since V2: * Added new utility functions to decode the memory addresses contained in registers when the 16-bit addressing encodings are used. This includes code to obtain and compute memory addresses using segment selectors for real-mode address translation. * Added support to emulate UMIP-protected instructions for virtual-8086 tasks. * Added self-tests for virtual-8086 mode that contains representative use cases: address represented as a displacement, address in registers and registers as operands. * Instead of maintaining a static variable for the dummy base addresses of the IDT and GDT, a hard-coded value is used. * The emulated SMSW instructions now return the value with which the CR0 register is programmed in head_32/64.S This is: PE | MP | ET | NE | WP | AM. For x86_64, PG is also enabled. * The new file arch/x86/lib/insn-utils.c is now renamed as arch/x86/lib/ insn-kernel.c. It also has its own header. This helps keep in sync the the kernel and objtool instruction decoders. Also, the new insn-kernel.c contains utility functions that are only relevant in a kernel context. * Removed printed warnings for errors that occur when decoding instructions with invalid operands. * Added more comments on fixes in the instruction-decoding MPX functions. * Now user_64bit_mode(regs) is used instead of test_thread_flag(TIF_IA32) to determine if the task is 32-bit or 64-bit. * Found and fixed a bug in insn-decoder in which X86_MODRM_RM was incorrectly used to obtain the mod part of the ModRM byte. * Added more explanatory code in emulation and instruction decoding code. This includes a comment regarding that copy_from_user could fail if there exists a memory protection key in place. * Tested code with CONFIG_X86_DECODER_SELFTEST=y and everything passes now. * Prefixed get_reg_offset_rm with insn_ as this function is exposed via a header file. For clarity, this function was added in a separate patch. Changes since V1: * Virtual-8086 mode tasks are not treated in a special manner. All code for this purpose was removed. * Instead of attempting to disable UMIP during a context switch or when entering virtual-8086 mode, UMIP remains enabled all the time. General protection faults that occur are fixed-up by returning dummy values as detailed above. * Removed umip= kernel parameter in favor of using clearcpuid=514 to disable UMIP. * Removed selftests designed to detect the absence of SIGSEGV signals when running in virtual-8086 mode. * Reused code from MPX to decode instructions operands. For this purpose code was put in a common location. * Fixed two bugs in MPX code that decodes operands. Ricardo Neri (26): ptrace,x86: Make user_64bit_mode() available to 32-bit builds x86/mm: Relocate page fault error codes to traps.h x86/mpx: Use signed variables to compute effective addresses x86/mpx: Do not use SIB.index if its value is 100b and ModRM.mod is not 11b x86/mpx: Do not use SIB.base if its value is 101b and ModRM.mod = 0 x86/mpx, x86/insn: Relocate insn util functions to a new insn-eval file x86/insn-eval: Do not BUG on invalid register type x86/insn-eval: Add a utility function to get register offsets x86/insn-eval: Add utility function to identify string instructions x86/insn-eval: Add utility functions to get segment selector x86/insn-eval: Add utility function to get segment descriptor x86/insn-eval: Add utility functions to get segment descriptor base address and limit x86/insn-eval: Add function to get default params of code segment x86/insn-eval: Indicate a 32-bit displacement if ModRM.mod is 0 and ModRM.rm is 5 x86/insn-eval: Incorporate segment base and limit in linear address computation x86/insn-eval: Support both signed 32-bit and 64-bit effective addresses x86/insn-eval: Handle 32-bit address encodings in virtual-8086 mode x86/insn-eval: Add support to resolve 16-bit addressing encodings x86/insn-eval: Add wrapper function for 16-bit and 32-bit address encodings x86/cpufeature: Add User-Mode Instruction Prevention definitions x86: Add emulation code for UMIP instructions x86/umip: Force a page fault when unable to copy emulated result to user x86/traps: Fixup general protection faults caused by UMIP x86: Enable User-Mode Instruction Prevention selftests/x86: Add tests for User-Mode Instruction Prevention selftests/x86: Add tests for instruction str and sldt arch/x86/Kconfig | 10 + arch/x86/include/asm/cpufeatures.h | 1 + arch/x86/include/asm/disabled-features.h | 8 +- arch/x86/include/asm/insn-eval.h | 25 + arch/x86/include/asm/ptrace.h | 6 +- arch/x86/include/asm/traps.h | 18 + arch/x86/include/asm/umip.h | 15 + arch/x86/include/uapi/asm/processor-flags.h | 2 + arch/x86/kernel/Makefile | 1 + arch/x86/kernel/cpu/common.c | 16 +- arch/x86/kernel/traps.c | 4 + arch/x86/kernel/umip.c | 286 +++++++ arch/x86/lib/Makefile | 2 +- arch/x86/lib/insn-eval.c | 1066 +++++++++++++++++++++++++ arch/x86/mm/fault.c | 88 +- arch/x86/mm/mpx.c | 120 +-- tools/testing/selftests/x86/entry_from_vm86.c | 89 ++- 17 files changed, 1580 insertions(+), 177 deletions(-) create mode 100644 arch/x86/include/asm/insn-eval.h create mode 100644 arch/x86/include/asm/umip.h create mode 100644 arch/x86/kernel/umip.c create mode 100644 arch/x86/lib/insn-eval.c -- 2.9.3

5 80

Re: [PATCH] kernel32: Add support for detecting volume information with libudev.
by Alexandre Julliard 28 Jul '17

28 Jul '17

Zebediah Figura <z.figura12(a)gmail.com> writes: > Using GetVolumeInformation may fail due to lack of permissions; e.g. > when the drive was mounted as root. This patch uses libudev, if it > is available, to retrieve the volume label, serial number, and type. > > Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> > --- > dlls/kernel32/Makefile.in | 3 +- > dlls/kernel32/volume.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 90 insertions(+), 1 deletion(-) libudev should be loaded dynamically, and that sort of thing should ideally be done in mountmgr. -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: [PATCH] winmm: Load MCI drivers from MCI32 key.
by Andrew Eikum 28 Jul '17

28 Jul '17

This looks good, but could you add some tests showing something that was wrong before but is now fixed? Andrew On Tue, Jul 25, 2017 at 09:18:15PM -0500, Zebediah Figura wrote: > MCI key/section is for 16-bit drivers, which can't be loaded by > 32-bit winmm. > > Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> > --- > dlls/winmm/mci.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/dlls/winmm/mci.c b/dlls/winmm/mci.c > index 56a537d..d770f6d 100644 > --- a/dlls/winmm/mci.c > +++ b/dlls/winmm/mci.c > @@ -66,10 +66,10 @@ WINE_DEFAULT_DEBUG_CHANNEL(mci); > #define MCI_MAGIC 0x0001 > > /* MCI settings */ > -static const WCHAR wszHklmMci [] = {'S','o','f','t','w','a','r','e','\\','M','i','c','r','o','s','o','f','t','\\','W','i','n','d','o','w','s',' ','N','T','\\','C','u','r','r','e','n','t','V','e','r','s','i','o','n','\\','M','C','I',0}; > +static const WCHAR wszHklmMci [] = {'S','o','f','t','w','a','r','e','\\','M','i','c','r','o','s','o','f','t','\\','W','i','n','d','o','w','s',' ','N','T','\\','C','u','r','r','e','n','t','V','e','r','s','i','o','n','\\','M','C','I','3','2',0}; > static const WCHAR wszNull [] = {0}; > static const WCHAR wszAll [] = {'A','L','L',0}; > -static const WCHAR wszMci [] = {'M','C','I',0}; > +static const WCHAR wszMci [] = {'M','C','I','3','2',0}; > static const WCHAR wszOpen [] = {'o','p','e','n',0}; > static const WCHAR wszSystemIni[] = {'s','y','s','t','e','m','.','i','n','i',0}; > > -- > 2.7.4 > > >

2 3

Re: [v2 PATCH] gdi32: Update metafile world transform immediately in winnt mode
by Alexandre Julliard 27 Jul '17

27 Jul '17

Andrew Eikum <aeikum(a)codeweavers.com> writes: > The Chrome browser print function implements their own handling for > EMR_MODIFYWORLDTRANSFORM which calls ModifyWorldTransform on the HDC > directly without ever calling PlayEnhMetaFileRecord. In Wine, this > transformation would get discarded when the callback function returned, > causing the page to be printed at the wrong scale. > > Tests show that the transform is updated immediately during > PlayEnhMetaFileRecord. In addition, a modified transform persists > between callbacks until PlayEnhMetaFileRecord is called on a relevant > type of callback, at which point the transform is reverted before > playing back the record. > > Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> > --- > > v2: Get rid of win9x checks in tests, and reformat test code. > > dlls/gdi32/enhmetafile.c | 41 ++++++- > dlls/gdi32/tests/metafile.c | 253 ++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 289 insertions(+), 5 deletions(-) This breaks the tests: ../../../tools/runtest -q -P wine -T ../../.. -M gdiplus.dll -p gdiplus_test.exe.so metafile && touch metafile.ok wine: Unhandled exception 0xc0000093 in thread a0 at address 0x7e8f10c5 (thread 00a0), starting debugger... Unhandled exception: floating point underflow in 32-bit code (0x7e8f10c7). Register dump: CS:0023 SS:002b DS:002b ES:002b FS:0063 GS:006b EIP:7e8f10c7 ESP:0033f74c EBP:0033f768 EFLAGS:00010202( R- -- I - - - ) EAX:00000001 EBX:7e99b000 ECX:7e99b000 EDX:0033f7a8 ESI:7e99b000 EDI:0033f7a8 Stack dump: 0x0033f74c: 3f000000 00000000 00000000 3f000000 0x0033f75c: 7e99b000 00140bd0 0033f7a8 0033f7d8 0x0033f76c: 7e92e9f0 0033f7a8 0033f7a8 00140bd0 0x0033f77c: 00000000 3f800000 00000000 002f003e 0x0033f78c: 3f800000 3f800000 00000000 00000000 0x0033f79c: 3f800000 00000000 00000000 3f000000 Backtrace: =>0 0x7e8f10c7 CombineTransform+0x87() in gdi32 (0x0033f768) 1 0x7e92e9f0 EMF_Update_MF_Xform+0x6f(hdc=<is not available>, info=<is not available>) [/home/julliard/wine/wine/dlls/gdi32/enhmetafile.c:545] in gdi32 (0x0033f7d8) 2 0x7e930a4e PlayEnhMetaFileRecord+0x191d(hdc=<is not available>, handletable=<is not available>, mr=<is not available>, handles=<is not available>) [/home/julliard/wine/wine/dlls/gdi32/enhmetafile.c:1276] in gdi32 (0x0033f848) 3 0x7eca4fc1 GdipPlayMetafileRecord+0x170(metafile=<is not available>, recordType=<is not available>, flags=<is not available>, dataSize=<is not available>, data=<is not available>) [/home/julliard/wine/wine/dlls/gdiplus/metafile.c:1428] in gdiplus (0x0033f8c8) 4 0x7ed3887f test_gditransform_cb+0x2e(metafile=0x13f420, record_type=EmfRecordTypeGdiComment, flags=0, dataSize=0xc, pStr=") [/home/julliard/wine/wine/dlls/gdiplus/tests/metafile.c:2278] in gdiplus_test (0x0033f928) 5 0x7ed3881c play_metafile_proc+0x25b(record_type=<couldn't compute location>, flags=<couldn't compute location>, dataSize=<couldn't compute location>, pStr=<couldn't compute location>, userdata=<couldn't compute location>) [/home/julliard/wine/wine/dlls/gdiplus/tests/metafile.c:264] in gdiplus_test (0x0033f988) 6 0x7eca3495 enum_metafile_proc+0x64(hDC=<is not available>, lpHTable=<is not available>, lpEMFR=<is not available>, nObj=<is not available>, lpData=<is not available>) [/home/julliard/wine/wine/dlls/gdiplus/metafile.c:1841] in gdiplus (0x0033f9e8) 7 0x7e931af8 EnumEnhMetaFile+0x497() in gdi32 (0x0033faf8) 8 0x7eca5e56 GdipEnumerateMetafileSrcRectDestPoints+0x505() in gdiplus (0x0033fba8) 9 0x7ed39ea4 play_metafile.constprop+0x43() in gdiplus_test (0x0033fc08) 10 0x7ed43f42 func_metafile+0x2971() [/home/julliard/wine/wine/dlls/gdiplus/tests/metafile.c:2369] in gdiplus_test (0x0033fd48) 11 0x7eced39d main+0x25c(argc=<is not available>, argv=<is not available>) [/home/julliard/wine/wine/dlls/gdiplus/tests/../../../include/wine/test.h:603] in gdiplus_test (0x0033fdf8) 12 0x7ed5947b __wine_spec_exe_entry+0x4a(peb=<couldn't compute location>) [/home/julliard/wine/wine/dlls/winecrt0/exe_entry.c:36] in gdiplus_test (0x0033fe38) 13 0x7b45e85c call_process_entry+0xb() in kernel32 (0x0033fe58) 14 0x7b45f799 start_process+0x68(entry=<couldn't compute location>) [/home/julliard/wine/wine/dlls/kernel32/process.c:1116] in kernel32 (0x0033fe98) 15 0x7bc7ea9c call_thread_func_wrapper+0xb() in ntdll (0x0033feb8) 16 0x7bc81a59 call_thread_func+0xa8(entry=0x7b45f730, arg=0x7ed59430, frame=0x33ffc8) [/home/julliard/wine/wine/dlls/ntdll/signal_i386.c:2912] in ntdll (0x0033ffa8) 17 0x7bc7ea7a call_thread_entry_point+0x11() in ntdll (0x0033ffc8) 18 0x7bc53d43 start_process+0x12(arg=0xff987f08) [/home/julliard/wine/wine/dlls/ntdll/loader.c:3083] in ntdll (0x0033ffe8) 19 0xf753b91d __x86.get_pc_thunk.di+0x2a() in libwine.so.1 (0x00000000) 20 0xf753ba80 wine_switch_to_stack+0x1f(func=0x7bc53d30, arg=0xff987f08, stack=0x340000) [/home/julliard/wine/wine/libs/wine/port.c:77] in libwine.so.1 (0xff987ed8) 21 0x7bc599fe LdrInitializeThunk+0x22d(kernel_start=<couldn't compute location>, unknown2=<couldn't compute location>, unknown3=<couldn't compute location>, unknown4=<couldn't compute location>) [/home/julliard/wine/wine/dlls/ntdll/loader.c:3144] in ntdll (0xff987f28) 22 0x7b4656ff __wine_kernel_init+0xa4e() [/home/julliard/wine/wine/dlls/kernel32/process.c:1310] in kernel32 (0xff988e18) 23 0x7bc5a853 __wine_process_init+0x152() [/home/julliard/wine/wine/dlls/ntdll/loader.c:3353] in ntdll (0xff988e88) 24 0xf7539b83 wine_init+0x2a2(argc=0x3, argv=0xff9893d4, error="", error_size=0x400) [/home/julliard/wine/wine/libs/wine/loader.c:974] in libwine.so.1 (0xff988ed8) 25 0x7c000b58 main+0x97(argc=<is not available>, argv=<is not available>) [/home/julliard/wine/wine/loader/main.c:258] in <wine-loader> (0xff989328) 26 0xf7343276 __libc_start_main+0xf5() in libc.so.6 (0x00000000) 0x7e8f10c7 CombineTransform+0x87 in gdi32: flds 0xfffffff0(%ebp) -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH] gdi32: Update metafile world transform immediately in winnt mode
by Dmitry Timoshkov 27 Jul '17

27 Jul '17

Andrew Eikum <aeikum(a)codeweavers.com> wrote: > +#define IS_WIN9X() (GetVersion() & 0x80000000) ... > + if (IS_WIN9X()) > + /* win9x doesn't support world transforms */ > + return; The tests are supposed to get rid of win9x checks, not add new ones. -- Dmitry.

2 1

Re: [PATCH] gdiplus: Add stub for GdipGraphicsSetAbort
by Dmitry Timoshkov 27 Jul '17

27 Jul '17

Andrew Eikum <aeikum(a)codeweavers.com> wrote: > +static void test_GdipGraphicsSetAbort(void) > +{ > + HDC hdc = GetDC(hwnd); > + GpStatus status; > + GpGraphics *graphics; > + > + if (!pGdipGraphicsSetAbort) > + { > + win_skip("GdipGraphicsSetAbort() is not supported.\n"); > + return; hdc is leaked. > + } > + > + status = GdipCreateFromHDC(hdc, &graphics); > + expect(Ok, status); > + > + status = pGdipGraphicsSetAbort(NULL, NULL); > + expect(InvalidParameter, status); > + > + status = pGdipGraphicsSetAbort(graphics, NULL); > + expect(Ok, status); > + > + ReleaseDC(hwnd, hdc); > +} graphics is leaked. Also, it would be useful to test not invalid input as well. -- Dmitry.

2 1

Re: [PATCH] wmvcore/tests: add tests for WMReader methods
by Alistair Leslie-Hughes 27 Jul '17

27 Jul '17

Hi Jefferson, Thanks for the updated patch. When applying your patch there lot of whitespace errors (mostly from the header). warning: squelched 3569 whitespace errors warning: 3574 lines add whitespace errors. After look at qedit/tests, include the wma directly as a resource. (unless someone else objects) >// MSDN says dwType is a QWORD, but it is actually a DWORD C++ style comments aren't allowed. > ok(dwType == WMT_TYPE_DWORD, "onStatus pValue not WMT_TYPE_QWORD (%d), but %d\n", (int)WMT_TYPE_DWORD, dwType); Again WMT_TYPE_QWORD is incorrect here, and remove the (int)WMT_TYPE_DWORD parameter. TestWMReaderCallback_AddRef/Release could just return 2/1 respectfully. The object doesn't need to be deleted nor is there any reference type checks. wmvcore.c: In function ‘test_wmreader_play_wma’: wmvcore.c:425:8: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] ok((int)(onStatus_event = CreateEventW(NULL, FALSE, FALSE, NULL)), "Could not create event\n"); ^ You dont delete the wma file. Best Regards Alistair Leslie-Hughes

1 0

Re: [PATCH 2/7] regedit: Use a helper function to allocate memory and check for a valid pointer
by Alexandre Julliard 27 Jul '17

27 Jul '17

Hugh McMaster <hugh.mcmaster(a)outlook.com> writes: > +static void *heap_alloc(size_t size) > +{ > + void *buf; > + > + if (!(buf = HeapAlloc(GetProcessHeap(), 0, size))) > + { > + ERR("Out of memory!\n"); > + exit(1); > + } > + > + return buf; > +} That's not the standard heap_alloc(), so a different name would be preferable. I'd suggest heap_xalloc by analogy with xmalloc. -- Alexandre Julliard julliard(a)winehq.org

2 2

Unicode normalization for Wine
by Artur Świgoń 26 Jul '17

26 Jul '17

Dear All, My name is Artur and I'm participating in Google Summer of Code 2017 for Wine. Under Nikolay's supervision, I'm working on implementation of Unicode normalization. I probably should have introduced myself some time ago to share results of my research and my ideas, but I also wanted to wait until I could illustrate my points with some code. I'm not sure whether everyone is familiar with Unicode normalization forms, so here's a short summary of concepts and terms I may use. For detailed information, please see [1]. - Characters that look the same on the screen may be encoded in a few different ways. Accented Latin letters may constitute a familiar example. The letter 'é' can be U+00E9 (LATIN SMALL LETTER E WITH ACUTE), but it also can be represented as the sequence U+0065 U+0301 (LATIN SMALL LETTER E, COMBINING ACUTE ACCENT). Many of 1114112 (if I remember the number correctly) Unicode characters are decomposable, i.e. they can also be represented by a different sequence of characters. - Text in normalization form NFD ('D' stands for 'decomposition') does not contain any decomposable characters, e.g. there is U+0065 U+0301 instead of U+00E9. The combining characters (such as the above-mentioned acute accent) have a well defined order, using the concept of combining classes. [2 §3.11] - Text in normalization form NFKD ('K' stands for 'compatibility') has the properties of NFD, plus characters that are typographical variants of other characters (e.g. superscripts, ligatures, etc.) are mapped to 'plain' characters. - Text that will be normalized into the form NFC and NFKC ('C' stands for 'composition') has to be fully decomposed first (NFD and NFKD, respectively) and starters (characters with a combining class zero) plus following non- starters (combining class greater than zero) can be composed into single characters provided that they exist in Unicode and are not present in [3]. I've developed a draft implementation of NFD & NFKD (mapping characters to decomposed sequences as well as reordering combining characters) and written a test program that uses official Unicode test data. [4] My goal was to provide support for codepoints greater than 0xFFFF, encoded in UTF-16 as surrogate pairs. More on that below. The attachment [gsoc-v2.patch] is a diff of my working branch against master. Most notable changes include: - Necessary changes to tools/make_unicode. - New function wine_decompose_string(). - Draft implementation of two Windows API functions: NormalizeString() and IsNormalizedString() (the latter temporarily returns TRUE). [5][6] Rationale and comments: - tools/make_unicode did not collect all necessary data (most notably, compatibility mappings), so a new data structure, @decomp_table_full has been added. Its unification with @decomp_table is yet to be resolved. - The procedure dump_decompose_table() in that script has been rewritten to accomodate all aspects described above and below. Two helper subroutines have been added. - However, the C code section embedded within the script has grown substantially, so it may be desirable to move it to a separate file. - Adding wine_decompose_string() makes sense since surrogate pairs behave like a string and the Unicode Canonical Ordering Algorithm (for combining characters) is an in-place bubble sort on the string, per Unicode specification. - While the code in libs/port/decompose.c (and embedded in tools/make_unicode) implements normalization, it currently serves the purpose of confirmation that the generated data is correct. It hasn't been yet optimized or cleaned up, because the organization of the data is open to discussion now, so the code may still change substantially. - It has been previously assumed that a decomposition mapping of any character is two characters long. This is certainly not true for NFKD, where a character like U+FDFA decomposes to a string of length 18. Therefore, the tables in libs/port/decompose.c contain pairs of offsets to null-terminated mappings in another table (decomp_data, first in that file), one entry for NFD and one for NFKD, instead of two-character mappings. Moreover, since many NFD and NFKD are identical, they have been placed in the same table, with identical mappings merged and fully expanded (i.e. any decomposable characters in the data have been replaced by fully decomposed sequences), to minimize runtime overhead. - The original idea of multi-stage tables (using the most significant bits of a codepoint to obtain an offset to another table etc.) has been preserved, although the tables are no longer combined into a single table. This is because some sub-tables can use a narrower data type, which saves space, especially when data for all Unicode codepoints is included. - Tables of combining classes have a similar structure. - Mappings for characters above 0xFFFF are encoded as UTF-16 (using surrogate pairs), but a single codepoint (UTF-32 if you like) is used for table indexing. Setting $utflim in make_unicode to 65536 is the simplest way to disable support for such characters, but supporting surrogate pairs should not affect any text-related Wine component in a negative way. - As composition depends directly on decomposition, it is the next thing to work on. The attachment [unorm_test.c] is my test program. It should build fine with mingw-w64 or winegcc without any specific compiler flags. It looks for [4] in its directory or accepts a path to that file as an argument. Option -d makes it run only decomposition tests, option -s limits the range to 0xFFFF. No tests should fail with the -s option and 80 of 187460 should fail without it due to incomplete support for surrogate pairs in the reordering part. For details of failed tests, please run with the -v option. Is there a place for such a program somewhere in the Wine source tree? I'd be more than happy to answer your questions, address your comments, and listen to your opinions regarding Unicode normalization. Regards, Artur Świgoń [1] http://www.unicode.org/reports/tr15/ [2] http://www.unicode.org/versions/Unicode10.0.0/ch03.pdf [3] http://www.unicode.org/Public/10.0.0/ucd/CompositionExclusions.txt [4] http://www.unicode.org/Public/10.0.0/ucd/NormalizationTest.txt [5] https://msdn.microsoft.com/en-us/library/windows/desktop/dd319093(v=vs.85).… [6] https://msdn.microsoft.com/en-us/library/windows/desktop/dd318671(v=vs.85).…

2 1

Unicode normalization for Wine - new patch
by Artur Świgoń 26 Jul '17

26 Jul '17

I'm not sure whether I've sent the updated patch in my previous email. This one is the one that passes the number of tests I described in my previous message. Sorry for confusion. Artur

1 0

Re: [PATCH] wmvcore/tests: add tests for WMReader methods
by Alistair Leslie-Hughes 26 Jul '17

26 Jul '17

Hi Jefferson, > > +typedef struct { > + IWMReaderCallback IWMReaderCallback_iface; > + LONG ref; > + HRESULT (*onStatusCallback)(WMT_STATUS, HRESULT, WMT_ATTR_DATATYPE, BYTE*, void*, void*); > + HRESULT (*onSampleCallback)(DWORD, QWORD, QWORD, DWORD, INSSBuffer*, void*, void*); > + void *pvCallbackContext; > +} TestWMReaderCallback; I'm still a little confused as to why you need this structure when the IWMReaderCallbackVtbl with a few globals would work just as well. > +static HRESULT onStatus_test_wmreader_play_wma( > + WMT_STATUS status, > + HRESULT hr, > + WMT_ATTR_DATATYPE dwType, > + BYTE *pValue, > + void *pvContext, > + void *pvCallbackContext) > +{ > + struct callback_context_test_wmreader_play_wma *pTestContext = > + (struct callback_context_test_wmreader_play_wma*)pvCallbackContext; > + DWORD *dwordValue; > + > + CHECK_EXPECT(onStatus); > + ok(status == onStatus_expected_statuses_test_wmreader_play_wma[pTestContext->onStatus_call_count], > + "onStatus called with unexpected status: %d\n", status); > + if (status == WMT_OPENED) { > + ok(pvContext == pTestContext->pvContextOpen, > + "onStatus was not passed pvContext: expected %p, actual %p\n", pTestContext->pvContextOpen, pvContext); > + } > + else /* status == WMT_STARTED, or later */ { > + ok(pvContext == pTestContext->pvContextStart, > + "onStatus was not passed pvContext: expected %p, actual %p\n", pTestContext->pvContextStart, pvContext); > + } > + switch (status) { > + case WMT_STARTED: > + // MSDN says dwType is a QWORD, but it is actually a DWORD > + ok(dwType == WMT_TYPE_DWORD, "onStatus pValue not WMT_TYPE_QWORD (%d), but %d\n", (int)WMT_TYPE_DWORD, dwType); WMT_TYPE_QWORD typo? > + dwordValue = (DWORD*)pValue; > + ok(*dwordValue == 0, "onStatus call for WMT_STARTED not passed correct starting timestamp: %u\n", *dwordValue); > + SET_EXPECT(onSample); > + break; > + default: > + break; > + } The switch and if could be merged > + /* Get format count. */ > + formatCount = 0; > + hr = IWMReader_GetOutputFormatCount(reader, 0, &formatCount); > + todo_wine ok(hr == S_OK, "Failed to get output format count: 0x%08x\n", hr); formatCount should be checked Best Regards Alistair Leslie-Hughes

2 2

Re: wined3d: Fix context backup window handling.
by Henri Verbeet 25 Jul '17

25 Jul '17

On 25 July 2017 at 22:35, Sebastian Lackner <sebastian(a)fds-team.de> wrote: > - if (dc == context->hdc && !context->hdc_is_private && WindowFromDC(dc) != context->win_handle) > + if (dc == context->hdc && !private && WindowFromDC(dc) != context->win_handle) While that works, I think I'd feel happier about the code if the "dc", "private" and "format" parameters just went away. I.e., the reason they still exist is mostly the backup window path in context_set_gl_context(), and I don't think there's a reason that particular path shouldn't just set the appropriate (hdc, hdc_is_private, hdc_has_format) fields in the context instead.

1 0

Re: [PATCH] gdiplus: Avoid division by zero in SOFTWARE_GdipDrawThinPath
by Fabian Maurer 25 Jul '17

25 Jul '17

Or maybe we should draw this single pixel? Regards, Fabian Maurer

2 2

Re: [PATCH 4/4 v2] mshtml: Add IElementTraversal::get_firstElementChild implementation
by Marvin 25 Jul '17

25 Jul '17

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=32476 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 3/4 v2] mshtml: Expose IElementTraversal interface to scripts
by Marvin 25 Jul '17

25 Jul '17

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=32475 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 3/4 v2] mshtml: Expose IElementTraversal interface to scripts
by Jacek Caban 25 Jul '17

25 Jul '17

Signed-off-by: Jacek Caban <jacek(a)codeweavers.com>

1 0

Re: [PATCH 4/4] mshtml: Add IElementTraversal::get_firstElementChild implementation
by Marvin 25 Jul '17

25 Jul '17

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

1 0

Re: [PATCH 3/4] mshtml: Expose IElementTraversal interface to scripts
by Marvin 25 Jul '17

25 Jul '17

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

1 0

Re: [PATCH 1/5] po: Fix a typo in the Norwegian translation
by Kim Malmo 25 Jul '17

25 Jul '17

Thank you kindly! How did you find it? I read through the patch 3 times and missed it. Kim --- Den 24. juli 2017 14:47, skrev Hugh McMaster: > Signed-off-by: Hugh McMaster <hugh.mcmaster(a)outlook.com> > --- > po/nb_NO.po | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/po/nb_NO.po b/po/nb_NO.po > index 2fa8d89863..0ad494b4f2 100644 > --- a/po/nb_NO.po > +++ b/po/nb_NO.po > @@ -12512,7 +12512,7 @@ msgstr "regedit: Registerverdien \"%1\" vil bli eksportert som binære data.\n" > > #: regedit.rc:427 > msgid "regedit: Invalid system key [%1]\n" > -msgstr "reg: Ugyldig systemnøkkel [%1]\n" > +msgstr "regedit: Ugyldig systemnøkkel [%1]\n" > > #: regedit.rc:428 > msgid ""

1 0

Re: [1/2] gdi32/tests: Add test for SetPixel erase with R2_XORPEN
by Huw Davies 25 Jul '17

25 Jul '17

On Mon, Jul 24, 2017 at 12:57:30AM +1000, Tim Worthington wrote: > diff --git a/dlls/gdi32/tests/dib.c b/dlls/gdi32/tests/dib.c > --- a/dlls/gdi32/tests/dib.c > +++ b/dlls/gdi32/tests/dib.c > @@ -2703,6 +2703,26 @@ > ret = ExtFloodFill( hdc, -1, -1, RGB( 0, 0xff, 0 ), FLOODFILLSURFACE ); > ok (!ret, "got ret %d\n", ret); > > + /* SetPixel erase with R2_XORPEN */ > + if ( ( bmi->bmiHeader.biBitCount >= 16 ) > + || ( ( bmi->bmiHeader.biClrUsed == 256 ) && ( bmi->bmiHeader.biBitCount == 8 ) ) ) > + { > + int rop2; > + COLORREF set, get; > + > + rop2 = GetROP2( hdc ); > + for (i = 0; i < 256; i++) > + { > + SetROP2( hdc, R2_COPYPEN ); > + set = SetPixel( hdc, i * 2, 30, RGB( (i & 0x07) << 5, (i & 0x38) << 2, i & 0xc0 ) ); > + SetROP2( hdc, R2_XORPEN ); > + SetPixel( hdc, i * 2, 30, set ); > + get = GetPixel( hdc, i * 2, 30 ); > + ok( get == 0, "got %08x and %08x\n", set, get ); > + } > + SetROP2( hdc, rop2 ); > + } > + > SelectObject(hdc, orig_brush); > SelectObject(hdc, orig_pen); > DeleteObject(solid_brush); Hi Tim, Thank you for your contribution to Wine. There were a number of issues with this series, which by the time I'd reviewed them meant I'd essentially re-written the patches; so I just sent them in: https://source.winehq.org/patches/data/136093 https://source.winehq.org/patches/data/136094 This seemed more efficient than explaining the problems and suggesting alternative approaches. If you're interested, please free feel to look at my patches and ask if there's anything you're curious about. Huw.

2 1

Re: [PATCH] user32: In CURSORICON_FindBestCursor scale cursor correctly
by Fabian Maurer 24 Jul '17

24 Jul '17

Was too slow, not needed anymore after d0e8170c90ef63363f8389be2619c9332ae2c371. Regards, Fabian Maurer

1 0

Re: [PATCH] wmvcore/tests: add tests for WMReader methods
by Alistair Leslie-Hughes 24 Jul '17

24 Jul '17

Hi Jefferson, On 24/07/17 08:26, Jefferson Carpenter wrote: > Create a sample WMA file, which is saved into a temp directory at the > beginning of the wmvcore tests. > > Added a IWMReaderCallback implementation for use in conformance tests. > > Tested various playback-related WMReader methods. > > Changes in version 2 of this patch: Declared loop variable at top of > function, and using C89-style comments. The callback interface can just be static, see mshtml/tests/htmldoc.c for an example. Maybe using the CHECKED_CALLED syntax to check if something was been called. If the order of called is important then see msxml3/tests/saxreader.c for an example. You cannot use Sleep in tests, no matter how small it has to wait, instead use an Event+WaitForSingleObject. Use HeapAlloc/HeapFree instead of malloc/free, create a worker function if required. + IWMReader *reader; Leaking reader + /* Get output props. */ + hr = IWMReader_GetOutputProps(reader, 0, &props); + todo_wine ok(hr == S_OK, "Failed to get output props: 0x%08x\n", Leaking props here. .... + for (i = 0; i < formatCount; i++) { + /* Get output props for format. */ + hr = IWMReader_GetOutputFormat(reader, 0, i, &props); + todo_wine ok(hr == S_OK, "Failed to get output format: and here. Best Regards Alistair Leslie-Hughes

2 1

Re: mscoree: Update for mono profiler api v2.
by Vincent Povirk 24 Jul '17

24 Jul '17

Please don't commit this. They just changed it again and put in compatibility functions for the old API.

1 0

Some clarification of msi messages for translation and pedantic remarks
by Julian Rüger 24 Jul '17

24 Jul '17

Hi Zebediah (and list), I'd like some clarification of a few terms used in the new msi messages, to make sure I translate the correct meaning. :) #: msi.rc:103 msgid "Searching for related applications" ... #: msi.rc:111 msgid "Migrating feature states from related applications" related as in 'similar' or 'belonging together' or something else? I'd guess #2. #: msi.rc:113 msgid "Publishing assembly information" assembly as in 'binary/executable file/dll', 'the assembler language' or just 'putting together'? I strongly suspect #1, and that is a tough one. There is no fitting translation in German and this is not one of the many, many cases where you would just use the original English term. It sounds totally awkward. (Maybe because words ending in y are usually Germanized to use other suffixes. Just does not work with 'Assembly') EDIT: Never mind, I just looked it up and M$ does indeed use the untranslated original, even in compound words.. Ughhh! #: msi.rc:120 msgid "Registering Class servers" ... #: msi.rc:122 msgid "Registering extension servers" It's 'Server' in German as well for the hardware (machine/host) or applications/services like Apache, but this feels like a different thing. Am I being silly? Maybe this can only be answered by other German speakers. #: msi.rc:130 msi.rc:154 msgid "Updating environment strings" Can I just call this environment variables? Sounds more natural than the compound word with string ("Umgebungszeichenketten")... Thanks for your help and for all your work! Best, Julian PS: Some other questions/comments, unrelated to translation: #: msi.rc:168 msi.rc:171 msi.rc:175 msgid "File: [1], Directory: [9], Size: [6]" ... #: msi.rc:170 msgid "File: [1], Directory: [9], Size: [6]" Are these supposed to be separate, but identical? #: msi.rc:173 msgid "File: [1], Dependencies: [2]" ^^ Two spaces on purpose? #: msi.rc:176 msi.rc:177 msgid "Application Context:[1], Assembly Name:[2]" Missing spaces? #: msi.rc:189 msi.rc:209 msgid "Name: [1], Value: [2], Action [3]" Missing colon? (Action: [3])? #: msi.rc:178 msgid "File: [1], Directory: [2], Size: [3]" ... #: msi.rc:193 msi.rc:210 msgid "File: [1], Section: [2], Key: [3], Value: [4]" Duplicate spaces again?

2 1

Re: [PATCH 4/5] reg: Use a helper function to allocate and re-allocate memory
by Alexandre Julliard 24 Jul '17

24 Jul '17

Hugh McMaster <hugh.mcmaster(a)outlook.com> writes: > @@ -174,6 +177,26 @@ static BOOL ask_confirm(unsigned int msgid, WCHAR *reg_info) > } > } > > +#define CHECK_ENOUGH_MEMORY(p) \ > +if (!(p)) \ > +{ \ > + output_message(STRING_OUT_OF_MEMORY, __FILE__, __LINE__); \ > + exit(1); \ > +} > + > +static void *resize_buffer(void *buf, size_t count) > +{ > + void *new_buf; > + > + if (buf) > + new_buf = HeapReAlloc(GetProcessHeap(), 0, buf, count); > + else > + new_buf = HeapAlloc(GetProcessHeap(), 0, count); > + > + CHECK_ENOUGH_MEMORY(new_buf); > + return new_buf; > +} The macro is not needed, and the message doesn't need to be localized, since this would require allocating memory. I think you'll also want a separate wrapper for HeapAlloc. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 1/3] msvcrt: Don't set errno when calling trigonometric functions with NAN.
by Piotr Caban 24 Jul '17

24 Jul '17

Hi, I've done some more reading about it and it looks like our implementation is not behaving in similar way as native does. The error handling should call _matherr function (see also __setusermatherr). Thanks to implementing it that way we easily write code that is compatible with older versions of C-runtime (by providing different default implementation of matherr function that prevents setting errno). I think it's best to implement it that way to avoid potential regressions. Thanks, Piotr

2 2

Re: ntdll: Add padding to the stack structure for quad-word alignment on ARM64
by Alexandre Julliard 24 Jul '17

24 Jul '17

André Hentschel <nerv(a)dawncrow.de> writes: > EXCEPTION_RECORD is not correctly aligned and until bd1f7ef1868dd563a82eacd1bf414bf8d1a7254e, CONTEXT wasn't either, > which ended up in a correctly aligned structure, but the mentioned commit changed that. > > Signed-off-by: André Hentschel <nerv(a)dawncrow.de> > --- > dlls/ntdll/signal_arm64.c | 1 + > 1 file changed, 1 insertion(+) You should probably handle that in the existing alignment computation instead. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH v2 3/3] msvcr120: Add log1p.
by Piotr Caban 24 Jul '17

24 Jul '17

On 07/14/17 07:14, Alex Henrie wrote: > +/********************************************************************* > + * log1pf (MSVCR120.@) > + */ > +float CDECL MSVCR120_log1pf(float x) > +{ > +#ifdef HAVE_LOG1PF > + return log1pf(x); > +#else > + return log(1 + x); > +#endif > +} Please also set errno in this function. Thanks, Piotr

2 3

Re: [PATCH] wmvcore/tests: add tests for WMReader methods
by Marvin 23 Jul '17

23 Jul '17

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

1 0

Re: [PATCH] gdiplus: free dash_pattern_scaled (Coverity)
by Vincent Povirk 23 Jul '17

23 Jul '17

Thanks, Marcus. Unfortunately, with my patch for the same issue (caught by valgrind), this no longer applies. Can you rebase this please?

2 1

Re: ntdll: Fix stack alignment in RtlRaiseException assembly code.
by Alexandre Julliard 21 Jul '17

21 Jul '17

Sebastian Lackner <sebastian(a)fds-team.de> writes: > Signed-off-by: Sebastian Lackner <sebastian(a)fds-team.de> > --- > > Please review carefully, don't want to break anything shortly before the > release. ;) > > dlls/ntdll/signal_i386.c | 41 ++++++++++++++++++++++++----------------- > 1 file changed, 24 insertions(+), 17 deletions(-) All it does is call other API functions that will align the stack anyway, so I'm not sure it's worth the trouble. -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: [PATCH 1/2 v2] reg: Account for sizeof(WCHAR) when resizing the value name buffer during the query operation
by Alexandre Julliard 21 Jul '17

21 Jul '17

Hugh McMaster <hugh.mcmaster(a)outlook.com> writes: > Changes since v1: > - Handle HeapReAlloc() failure This is a stand-alone app, so adding a couple of helpers that die on allocation failure would probably be a better choice than checking everywhere. -- Alexandre Julliard julliard(a)winehq.org

2 1

Re: [PATCH] mshtml: Added IHTMLDOMNode3::put_textContent implementation.
by Marvin 21 Jul '17

21 Jul '17

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=32419 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 1/2] xmllite/reader: Implement MoveToAttributeByName()
by Marvin 21 Jul '17

21 Jul '17

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=32415 Your paranoid android. === wxppro (32 bit reader) === reader.c:2597: Test failed: Unexpected hr 0x1. === w2003std (32 bit reader) === reader.c:2597: Test failed: Unexpected hr 0x1.

1 0

Re: [PATCH 2/8] d3d11: Fix race condition in d3d_blend_state_create().
by Henri Verbeet 21 Jul '17

21 Jul '17

On 20 July 2017 at 14:10, Józef Kucia <jkucia(a)codeweavers.com> wrote: > Arguably we could use a different mutex for "device->blend_states". > Yeah. Generally speaking we'd like wined3d locking to be more fine-grained, but there are probably a bunch of issues with that.

1 0

Re: [PATCH] reg: Account for sizeof(WCHAR) when resizing the value name buffer during the query operation
by Henri Verbeet 21 Jul '17

21 Jul '17

On 21 July 2017 at 10:47, Hugh McMaster <hugh.mcmaster(a)outlook.com> wrote: > - value_name = HeapReAlloc(GetProcessHeap(), 0, value_name, max_value_len); > + value_name = HeapReAlloc(GetProcessHeap(), 0, value_name, max_value_len * sizeof(WCHAR)); Note that this leaks the existing "value_name" allocation if the HeapReAlloc() call should fail.

1 0

Re: [PATCH 3/6] msi: Don't set _BrowseProperty.
by Zebediah Figura 21 Jul '17

21 Jul '17

On 07/20/2017 11:43 PM, Zebediah Figura wrote: > This is a revert of 166a7e993cb0033fac7ec2712c346558fb7bd100, > which was introduced to fix bug #28219. _BrowseProperty is not set > by Windows, but should rather be set by the program in question, and > was not being set due to a bug in Wine, fixed by the previous patch. Sorry, typo—meant to say next patch (4/6).

1 0

Re: [PATCH 2/2] mshtml: Added IHTMLDOMNode3::get_textContent implementation.
by Marvin 20 Jul '17

20 Jul '17

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=32404 Your paranoid android. === w7u (32 bit htmldoc) === htmldoc.c:2682: Test failed: unexpected call GetOverrideKeyPath === w864 (64 bit events) === events.c:987: Test failed: unexpected call img_onerror

1 0

Re: [v3] gdi32: return non-zero v-refresh value for display devices.
by Huw Davies 20 Jul '17

20 Jul '17

On Thu, Jul 20, 2017 at 10:56:40PM +0900, Akihiro Sagawa wrote: > > To avoid division by zero (Bug 43369). > > In version 3, refine a testcase. > The refresh rate value is neither zero nor negative, thus we should test both. > Thanks for reviewing. > > Signed-off-by: Akihiro Sagawa <sagawa.aki(a)gmail.com> > --- > dlls/gdi32/driver.c | 2 +- > dlls/gdi32/tests/dc.c | 8 ++++++++ > 2 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/dlls/gdi32/driver.c b/dlls/gdi32/driver.c > index fe7812c..6a3975a 100644 > --- a/dlls/gdi32/driver.c > +++ b/dlls/gdi32/driver.c > @@ -344,7 +344,7 @@ static INT nulldrv_GetDeviceCaps( PHYSDEV dev, INT cap ) > case PHYSICALOFFSETY: return 0; > case SCALINGFACTORX: return 0; > case SCALINGFACTORY: return 0; > - case VREFRESH: return 0; > + case VREFRESH: return GetDeviceCaps( dev->hdc, TECHNOLOGY ) == DT_RASDISPLAY ? 1 : 0; > case DESKTOPVERTRES: return GetDeviceCaps( dev->hdc, VERTRES ); > case DESKTOPHORZRES: return GetDeviceCaps( dev->hdc, HORZRES ); > case BLTALIGNMENT: return 0; > diff --git a/dlls/gdi32/tests/dc.c b/dlls/gdi32/tests/dc.c > index add2c04..d316f3f 100644 > --- a/dlls/gdi32/tests/dc.c > +++ b/dlls/gdi32/tests/dc.c > @@ -388,6 +388,7 @@ static void test_device_caps( HDC hdc, HDC ref_dc, const char *descr, int scale > { > INT precision = 0; > INT hdc_caps = GetDeviceCaps( hdc, caps[i] ); > + INT ref_caps; > > switch (caps[i]) > { > @@ -400,6 +401,13 @@ static void test_device_caps( HDC hdc, HDC ref_dc, const char *descr, int scale > case LOGPIXELSY: > hdc_caps *= scale; > break; > + case VREFRESH: > + ref_caps = GetDeviceCaps( ref_dc, caps[i] ); > + if (GetDeviceCaps( ref_dc, TECHNOLOGY ) == DT_RASDISPLAY) > + ok( ref_caps > 0, "expected a positive value on %s, got %d\n", descr, ref_caps ); > + else > + ok( ref_caps == 0, "expected 0 on %s, got %d\n", descr, ref_caps ); > + break; > } You don't want to use 'ref_dc' here, use 'hdc' instead. Also, note that 'hdc_caps' already contains the thing you want to test, so you don't need the first call to GetDeviceCaps(). Huw.

1 0

Re: [PATCH] widl: Handle C++ aggregate returns in a MSVC compatible way.
by Jacek Caban 20 Jul '17

20 Jul '17

Hi Henri, On 20.07.2017 14:18, Henri Verbeet wrote: > This mainly affects 64-bit winelib applications, and potentially mingw-w64 > usage of the Wine headers. As explained in commit > fabfa59aea5c5b3201142382038beb3e76cb7567, MSVC returns aggregates through an > implicit parameter immediately after the this/interface pointer. GCC's > "ms_abi" attribute is supposed to match this, but unfortunately current > versions of g++ (confirmed up to at least g++ 6.3) place the implicit return > pointer before the this/interface pointer. See also > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52792. FWIW, here is a bug about 32-bit problem: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64384 > This commit takes an approach similar to the earlier commit > fabfa59aea5c5b3201142382038beb3e76cb7567: For ABI compatibility the return > pointer is listed as an explicit parameter in the vtbl entry, while an inline > helper is provided for source code compatibility. While it fixes the problem for C++ callers, it breaks building classes implementing those interfaces. Implementation will try to override inline function instead of virtual one. Given that the workaround has bad side effects, I'd say that we should at least try harder to not use it. For example it could be #ifdefed to not be used by compilers doing the right thing. With that, using our headers would require modifications in compiled code compatible with MSVC, so there would be a point in having a nice to use macro that interface implementations could use to decide if they should apply needed changes. Obviously fixing compilers sounds much better. Jacek

2 3

Re: [PATCH] gdi32: Update enhanced metafile world transform immediately in winnt mode
by Huw Davies 20 Jul '17

20 Jul '17

On Wed, Jul 19, 2017 at 02:06:29PM -0500, Andrew Eikum wrote: > /***************************************************************************** > * PlayEnhMetaFileRecord (GDI32.@) > @@ -1443,14 +1458,20 @@ BOOL WINAPI PlayEnhMetaFileRecord( > info->state.world_transform.eM11 = info->state.world_transform.eM22 = 1; > info->state.world_transform.eM12 = info->state.world_transform.eM21 = 0; > info->state.world_transform.eDx = info->state.world_transform.eDy = 0; > + if (!IS_WIN9X()) > + EMF_Update_MF_Xform(hdc, info); > break; > case MWT_LEFTMULTIPLY: > CombineTransform(&info->state.world_transform, &lpModifyWorldTrans->xform, > &info->state.world_transform); > + if (!IS_WIN9X()) > + ModifyWorldTransform(hdc, &lpModifyWorldTrans->xform, MWT_LEFTMULTIPLY); > break; > case MWT_RIGHTMULTIPLY: > CombineTransform(&info->state.world_transform, &info->state.world_transform, > &lpModifyWorldTrans->xform); > + if (!IS_WIN9X()) > + EMF_Update_MF_Xform(hdc, info); > break; > default: > FIXME("Unknown imode %d\n", lpModifyWorldTrans->iMode); > @@ -2216,6 +2237,13 @@ BOOL WINAPI PlayEnhMetaFileRecord( > LPtoDP(hdc, (POINT*)&tmprc, 2); > TRACE("L:0,0 - 1000,1000 -> D:%s\n", wine_dbgstr_rect(&tmprc)); > > + if (emr_modifies_transform( mr->iType ) && !IS_WIN9X()) > + { > + /* WinNT - update the transform (win9x updates when the next graphics > + output record is played). */ > + EMF_Update_MF_Xform(hdc, info); > + } > + > return TRUE; > } > I think it would be better to call EMF_Update_MF_Xform() in each 'case' block that needs it, just like you are doing for EMR_MODIFYWORLDTRANSFORM. Also, what about EMR_SETMAPMODE? > diff --git a/dlls/gdi32/tests/metafile.c b/dlls/gdi32/tests/metafile.c > index eeec79c800..eaad666361 100644 > --- a/dlls/gdi32/tests/metafile.c > +++ b/dlls/gdi32/tests/metafile.c [snip] > START_TEST(metafile) > { > init_function_pointers(); > @@ -3928,6 +4430,9 @@ START_TEST(metafile) > test_emf_paths(); > test_emf_PolyPolyline(); > test_emf_GradientFill(); > + test_emf_WorldTransform_simple(); > + test_emf_WorldTransform_modify(); > + test_emf_WorldTransform_modified_playback(); > You have a lot of duplicated code here. It should be possible to have a single test_emf_WorldTransform() that uses a table to drive these three tests. Could you also make the matrices a little more interesting? Adding a translation would make them non-commuting which would be good to test. Huw.

1 0

Re: [PATCH 3/3] mshtml: Expose IHTMLDOMNode3 interface to scripts.
by Marvin 19 Jul '17

19 Jul '17

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

1 0

Re: [v2] gdi32: return non-zero v-refresh value for display devices.
by Huw Davies 19 Jul '17

19 Jul '17

On Wed, Jul 19, 2017 at 11:28:58PM +0900, Akihiro Sagawa wrote: > diff --git a/dlls/gdi32/tests/dc.c b/dlls/gdi32/tests/dc.c > index add2c04..2b48542 100644 > --- a/dlls/gdi32/tests/dc.c > +++ b/dlls/gdi32/tests/dc.c > @@ -445,6 +445,7 @@ static void test_device_caps( HDC hdc, HDC ref_dc, const char *descr, int scale > if (type != COMPLEXREGION && type != ERROR) /* region can be complex on multi-monitor setups */ > { > RECT ref_rect; > + int rate = GetDeviceCaps( ref_dc, VREFRESH ); > > ok( type == SIMPLEREGION, "GetClipBox returned %d on %s\n", type, descr ); > if (GetDeviceCaps( ref_dc, TECHNOLOGY ) == DT_RASDISPLAY) > @@ -462,11 +463,15 @@ static void test_device_caps( HDC hdc, HDC ref_dc, const char *descr, int scale > SetRect( &ref_rect, GetSystemMetrics( SM_XVIRTUALSCREEN ), GetSystemMetrics( SM_YVIRTUALSCREEN ), > GetSystemMetrics( SM_XVIRTUALSCREEN ) + GetSystemMetrics( SM_CXVIRTUALSCREEN ), > GetSystemMetrics( SM_YVIRTUALSCREEN ) + GetSystemMetrics( SM_CYVIRTUALSCREEN ) ); > + > + ok( rate >= 1 && rate <= 85, "returned rate is out of range (1 -- 85), got %d\n", rate); Don't check the range, just that it's != 0. Also, please move this to a new VREFRESH case in the switch block higher up in this function. Huw.

1 0

Re: winhttp: Rename host_t to hostdata_t.
by Henri Verbeet 19 Jul '17

19 Jul '17

On 19 July 2017 at 17:22, Sebastian Lackner <sebastian(a)fds-team.de> wrote: > Commit fa2cc9868b7db7a4cbf7d01a48500cb3f31e109a broke the compilation macOS > because host_t conflicts with a system header file: > > [19:31:41] In file included from ../../../wine/dlls/winhttp/session.c:50: > [19:31:41] ../../../wine/dlls/winhttp/winhttp_private.h:106:3: error: typedef redefinition with different types ('struct host_t' vs 'mach_port_t' (aka 'unsigned int')) > [19:31:41] } host_t; > [19:31:41] ^ > [19:31:41] /build/macos-rootfs/usr/include/mach/mach_types.h:120:22: note: previous definition is here > [19:31:41] typedef mach_port_t host_t; > [19:31:41] ^ > It may be too late for winhttp or Wine in general (although Michael does love these kinds of things), but note that the _t suffix is reserved by POSIX and should in principle be avoided on POSIX systems.

1 0

Re: gdi32: return non-zero v-refresh value for display devices.
by Huw Davies 19 Jul '17

19 Jul '17

On Wed, Jul 19, 2017 at 12:11:28AM +0900, Akihiro Sagawa wrote: > > To avoid division by zero (Bug 43369). > > Signed-off-by: Akihiro Sagawa <sagawa.aki(a)gmail.com> > --- > dlls/gdi32/driver.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/dlls/gdi32/driver.c b/dlls/gdi32/driver.c > index fe7812c..6a3975a 100644 > --- a/dlls/gdi32/driver.c > +++ b/dlls/gdi32/driver.c > @@ -344,7 +344,7 @@ static INT nulldrv_GetDeviceCaps( PHYSDEV dev, INT cap ) > case PHYSICALOFFSETY: return 0; > case SCALINGFACTORX: return 0; > case SCALINGFACTORY: return 0; > - case VREFRESH: return 0; > + case VREFRESH: return GetDeviceCaps( dev->hdc, TECHNOLOGY ) == DT_RASDISPLAY ? 1 : 0; > case DESKTOPVERTRES: return GetDeviceCaps( dev->hdc, VERTRES ); > case DESKTOPHORZRES: return GetDeviceCaps( dev->hdc, HORZRES ); > case BLTALIGNMENT: return 0; Could you add a test (probably in dc.c:test_device_caps())? The values returned by the testbot range from 1 -- 85, so 1 is as good a choice as any. Huw.

2 2

Re: [v2 3/5] d3dx9: Support skip_constants parameter for effect.
by Matteo Bruni 18 Jul '17

18 Jul '17

2017-07-11 13:26 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > @@ -6397,24 +6464,75 @@ static HRESULT d3dx9_base_effect_init(struct d3dx9_base_effect *base, > TRACE("Tag: %x\n", tag); > } > > + if (skip_constants_string) > + { > + skip_constants_buffer = HeapAlloc(GetProcessHeap(), 0, > + sizeof(*skip_constants_buffer) * (strlen(skip_constants_string) + 1)); I think you can get rid of the sizeof() in this case. If sizeof(*skip_constants_buffer) is not 1 we have bigger problems anyway... > + if (!skip_constants_buffer) > + { > + if (bytecode) > + ID3D10Blob_Release(bytecode); > + return E_OUTOFMEMORY; > + } > + strcpy(skip_constants_buffer, skip_constants_string); Ideally this one should be a memcpy(), reusing the string length computed above. > @@ -588,7 +588,8 @@ static unsigned int *parse_pres_ins(unsigned int *ptr, unsigned int count, struc > return ptr; > } > > -static HRESULT get_ctab_constant_desc(ID3DXConstantTable *ctab, D3DXHANDLE hc, D3DXCONSTANT_DESC *desc) > +static HRESULT get_ctab_constant_desc(ID3DXConstantTable *ctab, D3DXHANDLE hc, D3DXCONSTANT_DESC *desc, > + WORD *constantinfo_reserved) > { > const struct ctab_constant *constant = d3dx_shader_get_ctab_constant(ctab, hc); > > @@ -598,10 +599,13 @@ static HRESULT get_ctab_constant_desc(ID3DXConstantTable *ctab, D3DXHANDLE hc, D > return D3DERR_INVALIDCALL; > } > *desc = constant->desc; > + if (constantinfo_reserved) > + *constantinfo_reserved = constant->constantinfo_reserved; > return D3D_OK; > } For future reference: these (and related) changes should probably have been in a separate patch, or even part of the previous patch in the series. Also, as it becomes more obvious here, one could go one step further with this and directly return struct ctab_constant to the caller. That would essentially make the get_ctab_constant_desc() helper redundant so it could be merged into the callers and removed entirely. > + for (j = 0; j < skip_constants_count; ++j) > + { > + if (!strcmp(cdesc[index].Name, skip_constants[j])) > + { > + if (!constantinfo_reserved) > + { > + WARN("skip_constants parameter %s is not register bound.\n", > + cdesc[index].Name); > + hr = D3DERR_INVALIDCALL; > + goto cleanup; > + } > + TRACE("Skipping constant %s.\n", cdesc[index].Name); > + break; > + } > + } > + if (j < skip_constants_count) > + continue; > + ++out->input_count; > + if (inputs_param[index]->class == D3DXPC_OBJECT) > + continue; > + if (FAILED(hr = init_set_constants_param(out, ctab, hc, inputs_param[index]))) > + goto cleanup; > } > - out->input_count = desc.Constants; > if (out->const_set_count) > { > out->const_set = HeapReAlloc(GetProcessHeap(), 0, out->const_set, Not particularly important but we could shrink out->inputs and out->inputs_param to out->input_count if any parameter was skipped.

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

2003

2002

2001

wine-devel