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

Re: [PATCH 6/6] urlmon/tests: Added BindProtocol redirection handling tests.
by Alexandre Julliard June 6, 2017

June 6, 2017

Jacek Caban <jacek(a)codeweavers.com> writes: > Signed-off-by: Jacek Caban <jacek(a)codeweavers.com> > --- > dlls/urlmon/tests/protocol.c | 155 > ++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 152 insertions(+), 3 deletions(-) This is failing consistently here: ../../../tools/runtest -q -P wine -T ../../.. -M urlmon.dll -p urlmon_test.exe.so protocol && touch protocol.ok protocol.c:1483: Test failed: unexpected call SetPriority protocol.c:1483: Test failed: unexpected call SetPriority Makefile:249: recipe for target 'protocol.ok' failed make: *** [protocol.ok] Error 2 -- Alexandre Julliard julliard(a)winehq.org

1 0

Wine uses the wrong pkg-config for 32 bit builds
by Francois Gouget June 6, 2017

June 6, 2017

The problem with pkg-config is that it has no idea what the build target is so it always returns the include and library paths appropriate for the build host. For instance when Wine tries to figure out how to use GStreamer 1.0 it calls: $ pkg-config --cflags gstreamer-1.0 -pthread -I/usr/include/gstreamer-1.0 -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include It's pretty obvious that the 'x86_64-linux-gnu' is wrong for a 32 bit build. But then we never told pkg-config that we wanted to do a 32 bit build! There are two ways to tell pkg-config we want to do a 32 bit build. * The first way is to set PKG_CONFIG_LIBDIR to force pkg-config to only use the .pc files we want. However this requires knowing where the .pc files are stored and you'd want to make sure to include /usr/lib/<triplet>/pkgconfig, /usr/local/lib/<triplet>/pkgconfig. And then should you also include /usr/lib/pkgconfig? * Another way is to use a <triplet>-pkg-config tool, which will give us the right information for the specified <triplet>. Red Hat, Debian and its derivatives (Ubuntu, etc) all provide them. This gives: $ i686-linux-gnu-pkg-config --cflags gstreamer-1.0 -pthread -I/usr/include/gstreamer-1.0 -I/usr/include/glib-2.0 -I/usr/lib/i386-linux-gnu/glib-2.0/include We already have a WINE_CHECK_HOST_TOOL() macro that looks like it should do the trick. But it only takes ac_tool_prefix into account. So I modified it to take the host target into account if ac_tool_prefix is not set. This is modeled after the WINE_CHECK_MINGW_PROG macro and you can see the result in the patch below. On Red Hat the it's a quadruplet: it has an extra 'redhat-' in the name (it's i686-redhat-linux-gnu-pkg-config for instance). The patch below does not account for it. I'm hoping Red Hat developers can suggest how to best take this into account. Is this approach ok? ----- configure: Take the target host into account in WINE_CHECK_HOST_TOOL(). In particular this gets us the right pkg-config tool when building the 32 bit Wine. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- aclocal.m4 | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/aclocal.m4 b/aclocal.m4 index 9364188e334..4ac467a71e5 100644 --- a/aclocal.m4 +++ b/aclocal.m4 @@ -27,13 +27,17 @@ dnl dnl Like AC_CHECK_TOOL but without the broken fallback to non-prefixed name dnl AC_DEFUN([WINE_CHECK_HOST_TOOL], -[AS_VAR_SET_IF([ac_tool_prefix], - AC_CHECK_PROG([$1],[${ac_tool_prefix}$2],[${ac_tool_prefix}$2],,[$4])) -AS_VAR_IF([ac_cv_prog_$1],[], - [AS_VAR_IF([cross_compiling],[yes],[], - [AS_UNSET([ac_cv_prog_$1]) - AC_CHECK_PROG([$1],[$2],[$2],[$3],[$4])])], -[AS_VAR_COPY([$1],[ac_cv_prog_$1])])]) +[AS_VAR_IF([ac_tool_prefix],[], + [case "$host_cpu" in + i[[3456789]]86*) + ac_host_tool_list="m4_foreach([ac_wine_cpu],[i686,i586,i486,i386],[ac_wine_cpu-${host_os}-$2 ]) $2" ;; + x86_64) + ac_host_tool_list="x86_64-${host_os}-$2 $2" ;; + *) + ac_host_tool_list="" ;; + esac], + [ac_host_tool_tool_list="${ac_tool_prefix}-$2"]) + AC_CHECK_PROGS($1,$ac_host_tool_list,[$3],[$4])]) dnl **** Initialize the programs used by other checks **** dnl -- 2.11.0

4 11

Re: [PATCH v2 1/2] msvcrt: Clean up registered C++ object in handler.
by Piotr Caban June 6, 2017

June 6, 2017

Hi Daniel, On 06/06/17 00:43, Daniel Lehman wrote: > +typedef void (CALLBACK *__WINE_FINALLY_CTX)(BOOL, PEXCEPTION_RECORD, void*); The idea behind __FINALLY_CTX was to model native __finally behavior while extending its functionality for wine. It's not possible to call GetExceptionInfo/GetExceptionCode inside of finally on Windows so I don't think it's a change in good direction. If you really need the exception record you shouldn't use __FINALLY_CTX (but I don't think you need). > +static void CALLBACK cxx_catch_cleanup(BOOL normal, EXCEPTION_RECORD *rec, void *ctx) > +{ > + thread_data_t *data = msvcrt_get_thread_data(); > + cxx_frame_info *frame = ctx; > + > + if (normal) > + __CxxUnregisterExceptionObject(frame, FALSE); > + else > + { > + if (cxx_is_consolidate(rec)) > + rec = (void*)rec->ExceptionInformation[4]; > + > + __CxxUnregisterExceptionObject(frame, > + rec->ExceptionCode == CXX_EXCEPTION && > + data->exc_record->ExceptionCode == CXX_EXCEPTION && > + rec->ExceptionInformation[1] == data->exc_record->ExceptionInformation[1]); > + } Do you need to call __CxxUnregisterExceptionObject with TRUE as second argument in any case except of rethrow? > /* FIXME: native does local_unwind here in case of exception rethrow */ If you only need to handle rethrow in a special way I think it should be done differently. Something like this should do the job: __TRY { __TRY { ret_addr = handler(0, frame); } __EXCEPT(rethrow_filter) { /* probably all of this should be done inside of the filter */ /* do some unwinding here or add a FIXME */ catch_cleanup_ctx.in_use = TRUE; rethrow; } __ENDTRY; } __FINALLY_CTX(cxx_catch_cleanup, &catch_cleanup_ctx); The above code is incorrect if exception object needs to be preserved not only in case of rethrow. What do you think about it? Thanks, Piotr

2 1

Re: [PATCH 6/6] urlmon/tests: Added BindProtocol redirection handling tests.
by Marvin June 6, 2017

June 6, 2017

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=31439 Your paranoid android. === w864 (32 bit protocol) === protocol.c:875: Test failed: unexpected call ReportProgress_CONNECTING === w864 (64 bit protocol) === protocol.c:875: Test failed: unexpected call ReportProgress_CONNECTING

1 0

Re: [v2 PATCH 2/3] mfplat: Implement MFCreateMediaType
by Nikolay Sivov June 6, 2017

June 6, 2017

On 31.05.2017 4:09, Alistair Leslie-Hughes wrote: > +typedef struct _mfmedaitype > +{ > + IMFMediaType IMFMediaType_iface; > + LONG ref; > +} mfmedaitype; You got a typo in structure name.

4 4

Re: [v3 PATCH] oleaut32: Improve support for IDispatch in VarCat
by Jacek Caban June 6, 2017

June 6, 2017

Hi Alistair, The patch looks mostly good now. On 26.05.2017 07:13, Alistair Leslie-Hughes wrote: > + hres = VariantChangeTypeEx(&bstrvar_left,tmp,0,VARIANT_ALPHABOOL|VARIANT_LOCALBOOL,VT_BSTR); > + if (hres == DISP_E_TYPEMISMATCH) > { It seems that might be the right thing to do, but one more test would make sure about that. Please add a test that will hit this case without VT_DISPATCH nor VT_NULL, since both of them are kind of special. As far as I can see, a VT_BYREF with NULL reference would be good for that. > - hres = VariantChangeTypeEx(&bstrvar_left,left,0,VARIANT_ALPHABOOL|VARIANT_LOCALBOOL,VT_BSTR); > - if (hres != S_OK) { > - VariantClear(&bstrvar_left); > - VariantClear(&bstrvar_right); > - return hres; > - } > + V_VT(&bstrvar_left) = VT_BSTR; > + V_BSTR(&bstrvar_left) = SysAllocString(sz_empty); You could just set it to NULL. Thanks, Jacek

1 0

Re: [PATCH 3/3] inetcomm: Implemented IMimeBody::GetClassID()
by Jacek Caban June 6, 2017

June 6, 2017

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

1 0

Re: [PATCH 2/3] inetcomm: Release previously set data (Valgrind)
by Jacek Caban June 6, 2017

June 6, 2017

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

1 0

Re: [PATCH 5/8] d3dx9/tests: Factor out test_effect_preshader_compare_pbool_consts() function.
by Matteo Bruni June 5, 2017

June 5, 2017

2017-06-02 11:58 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > + if (!const_updated_mask) > + { > + for (i = 0; i < ARRAY_SIZE(test_effect_preshader_bconsts); ++i) > + { > + ok_(__FILE__, line)(!bdata[i] == !test_effect_preshader_bconsts[i], > + "Pixel shader boolean constants do not match, expected %#x, got %#x, i %u.\n", > + test_effect_preshader_bconsts[i], bdata[i], i); > + } > + } > + else > + { > + for (i = 0; i < ARRAY_SIZE(test_effect_preshader_bconsts); ++i) > + { > + if (const_updated_mask[i / TEST_EFFECT_BITMASK_BLOCK_SIZE] > + & (1u << (i % TEST_EFFECT_BITMASK_BLOCK_SIZE))) > + { > + ok_(__FILE__, line)(!bdata[i] == !test_effect_preshader_bconsts[i], > + "Pixel shader boolean constants do not match, expected %#x, got %#x, i %u, parameter %s.\n", > + test_effect_preshader_bconsts[i], bdata[i], i, updated_param); > + } > + else > + { > + ok_(__FILE__, line)(!bdata[i], > + "Pixel shader boolean constants updated unexpectedly, parameter %s.\n", updated_param); > + } > + } > + } Minor simplification possibility: in the (!const_updated_mask) case you could use a default "all ones" mask and use what's now the 'else' branch, getting rid of the 'if' branch altogether.

2 2

Re: [PATCH 5/5] d3drm: Consolidate GetClassName() implementation, reduce tests duplication
by Henri Verbeet June 5, 2017

June 5, 2017

On 5 June 2017 at 01:07, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > +HRESULT d3drm_object_get_class_name(struct d3drm_object *object, DWORD *size, char *name) > +{ > + DWORD req_size; > + > + if (!size) > + return E_INVALIDARG; > + > + req_size = strlen(object->classname) + 1; > + if (name && *size < req_size) > + return E_INVALIDARG; > + > + *size = req_size; > + > + if (name) > + strcpy(name, object->classname); Note that you already know the string length, so just use memcpy(). (I.e., there's almost never a good reason to use strcpy().) > diff --git a/dlls/d3drm/tests/d3drm.c b/dlls/d3drm/tests/d3drm.c > index e1091be423..a0dfe6c7d2 100644 > --- a/dlls/d3drm/tests/d3drm.c > +++ b/dlls/d3drm/tests/d3drm.c > @@ -52,6 +52,42 @@ static D3DRMMATRIX4D identity = { > { 0.0f, 0.0f, 0.0f, 1.0f } > }; > > + > +static void test_class_name(IDirect3DRMObject *object, const char *name) > +{ > + char cname[64] = {0}; > + HRESULT hr; > + DWORD size; > + > + hr = IDirect3DRMObject_GetClassName(object, NULL, cname); > + ok(hr == E_INVALIDARG, "GetClassName failed with %x\n", hr); "Got unexpected hr %#x.\n" > + hr = IDirect3DRMViewport_GetClassName(object, NULL, NULL); > + ok(hr == E_INVALIDARG, "GetClassName failed with %x\n", hr); > + > + size = 0; > + hr = IDirect3DRMObject_GetClassName(object, &size, NULL); > + ok(hr == D3DRM_OK, "Cannot get classname size (hr = %x)\n", hr); > + ok(size == strlen(name) + 1, "wrong size: %u\n", size); > + > + size = 1; > + hr = IDirect3DRMObject_GetClassName(object, &size, cname); > + ok(hr == E_INVALIDARG, "GetClassName failed with %x\n", hr); > + ok(size == 1, "Got size %u.\n", size); > + > + size = sizeof(cname); > + hr = IDirect3DRMObject_GetClassName(object, &size, cname); > + ok(hr == D3DRM_OK, "Cannot get classname (hr = %x)\n", hr); > + ok(size == strlen(name) + 1, "wrong size: %u\n", size); > + ok(!strcmp(cname, name), "Expected cname to be \"%s\", but got \"%s\"\n", name, cname); > + > + size = strlen(name); > + strcpy(cname, "XXX"); > + hr = IDirect3DRMObject_GetClassName(object, &size, cname); > + ok(hr == E_INVALIDARG, "GetClassName failed with %x\n", hr); > + ok(size == strlen(name), "wrong size: %u\n", size); > + ok(!strcmp(cname, "XXX"), "Expected unchanged buffer, but got \"%s\"\n", cname); > +} I would be happier if this printed the line number of the caller on failure, see e.g. check_interface_() in the d3d11 tests.

1 0