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

Re: [2/3] kernel32/tests: Use SetFilePointer instead of GetFileSize with WriteFile
by Marvin Aug. 7, 2016

Aug. 7, 2016

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=24823 Your paranoid android. === w7u (32 bit loader) === loader.c:2440: Test failed: WaitForSingleObject failed: 102 loader.c:2442: Test failed: expected exit code 199, got 259 loader.c:2461: Test failed: expected 198, got 199 === w1064 (32 bit loader) === loader.c:2857: Test failed: Test 0: ResolveDelayLoadedAPI failed loader.c:2858: Test failed: Test 0: expected 74374550, got 00000000 loader.c:2857: Test failed: Test 1: ResolveDelayLoadedAPI failed loader.c:2858: Test failed: Test 1: expected 72BC2CF0, got 00000000 loader.c:2857: Test failed: Test 2: ResolveDelayLoadedAPI failed loader.c:2858: Test failed: Test 2: expected 74372DC0, got 00000000 loader.c:2865: Test failed: Test 3: ResolveDelayLoadedAPI succeeded with 00000000 loader.c:2866: Test failed: Test 3: Wrong callback count: 0 loader.c:2865: Test failed: Test 4: ResolveDelayLoadedAPI succeeded with 00000000 loader.c:2866: Test failed: Test 4: Wrong callback count: 0 === wvistau64 (64 bit loader) === loader.c:718: Test failed: 2: unexpected error 193 loader.c:718: Test failed: 3: unexpected error 193 loader.c:718: Test failed: 4: unexpected error 193 loader.c:718: Test failed: 6: unexpected error 193 loader.c:718: Test failed: 7: unexpected error 193 loader.c:173: Test failed: WriteFile error 1784 === w2008s64 (64 bit loader) === loader.c:718: Test failed: 2: unexpected error 193 loader.c:718: Test failed: 3: unexpected error 193 loader.c:718: Test failed: 4: unexpected error 193 loader.c:718: Test failed: 6: unexpected error 193 loader.c:718: Test failed: 7: unexpected error 193 loader.c:173: Test failed: WriteFile error 1784 === w7pro64 (64 bit loader) === loader.c:718: Test failed: 2: unexpected error 193 loader.c:718: Test failed: 3: unexpected error 193 loader.c:718: Test failed: 4: unexpected error 193 loader.c:718: Test failed: 6: unexpected error 193 loader.c:718: Test failed: 7: unexpected error 193 loader.c:173: Test failed: WriteFile error 1784 === w864 (64 bit loader) === loader.c:718: Test failed: 2: unexpected error 193 loader.c:718: Test failed: 3: unexpected error 193 loader.c:718: Test failed: 4: unexpected error 193 loader.c:718: Test failed: 6: unexpected error 193 loader.c:718: Test failed: 7: unexpected error 193 loader.c:173: Test failed: WriteFile error 1784 === w1064 (64 bit loader) === loader.c:718: Test failed: 2: unexpected error 193 loader.c:718: Test failed: 3: unexpected error 193 loader.c:718: Test failed: 4: unexpected error 193 loader.c:718: Test failed: 6: unexpected error 193 loader.c:718: Test failed: 7: unexpected error 193 loader.c:173: Test failed: WriteFile error 1784 The test timed out

1 0

Re: [1/3] kernel32/tests: Explicitly set SizeOfRawData
by Marvin Aug. 7, 2016

Aug. 7, 2016

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=24822 Your paranoid android. === w1064 (32 bit loader) === loader.c:2867: Test failed: Test 0: ResolveDelayLoadedAPI failed loader.c:2868: Test failed: Test 0: expected 74374550, got 00000000 loader.c:2867: Test failed: Test 1: ResolveDelayLoadedAPI failed loader.c:2868: Test failed: Test 1: expected 72BC2CF0, got 00000000 loader.c:2867: Test failed: Test 2: ResolveDelayLoadedAPI failed loader.c:2868: Test failed: Test 2: expected 74372DC0, got 00000000 loader.c:2875: Test failed: Test 3: ResolveDelayLoadedAPI succeeded with 00000000 loader.c:2876: Test failed: Test 3: Wrong callback count: 0 loader.c:2875: Test failed: Test 4: ResolveDelayLoadedAPI succeeded with 00000000 loader.c:2876: Test failed: Test 4: Wrong callback count: 0 === wvistau64 (64 bit loader) === loader.c:718: Test failed: 2: unexpected error 193 loader.c:718: Test failed: 3: unexpected error 193 loader.c:718: Test failed: 4: unexpected error 193 loader.c:718: Test failed: 6: unexpected error 193 loader.c:718: Test failed: 7: unexpected error 193 loader.c:173: Test failed: WriteFile error 1784 === w2008s64 (64 bit loader) === loader.c:718: Test failed: 2: unexpected error 193 loader.c:718: Test failed: 3: unexpected error 193 loader.c:718: Test failed: 4: unexpected error 193 loader.c:718: Test failed: 6: unexpected error 193 loader.c:718: Test failed: 7: unexpected error 193 loader.c:173: Test failed: WriteFile error 1784 === w7pro64 (64 bit loader) === loader.c:718: Test failed: 2: unexpected error 193 loader.c:718: Test failed: 3: unexpected error 193 loader.c:718: Test failed: 4: unexpected error 193 loader.c:718: Test failed: 6: unexpected error 193 loader.c:718: Test failed: 7: unexpected error 193 loader.c:173: Test failed: WriteFile error 1784 === w864 (64 bit loader) === loader.c:718: Test failed: 2: unexpected error 193 loader.c:718: Test failed: 3: unexpected error 193 loader.c:718: Test failed: 4: unexpected error 193 loader.c:718: Test failed: 6: unexpected error 193 loader.c:718: Test failed: 7: unexpected error 193 loader.c:173: Test failed: WriteFile error 1784 === w1064 (64 bit loader) === loader.c:718: Test failed: 2: unexpected error 193 loader.c:718: Test failed: 3: unexpected error 193 loader.c:718: Test failed: 4: unexpected error 193 loader.c:718: Test failed: 6: unexpected error 193 loader.c:718: Test failed: 7: unexpected error 193 loader.c:173: Test failed: WriteFile error 1784

1 0

Re: [PATCH 1/2] ddraw/tests: Skip overlay tests if DWM is on.
by Marvin Aug. 7, 2016

Aug. 7, 2016

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=24816 Your paranoid android. === w1064 (32 bit ddraw2) === ddraw2.c:2357: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw2) === ddraw2.c:2357: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (32 bit ddraw7) === ddraw7.c:2272: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw7) === ddraw7.c:2272: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (32 bit ddraw1) === ddraw1.c:2206: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw1) === ddraw1.c:2206: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (32 bit ddraw4) === ddraw4.c:2542: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw4) === ddraw4.c:2542: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768).

1 0

WineHQ upgrade breaking dinput8:dinput?
by Francois Gouget Aug. 7, 2016

Aug. 7, 2016

This is a truly weird one. dinput8:dinput start failing on the 17th, the day after WineHQ was upgraded: https://test.winehq.org/data/tests/dinput8:dinput.html What makes this weird is that: * There has been no change to these tests between bd332f53f2d7 and 966a07149a02. In fact the last patch goes back to 2013. So it's not a bad patch. * The failures happen on every 32 bit test environment, from XP to Windows 10, VM or not, TestBot or not. Only 64 bit is spared. * I compiled a dinput8_crosstest.exe binary bd332f53f2d7 which was not failing according to the test.winehq.org results. But all I got is failures: - 887b445bb878, 8th, 0 test.winehq.org failures, 13 here: https://testbot.winehq.org/JobDetails.pl?Key=21611 - bd332f53f2d7, 16th, 0 test.winehq.org failures, 13 here: https://testbot.winehq.org/JobDetails.pl?Key=21610 - 329dfee70c35, 22nd, 13 test.winehq.org failures, 13 here https://testbot.winehq.org/JobDetails.pl?Key=21609 My MinGW compiler is more recent than the one WineHQ was using before the upgrade however. So I think this points to a compiler-related issue. The official WineTest binaries are build on WineHQ so I suspect the MinGW upgrade is somehow responsible for the new failures. However I don't know if the old compiler was masking them, of if the new one is causing them. Any ideas? For reference: * Debian Testing (my environment): $ i686-w64-mingw32-gcc --version i686-w64-mingw32-gcc (GCC) 5.3.1 20160205 * Debian 7 (close to former WineHQ environment): $ i686-w64-mingw32-gcc --version i686-w64-mingw32-gcc (GCC) 4.9.1 -- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ Nouvelle version : les anciens bogues ont été remplacés par de nouveaux.

5 15

Re: [PATCH 2/2] d3drm: Implement IDirect3DRMViewport*::Clear.
by Stefan Dösinger Aug. 7, 2016

Aug. 7, 2016

On 2016-08-06 16:29, Aaryaman Vasishta wrote: > Signed-off-by: Aaryaman Vasishta <jem456.vasishta(a)gmail.com> > --- > dlls/d3drm/tests/d3drm.c | 640 +++++++++++++++++++++++++++++++++++++++++++++++ > dlls/d3drm/viewport.c | 71 +++++- > 2 files changed, 704 insertions(+), 7 deletions(-) > > diff --git a/dlls/d3drm/tests/d3drm.c b/dlls/d3drm/tests/d3drm.c > index c4020e4..b9a83a0 100644 > --- a/dlls/d3drm/tests/d3drm.c > +++ b/dlls/d3drm/tests/d3drm.c > @@ -5852,6 +5852,644 @@ static void test_viewport_qi(void) > IDirect3DRM_Release(d3drm1); > } > > +static D3DCOLOR get_surface_color(IDirectDrawSurface *surface, UINT x, UINT y) > +{ > + RECT rect = { x, y, x + 1, y + 1 }; > + DDSURFACEDESC surface_desc; > + D3DCOLOR color; > + HRESULT hr; > + > + memset(&surface_desc, 0, sizeof(surface_desc)); > + surface_desc.dwSize = sizeof(surface_desc); > + > + hr = IDirectDrawSurface_Lock(surface, &rect, &surface_desc, DDLOCK_READONLY | DDLOCK_WAIT, NULL); > + ok(SUCCEEDED(hr), "Failed to lock surface, hr %#x.\n", hr); > + if (FAILED(hr)) > + return 0xdeadbeef; > + > + color = *((DWORD *)surface_desc.lpSurface) & 0x00ffffff; > + > + hr = IDirectDrawSurface_Unlock(surface, NULL); > + ok(SUCCEEDED(hr), "Failed to unlock surface, hr %#x.\n", hr); > + > + return color; > +} > + > +static BOOL compare_color(D3DCOLOR c1, D3DCOLOR c2, BYTE max_diff) > +{ > + if ((c1 & 0xff) - (c2 & 0xff) > max_diff) return FALSE; > + c1 >>= 8; c2 >>= 8; > + if ((c1 & 0xff) - (c2 & 0xff) > max_diff) return FALSE; > + c1 >>= 8; c2 >>= 8; > + if ((c1 & 0xff) - (c2 & 0xff) > max_diff) return FALSE; > + c1 >>= 8; c2 >>= 8; > + if ((c1 & 0xff) - (c2 & 0xff) > max_diff) return FALSE; > + return TRUE; > +} > + > +static void clear_depth_surface(IDirectDrawSurface *surface, DWORD value) > +{ > + HRESULT hr; > + DDBLTFX fx; > + > + memset(&fx, 0, sizeof(fx)); > + fx.dwSize = sizeof(fx); > + U5(fx).dwFillDepth = value; > + > + hr = IDirectDrawSurface_Blt(surface, NULL, NULL, NULL, DDBLT_DEPTHFILL | DDBLT_WAIT, &fx); > + ok(SUCCEEDED(hr), "Got unexpected hr %#x.\n", hr); > +} > + > +static void set_execute_data(IDirect3DExecuteBuffer *execute_buffer, UINT vertex_count, UINT offset, UINT len) > +{ > + D3DEXECUTEDATA exec_data; > + HRESULT hr; > + > + memset(&exec_data, 0, sizeof(exec_data)); > + exec_data.dwSize = sizeof(exec_data); > + exec_data.dwVertexCount = vertex_count; > + exec_data.dwInstructionOffset = offset; > + exec_data.dwInstructionLength = len; > + hr = IDirect3DExecuteBuffer_SetExecuteData(execute_buffer, &exec_data); > + ok(SUCCEEDED(hr), "Failed to set execute data, hr %#x.\n", hr); > +} > + > +static void emit_set_ts(void **ptr, D3DTRANSFORMSTATETYPE state, DWORD value) > +{ > + D3DINSTRUCTION *inst = *ptr; > + D3DSTATE *ts = (D3DSTATE *)(inst + 1); > + > + inst->bOpcode = D3DOP_STATETRANSFORM; > + inst->bSize = sizeof(*ts); > + inst->wCount = 1; > + > + U1(*ts).dtstTransformStateType = state; > + U2(*ts).dwArg[0] = value; > + > + *ptr = ts + 1; > +} > + > +static void emit_set_rs(void **ptr, D3DRENDERSTATETYPE state, DWORD value) > +{ > + D3DINSTRUCTION *inst = *ptr; > + D3DSTATE *rs = (D3DSTATE *)(inst + 1); > + > + inst->bOpcode = D3DOP_STATERENDER; > + inst->bSize = sizeof(*rs); > + inst->wCount = 1; > + > + U1(*rs).drstRenderStateType = state; > + U2(*rs).dwArg[0] = value; > + > + *ptr = rs + 1; > +} > + > +static void emit_process_vertices(void **ptr, DWORD flags, WORD base_idx, DWORD vertex_count) > +{ > + D3DINSTRUCTION *inst = *ptr; > + D3DPROCESSVERTICES *pv = (D3DPROCESSVERTICES *)(inst + 1); > + > + inst->bOpcode = D3DOP_PROCESSVERTICES; > + inst->bSize = sizeof(*pv); > + inst->wCount = 1; > + > + pv->dwFlags = flags; > + pv->wStart = base_idx; > + pv->wDest = 0; > + pv->dwCount = vertex_count; > + pv->dwReserved = 0; > + > + *ptr = pv + 1; > +} > + > +static void emit_tquad(void **ptr, WORD base_idx) > +{ > + D3DINSTRUCTION *inst = *ptr; > + D3DTRIANGLE *tri = (D3DTRIANGLE *)(inst + 1); > + > + inst->bOpcode = D3DOP_TRIANGLE; > + inst->bSize = sizeof(*tri); > + inst->wCount = 2; > + > + U1(*tri).v1 = base_idx; > + U2(*tri).v2 = base_idx + 1; > + U3(*tri).v3 = base_idx + 2; > + tri->wFlags = D3DTRIFLAG_START; > + ++tri; > + > + U1(*tri).v1 = base_idx + 2; > + U2(*tri).v2 = base_idx + 1; > + U3(*tri).v3 = base_idx + 3; > + tri->wFlags = D3DTRIFLAG_ODD; > + ++tri; > + > + *ptr = tri; > +} > + > +static void emit_end(void **ptr) > +{ > + D3DINSTRUCTION *inst = *ptr; > + > + inst->bOpcode = D3DOP_EXIT; > + inst->bSize = 0; > + inst->wCount = 0; > + > + *ptr = inst + 1; > +} > + > +static void d3d_draw_quad1(IDirect3DDevice *device, IDirect3DViewport *viewport) > +{ > + IDirect3DExecuteBuffer *execute_buffer; > + D3DEXECUTEBUFFERDESC exec_desc; > + HRESULT hr; > + void *ptr; > + UINT inst_length; > + D3DMATRIXHANDLE world_handle, view_handle, proj_handle; > + static D3DMATRIX mat = > + { > + 1.0f, 0.0f, 0.0f, 0.0f, > + 0.0f, 1.0f, 0.0f, 0.0f, > + 0.0f, 0.0f, 1.0f, 0.0f, > + 0.0f, 0.0f, 0.0f, 1.0f, > + }; > + static const D3DLVERTEX quad_strip[] = > + { > + {{-1.0f}, {-1.0f}, {0.80f}, 0, {0xffbada55}}, > + {{-1.0f}, { 1.0f}, {0.80f}, 0, {0xffbada55}}, > + {{ 1.0f}, {-1.0f}, {0.80f}, 0, {0xffbada55}}, > + {{ 1.0f}, { 1.0f}, {0.80f}, 0, {0xffbada55}}, > + }; You could make this {{-1.0f}, {-1.0f}, {0.0f}, 0, {0xffbada55}}, {{-1.0f}, { 1.0f}, {0.0f}, 0, {0xffbada55}}, {{ 1.0f}, {-1.0f}, {1.0f}, 0, {0xffbada55}}, {{ 1.0f}, { 1.0f}, {1.0f}, 0, {0xffbada55}}, Then your currently discarded draws will draw to ca x = 250. That way you can make sure the draw is really discarded by the depth test and not some other reason. > + /* Check what happens if we release the depth surface that d3drm created, and clear the viewport */ > ... > + hr = IDirect3DRMViewport_Clear(viewport1); > + ok(SUCCEEDED(hr), "Cannot clear viewport (hr = %#x).\n", hr); It's worth checking the color surface here to see if the clear was completely ignored or if it realized that there's no depth surface and it only cleared color. Also clear with a different color than what's left behind by the previous test. > ... > + d3d_draw_quad1(d3d_device1, d3d_viewport); > + > + ret_color = get_surface_color(surface, 100, 200); > + todo_wine ok(compare_color(ret_color, 0x000000ff, 1), "Got unexpected color 0x%08x.\n", ret_color); My reading of this is that the depth surface wasn't cleared (everything else would be very surprising). If the color surface was indeed cleared this shows that native at some place expects your ugly deed of removing the depth surface. If the color is not cleared it and the clear call is completely ignored I'd say something is broken somewhere and we shouldn't care too much about this corner case. > + hr = IDirect3DRM_CreateViewport(d3drm1, device1, dummy_frame1, 0, 0, rc.right, > + rc.bottom, &viewport1); > + ok(SUCCEEDED(hr), "Cannot get IDirect3DRMViewport interface (hr = %#x)\n", hr); > + > + > + /* Clear uses the scene/root frame's background color. */ Two newlines. Is that intentional? > + hr = IDirect3DRMFrame_SetSceneBackgroundRGB(frame1, 1.0f, 1.0f, 1.0f); > + ok(SUCCEEDED(hr), "Cannot set scene background RGB (hr = %#x)\n", hr); > + ret_color = IDirect3DRMFrame_GetSceneBackground(frame1); > + ok(ret_color == 0xffffffff, "Expected scene color returned == 0xffffffff, got %#x.\n", ret_color); > + hr = IDirect3DRMFrame_SetSceneBackgroundRGB(dummy_frame1, 0.0f, 1.0f, 0.0f); > + ok(SUCCEEDED(hr), "Cannot set scene background RGB (hr = %#x)\n", hr); > + ret_color = IDirect3DRMFrame_GetSceneBackground(dummy_frame1); > + ok(ret_color == 0xff00ff00, "Expected scene color returned == 0xff00ff00, got %#x.\n", ret_color); > + hr = IDirect3DRMFrame_SetSceneBackgroundRGB(dummy_frame1, 0.0f, 0.0f, 1.0f); > + ok(SUCCEEDED(hr), "Cannot set scene background RGB (hr = %#x)\n", hr); > + ret_color = IDirect3DRMFrame_GetSceneBackground(dummy_frame1); > + ok(ret_color == 0xff0000ff, "Expected scene color returned == 0xff0000ff, got %#x.\n", ret_color); Can't the "where does the color come from" test be merged into the clear test at the start? > +static void draw_quad2(IDirect3DDevice2 *device, IDirect3DViewport *viewport) > +{ Do you need to draw with IDirect3DDevice2 directly here? What about QI'ing IDirect3DDevice and calling d3d_draw_quad1? > diff --git a/dlls/d3drm/viewport.c b/dlls/d3drm/viewport.c > index cb57fe0..371aae3 100644 > --- a/dlls/d3drm/viewport.c > +++ b/dlls/d3drm/viewport.c > @@ -39,6 +39,42 @@ static inline struct d3drm_viewport *impl_from_IDirect3DRMViewport2(IDirect3DRMV > return CONTAINING_RECORD(iface, struct d3drm_viewport, IDirect3DRMViewport2_iface); > } > > +static inline void d3drm_normalize_d3d_color(D3DCOLORVALUE *color_value, D3DCOLOR color) > +{ > + color_value->r = RGBA_GETRED(color) / 255.0f; > + color_value->g = RGBA_GETGREEN(color) / 255.0f; > + color_value->b = RGBA_GETBLUE(color) / 255.0f; > + color_value->a = RGBA_GETALPHA(color) / 255.0f; > +} > + > +static HRESULT d3drm_update_background_material(struct d3drm_viewport *viewport) > +{ > + IDirect3DRMFrame *root_frame; > + D3DCOLOR color; > + D3DMATERIAL mat; > + D3DMATERIALHANDLE hmat; > + HRESULT hr; > + > + if (FAILED(hr = IDirect3DRMFrame_GetScene(viewport->camera, &root_frame))) > + return hr; > + color = IDirect3DRMFrame_GetSceneBackground(root_frame); You are leaking root_frame here. > + if (FAILED(hr = IDirect3DMaterial_GetHandle(viewport->material, viewport->device->device, &hmat))) > + return hr; > + > + if (FAILED(hr = IDirect3DViewport_SetBackground(viewport->d3d_viewport, hmat))) > + return hr; You can do these two calls during viewport creation, you don't have to repeat them every time. > @@ -344,10 +380,7 @@ static HRESULT WINAPI d3drm_viewport2_Init(IDirect3DRMViewport2 *iface, IDirect3 > > memset(&mat, 0, sizeof(mat)); > mat.dwSize = sizeof(mat); > - mat.diffuse.r = RGBA_GETRED(color) / 255.0f; > - mat.diffuse.g = RGBA_GETGREEN(color) / 255.0f; > - mat.diffuse.b = RGBA_GETBLUE(color) / 255.0f; > - mat.diffuse.a = RGBA_GETALPHA(color) / 255.0f; > + d3drm_normalize_d3d_color(&mat.diffuse, color); > > if (FAILED(hr = IDirect3DMaterial_SetMaterial(material, &mat))) > goto cleanup; You can remove the IDirect3DMaterial::SetMaterial call from Init because you have to do it every time clear is called anyway. Keep the GetHandle / SetBackground call in Init though to link the material with the viewport. > static HRESULT WINAPI d3drm_viewport2_Clear(IDirect3DRMViewport2 *iface, DWORD flags) > ... > + if (flags & D3DRMCLEAR_ZBUFFER) > + clear_flags |= D3DCLEAR_ZBUFFER; I guess somewhere here or in the IDirect3DViewport::Clear implementation in ddraw you'll have to handle your missing depth buffer case.

2 4

Re: [PATCH 1/2] d3drm: Implement IDirect3DRMFrame*::GetScene.
by Stefan Dösinger Aug. 6, 2016

Aug. 6, 2016

On 2016-08-06 16:29, Aaryaman Vasishta wrote: > + while (!done) > + { > + if (FAILED(hr = IDirect3DRMFrame3_GetParent(frame, &parent_frame))) > + return hr; > + if (parent_frame) > + { > + frame = parent_frame; > + IDirect3DRMFrame3_Release(parent_frame); > + } > + else > + done = TRUE; > + } What happens if GetScene is called on the root frame? Does it return itself or NULL or an error? You can eliminate the done variable from this loop by replacing it with a break: for (;;) { if (FAILED(hr = IDirect3DRMFrame3_GetParent(frame, &parent_frame))) return hr; if (parent_frame) { frame = parent_frame; IDirect3DRMFrame3_Release(parent_frame); } else break; } I originally thought about a do {} while(parent_frame); kind of construct but that doesn't gain you much because you need the if check anyway to release parent_frame. It's not nice to call GetParent in the next iteration after you release the frame, but so far I haven't come up with an implementation that avoids that and isn't terribly ugly. I'll keep thinking.

2 1

Re: winealsa.drv: Use debugstr_guid() to trace a guid.
by Andrew Eikum Aug. 6, 2016

Aug. 6, 2016

On Fri, Aug 05, 2016 at 03:55:55PM +0200, Michael Stefaniuc wrote: > Signed-off-by: Michael Stefaniuc <mstefani(a)redhat.de> > --- > dlls/winealsa.drv/mmdevdrv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/dlls/winealsa.drv/mmdevdrv.c b/dlls/winealsa.drv/mmdevdrv.c > index 14eac39..8577e75 100644 > --- a/dlls/winealsa.drv/mmdevdrv.c > +++ b/dlls/winealsa.drv/mmdevdrv.c > @@ -3390,7 +3390,7 @@ static HRESULT WINAPI SimpleAudioVolume_SetMute(ISimpleAudioVolume *iface, > AudioSessionWrapper *This = impl_from_ISimpleAudioVolume(iface); > AudioSession *session = This->session; > > - TRACE("(%p)->(%u, %p)\n", session, mute, context); > + TRACE("(%p)->(%u, %p)\n", session, mute, debugstr_guid(context)); > The format on these should be changed to %s, right? Andrew

2 1

Re: [PATCH] ntdll: Catch page fault in RtlWow64EnableFsRedirectionEx
by Nikolay Sivov Aug. 6, 2016

Aug. 6, 2016

On 04.08.2016 20:05, Matt Robinson wrote: > Previously, RtlWow64EnableFsRedirectionEx only returned > STATUS_NOT_IMPLEMENTED if the old_value pointer started with 0x0000, and > would cause an unhandled page fault if an invalid handle not starting > with 0x00 was passed. However, Windows seems to catch page faults when > accessing old_value and returns STATUS_ACCESS_VIOLATION. > > This adds a test to confirm the behaviour and then wraps the access to > old_value in an exception handler. > > Fixes https://bugs.winehq.org/show_bug.cgi?id=41066 > > Signed-off-by: Matt Robinson <git(a)nerdoftheherd.com> > --- > dlls/ntdll/directory.c | 12 +++++++++++- > dlls/ntdll/tests/directory.c | 2 ++ > 2 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/dlls/ntdll/directory.c b/dlls/ntdll/directory.c > index 4a34475..13df0d1 100644 > --- a/dlls/ntdll/directory.c > +++ b/dlls/ntdll/directory.c > @@ -106,6 +106,7 @@ > #include "wine/list.h" > #include "wine/library.h" > #include "wine/debug.h" > +#include "wine/exception.h" > > WINE_DEFAULT_DEBUG_CHANNEL(file); > > @@ -2998,7 +2999,16 @@ NTSTATUS WINAPI RtlWow64EnableFsRedirectionEx( ULONG disable, ULONG *old_value ) > if (!is_wow64) return STATUS_NOT_IMPLEMENTED; > if (((ULONG_PTR)old_value >> 16) == 0) return STATUS_ACCESS_VIOLATION; Looks like you can remove this old pointer check now. > > - *old_value = !ntdll_get_thread_data()->wow64_redir; > + __TRY > + { > + *old_value = !ntdll_get_thread_data()->wow64_redir; > + } > + __EXCEPT_PAGE_FAULT > + { > + return STATUS_ACCESS_VIOLATION; > + } > + __ENDTRY > + > ntdll_get_thread_data()->wow64_redir = !disable; > return STATUS_SUCCESS; > }

1 0

Re: kernel32/tests: Convert fiber back to thread even when IsThreadAFiber is not available.
by Nikolay Sivov Aug. 6, 2016

Aug. 6, 2016

On 06.08.2016 15:40, Bernhard Übelacker wrote: > Testbot shows the fiber test failing for Windows 2003 recently. > https://test.winehq.org/data/tests/kernel32:fiber.html > > This is probably related to the last change in > http://source.winehq.org/git/wine.git/commit/556fef3dd184e71e1f0345bd8f3725… > > As Windows 2003 seems to have no IsThreadAFiber the test never > calls test_ConvertFiberToThread. > This leads probably sometimes to the crash now visible. > > By calling test_ConvertFiberToThread before return the crash disappears > on Windows 2003. > > Signed-off-by: Bernhard Übelacker <bernhardu(a)mailbox.org> > --- > dlls/kernel32/tests/fiber.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/dlls/kernel32/tests/fiber.c b/dlls/kernel32/tests/fiber.c > index 9ff9366..0c989af 100644 > --- a/dlls/kernel32/tests/fiber.c > +++ b/dlls/kernel32/tests/fiber.c > @@ -158,6 +158,7 @@ static void test_FiberHandling(void) > if (!pCreateFiberEx) > { > win_skip( "CreateFiberEx not present\n" ); > + test_ConvertFiberToThread(); > return; > } > > @@ -171,6 +172,7 @@ static void test_FiberHandling(void) > if (!pIsThreadAFiber) > { > win_skip( "IsThreadAFiber not present\n" ); > + test_ConvertFiberToThread(); > return; > } > > It's better to have a helper for such cleanup, and not to call one test from another.

2 1

Re: [v2 4/8] msvcirt: Add a partial implementation of class istream_withassign.
by Piotr Caban Aug. 6, 2016

Aug. 6, 2016

On 08/05/16 18:56, Iván Matellanes wrote: > +/* ??0istream_withassign@@QAE(a)XZ */ > +/* ??0istream_withassign@@QEAA(a)XZ */ > +DEFINE_THISCALL_WRAPPER(istream_withassign_ctor, 12) > +istream* __thiscall istream_withassign_ctor(istream *this, BOOL virt_init) There should be 8 instead of 12. Thanks, Piotr

1 0