August 2018 - Wine-devel - list.winehq.org

[PATCH 1/2] comctl32/listview: Use mask to disable change notifications.
by Nikolay Sivov 26 Aug '18

26 Aug '18

From: Fabian Maurer <dark.shadow4(a)web.de> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/comctl32/listview.c | 48 +++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c index 200bf93be5..d463607e45 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -211,6 +211,12 @@ typedef struct tagDELAYED_ITEM_EDIT INT iItem; } DELAYED_ITEM_EDIT; +enum notification_mask +{ + NOTIFY_MASK_ITEM_CHANGE = 0x1, + NOTIFY_MASK_UNMASK_ALL = 0xffffffff +}; + typedef struct tagLISTVIEW_INFO { /* control window */ @@ -225,7 +231,7 @@ typedef struct tagLISTVIEW_INFO /* notification window */ SHORT notifyFormat; HWND hwndNotify; - BOOL bDoChangeNotify; /* send change notification messages? */ + DWORD notify_mask; UINT uCallbackMask; /* tooltips */ @@ -3507,13 +3513,13 @@ Parameters: */ static void LISTVIEW_ShiftFocus(LISTVIEW_INFO *infoPtr, INT focus, INT item, INT direction) { - BOOL old_change = infoPtr->bDoChangeNotify; + DWORD old_mask = infoPtr->notify_mask & NOTIFY_MASK_ITEM_CHANGE; - infoPtr->bDoChangeNotify = FALSE; + infoPtr->notify_mask &= ~NOTIFY_MASK_ITEM_CHANGE; focus = shift_item(infoPtr, focus, item, direction); if (focus != infoPtr->nFocusedItem) LISTVIEW_SetItemFocus(infoPtr, focus); - infoPtr->bDoChangeNotify = old_change; + infoPtr->notify_mask |= old_mask; } /** @@ -3556,8 +3562,8 @@ static BOOL LISTVIEW_AddGroupSelection(LISTVIEW_INFO *infoPtr, INT nItem) INT nLast = max(infoPtr->nSelectionMark, nItem); HWND hwndSelf = infoPtr->hwndSelf; NMLVODSTATECHANGE nmlv; + DWORD old_mask; LVITEMW item; - BOOL bOldChange; INT i; /* Temporarily disable change notification @@ -3565,8 +3571,9 @@ static BOOL LISTVIEW_AddGroupSelection(LISTVIEW_INFO *infoPtr, INT nItem) * only one LVN_ODSTATECHANGED notification. * See MSDN documentation for LVN_ITEMCHANGED. */ - bOldChange = infoPtr->bDoChangeNotify; - if (infoPtr->dwStyle & LVS_OWNERDATA) infoPtr->bDoChangeNotify = FALSE; + old_mask = infoPtr->notify_mask & NOTIFY_MASK_ITEM_CHANGE; + if (infoPtr->dwStyle & LVS_OWNERDATA) + infoPtr->notify_mask &= ~NOTIFY_MASK_ITEM_CHANGE; if (nFirst == -1) nFirst = nItem; @@ -3585,7 +3592,7 @@ static BOOL LISTVIEW_AddGroupSelection(LISTVIEW_INFO *infoPtr, INT nItem) notify_hdr(infoPtr, LVN_ODSTATECHANGED, (LPNMHDR)&nmlv); if (!IsWindow(hwndSelf)) return FALSE; - infoPtr->bDoChangeNotify = bOldChange; + infoPtr->notify_mask |= old_mask; return TRUE; } @@ -3604,9 +3611,9 @@ static BOOL LISTVIEW_AddGroupSelection(LISTVIEW_INFO *infoPtr, INT nItem) static void LISTVIEW_SetGroupSelection(LISTVIEW_INFO *infoPtr, INT nItem) { RANGES selection; + DWORD old_mask; LVITEMW item; ITERATOR i; - BOOL bOldChange; if (!(selection = ranges_create(100))) return; @@ -3656,8 +3663,9 @@ static void LISTVIEW_SetGroupSelection(LISTVIEW_INFO *infoPtr, INT nItem) /* disable per item notifications on LVS_OWNERDATA style FIXME: single LVN_ODSTATECHANGED should be used */ - bOldChange = infoPtr->bDoChangeNotify; - if (infoPtr->dwStyle & LVS_OWNERDATA) infoPtr->bDoChangeNotify = FALSE; + old_mask = infoPtr->notify_mask & NOTIFY_MASK_ITEM_CHANGE; + if (infoPtr->dwStyle & LVS_OWNERDATA) + infoPtr->notify_mask &= ~NOTIFY_MASK_ITEM_CHANGE; LISTVIEW_DeselectAllSkipItems(infoPtr, selection); @@ -3668,8 +3676,7 @@ static void LISTVIEW_SetGroupSelection(LISTVIEW_INFO *infoPtr, INT nItem) /* this will also destroy the selection */ iterator_destroy(&i); - infoPtr->bDoChangeNotify = bOldChange; - + infoPtr->notify_mask |= old_mask; LISTVIEW_SetItemFocus(infoPtr, nItem); } @@ -4257,7 +4264,7 @@ static BOOL set_main_item(LISTVIEW_INFO *infoPtr, const LVITEMW *lpLVItem, BOOL and we are _NOT_ virtual (LVS_OWNERDATA), and change notifications are enabled. Even nothing really changed we still need to send this, in this case uChanged mask is just set to passed item mask. */ - if(lpItem && !isNew && infoPtr->bDoChangeNotify) + if (lpItem && !isNew && (infoPtr->notify_mask & NOTIFY_MASK_ITEM_CHANGE)) { HWND hwndSelf = infoPtr->hwndSelf; @@ -4350,7 +4357,8 @@ static BOOL set_main_item(LISTVIEW_INFO *infoPtr, const LVITEMW *lpLVItem, BOOL /* send LVN_ITEMCHANGED notification */ if (lpLVItem->mask & LVIF_PARAM) nmlv.lParam = lpLVItem->lParam; - if (infoPtr->bDoChangeNotify) notify_listview(infoPtr, LVN_ITEMCHANGED, &nmlv); + if (infoPtr->notify_mask & NOTIFY_MASK_ITEM_CHANGE) + notify_listview(infoPtr, LVN_ITEMCHANGED, &nmlv); return TRUE; } @@ -8973,7 +8981,7 @@ static BOOL LISTVIEW_SetItemState(LISTVIEW_INFO *infoPtr, INT nItem, const LVITE if (nItem == -1) { UINT oldstate = 0; - BOOL notify; + DWORD old_mask; /* special case optimization for recurring attempt to deselect all */ if (lvItem.state == 0 && lvItem.stateMask == LVIS_SELECTED && !LISTVIEW_GetSelectedCount(infoPtr)) @@ -8986,10 +8994,10 @@ static BOOL LISTVIEW_SetItemState(LISTVIEW_INFO *infoPtr, INT nItem, const LVITE /* focus all isn't allowed */ if (lvItem.state & lvItem.stateMask & LVIS_FOCUSED) return FALSE; - notify = infoPtr->bDoChangeNotify; + old_mask = infoPtr->notify_mask & NOTIFY_MASK_ITEM_CHANGE; if (infoPtr->dwStyle & LVS_OWNERDATA) { - infoPtr->bDoChangeNotify = FALSE; + infoPtr->notify_mask &= ~NOTIFY_MASK_ITEM_CHANGE; if (!(lvItem.state & LVIS_SELECTED) && LISTVIEW_GetSelectedCount(infoPtr)) oldstate |= LVIS_SELECTED; if (infoPtr->nFocusedItem != -1) oldstate |= LVIS_FOCUSED; @@ -9003,7 +9011,7 @@ static BOOL LISTVIEW_SetItemState(LISTVIEW_INFO *infoPtr, INT nItem, const LVITE { NMLISTVIEW nmlv; - infoPtr->bDoChangeNotify = notify; + infoPtr->notify_mask |= old_mask; nmlv.iItem = -1; nmlv.iSubItem = 0; @@ -9484,7 +9492,7 @@ static LRESULT LISTVIEW_NCCreate(HWND hwnd, WPARAM wParam, const CREATESTRUCTW * infoPtr->nHotItem = -1; infoPtr->redraw = TRUE; infoPtr->bNoItemMetrics = TRUE; - infoPtr->bDoChangeNotify = TRUE; + infoPtr->notify_mask = NOTIFY_MASK_UNMASK_ALL; infoPtr->autoSpacing = TRUE; infoPtr->iconSpacing.cx = GetSystemMetrics(SM_CXICONSPACING) - GetSystemMetrics(SM_CXICON); infoPtr->iconSpacing.cy = GetSystemMetrics(SM_CYICONSPACING) - GetSystemMetrics(SM_CYICON); -- 2.18.0

1 1

[PATCH] ntdll/tests: Add Nt[WaitFor|Release]KeyedEvent test with NULL handle.
by Gijs Vermeulen 26 Aug '18

26 Aug '18

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=44620 Signed-off-by: Gijs Vermeulen <gijsvrm(a)gmail.com> --- dlls/ntdll/tests/om.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/dlls/ntdll/tests/om.c b/dlls/ntdll/tests/om.c index 52b4a83968..c2b40cb69c 100644 --- a/dlls/ntdll/tests/om.c +++ b/dlls/ntdll/tests/om.c @@ -1677,6 +1677,13 @@ static void test_keyed_events(void) status = pNtReleaseKeyedEvent( handle, NULL, 0, &timeout ); ok( status == STATUS_TIMEOUT, "NtReleaseKeyedEvent %x\n", status ); + status = pNtWaitForKeyedEvent( NULL, (void *)8, 0, &timeout ); + todo_wine ok( status == STATUS_TIMEOUT || broken(status == STATUS_INVALID_HANDLE), /* XP/2003 */ + "NtWaitForKeyedEvent %x\n", status ); + status = pNtReleaseKeyedEvent( NULL, (void *)8, 0, &timeout ); + todo_wine ok( status == STATUS_TIMEOUT || broken(status == STATUS_INVALID_HANDLE), /* XP/2003 */ + "NtReleaseKeyedEvent %x\n", status ); + status = pNtWaitForKeyedEvent( (HANDLE)0xdeadbeef, (void *)9, 0, &timeout ); ok( status == STATUS_INVALID_PARAMETER_1, "NtWaitForKeyedEvent %x\n", status ); status = pNtReleaseKeyedEvent( (HANDLE)0xdeadbeef, (void *)9, 0, &timeout ); -- 2.15.2 (Apple Git-101.1)

1 0

[PATCH 3/3] comctl32/pager: Support WM_NOTIFYFORMAT message.
by Zhiyi Zhang 26 Aug '18

26 Aug '18

Signed-off-by: Zhiyi Zhang <zzhang(a)codeweavers.com> --- dlls/comctl32/pager.c | 25 +++++++++++++++++++++++++ dlls/comctl32/tests/pager.c | 8 ++++---- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/dlls/comctl32/pager.c b/dlls/comctl32/pager.c index fe68515e67..de63cad717 100644 --- a/dlls/comctl32/pager.c +++ b/dlls/comctl32/pager.c @@ -71,6 +71,7 @@ typedef struct HWND hwndSelf; /* handle of the control wnd */ HWND hwndChild; /* handle of the contained wnd */ HWND hwndNotify; /* handle of the parent wnd */ + BOOL bUnicode; /* send notifications in Unicode */ DWORD dwStyle; /* styles for this control */ COLORREF clrBk; /* background color */ INT nBorder; /* border size for the control */ @@ -555,6 +556,7 @@ static LRESULT PAGER_Create (HWND hwnd, const CREATESTRUCTW *lpcs) { PAGER_INFO *infoPtr; + INT ret; /* allocate memory for info structure */ infoPtr = heap_alloc_zero (sizeof(*infoPtr)); @@ -581,6 +583,9 @@ PAGER_Create (HWND hwnd, const CREATESTRUCTW *lpcs) if (infoPtr->dwStyle & PGS_DRAGNDROP) FIXME("[%p] Drag and Drop style is not implemented yet.\n", infoPtr->hwndSelf); + ret = SendMessageW(infoPtr->hwndNotify, WM_NOTIFYFORMAT, (WPARAM)infoPtr->hwndSelf, NF_QUERY); + infoPtr->bUnicode = (ret == NFR_UNICODE); + return 0; } @@ -998,6 +1003,23 @@ PAGER_StyleChanged(PAGER_INFO *infoPtr, WPARAM wStyleType, const STYLESTRUCT *lp return 0; } +static LRESULT PAGER_NotifyFormat(PAGER_INFO *infoPtr, INT command) +{ + INT ret; + switch (command) + { + case NF_REQUERY: + ret = SendMessageW(infoPtr->hwndNotify, WM_NOTIFYFORMAT, (WPARAM)infoPtr->hwndSelf, NF_QUERY); + infoPtr->bUnicode = (ret == NFR_UNICODE); + return ret; + case NF_QUERY: + /* Pager always wants Unicode notifications from children */ + return NFR_UNICODE; + default: + return 0; + } +} + static LRESULT WINAPI PAGER_WindowProc (HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam) { @@ -1089,6 +1111,9 @@ PAGER_WindowProc (HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam) case WM_TIMER: return PAGER_Timer (infoPtr, (INT)wParam); + case WM_NOTIFYFORMAT: + return PAGER_NotifyFormat (infoPtr, lParam); + case WM_NOTIFY: case WM_COMMAND: return SendMessageW (infoPtr->hwndNotify, uMsg, wParam, lParam); diff --git a/dlls/comctl32/tests/pager.c b/dlls/comctl32/tests/pager.c index f2ac8b299a..a1f6683a57 100644 --- a/dlls/comctl32/tests/pager.c +++ b/dlls/comctl32/tests/pager.c @@ -392,8 +392,8 @@ static void test_wm_notifyformat(void) /* Test parent */ notify_query_received = FALSE; ret = SendMessageW(pager, WM_NOTIFYFORMAT, (WPARAM)parent, NF_REQUERY); - todo_wine_if(notify_format == NFR_ANSI) ok(ret == notify_format, "Expect %d, got %ld\n", notify_format, ret); - todo_wine ok(notify_query_received, "Didn't receive notify\n"); + ok(ret == notify_format, "Expect %d, got %ld\n", notify_format, ret); + ok(notify_query_received, "Didn't receive notify\n"); /* Send NF_QUERY directly to parent */ notify_query_received = FALSE; @@ -404,8 +404,8 @@ static void test_wm_notifyformat(void) /* Pager send notifications to its parent regardless of wParam */ notify_query_received = FALSE; ret = SendMessageW(pager, WM_NOTIFYFORMAT, (WPARAM)parent_wnd, NF_REQUERY); - todo_wine_if(notify_format == NFR_ANSI) ok(ret == notify_format, "Expect %d, got %ld\n", notify_format, ret); - todo_wine ok(notify_query_received, "Didn't receive notify\n"); + ok(ret == notify_format, "Expect %d, got %ld\n", notify_format, ret); + ok(notify_query_received, "Didn't receive notify\n"); /* Pager always wants Unicode notifications from children */ ret = SendMessageW(child, WM_NOTIFYFORMAT, (WPARAM)pager, NF_REQUERY); -- 2.18.0

2 1

[PATCH 2/3] comctl32/tests: Add WM_NOTIFYFORMAT tests for pager.
by Zhiyi Zhang 26 Aug '18

26 Aug '18

Signed-off-by: Zhiyi Zhang <zzhang(a)codeweavers.com> --- dlls/comctl32/tests/pager.c | 90 +++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/dlls/comctl32/tests/pager.c b/dlls/comctl32/tests/pager.c index fa505dce68..f2ac8b299a 100644 --- a/dlls/comctl32/tests/pager.c +++ b/dlls/comctl32/tests/pager.c @@ -28,6 +28,8 @@ #define PAGER_SEQ_INDEX 0 static HWND parent_wnd, child1_wnd, child2_wnd; +static INT notify_format; +static BOOL notify_query_received; #define CHILD1_ID 1 #define CHILD2_ID 2 @@ -330,6 +332,93 @@ static void test_pager(void) DestroyWindow( pager ); } +static LRESULT WINAPI test_notifyformat_proc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) +{ + switch (message) + { + case WM_NOTIFYFORMAT: + if (lParam == NF_QUERY) + { + notify_query_received = TRUE; + return notify_format; + } + else if (lParam == NF_REQUERY) + return SendMessageA(GetParent(hwnd), WM_NOTIFYFORMAT, (WPARAM)hwnd, NF_QUERY); + else + return 0; + default: + return notify_format == NFR_UNICODE ? DefWindowProcW(hwnd, message, wParam, lParam) + : DefWindowProcA(hwnd, message, wParam, lParam); + } +} + +static BOOL register_notifyformat_class(void) +{ + static const WCHAR class_w[] = {'P', 'a', 'g', 'e', 'r', ' ', 'n', 'o', 't', 'i', 'f', 'y', 'f', + 'o', 'r', 'm', 'a', 't', ' ', 'c', 'l', 'a', 's', 's', 0}; + WNDCLASSW cls = {0}; + + cls.lpfnWndProc = test_notifyformat_proc; + cls.hInstance = GetModuleHandleW(NULL); + cls.lpszClassName = class_w; + return RegisterClassW(&cls); +} + +static void test_wm_notifyformat(void) +{ + static const WCHAR class_w[] = {'P', 'a', 'g', 'e', 'r', ' ', 'n', 'o', 't', 'i', 'f', 'y', 'f', + 'o', 'r', 'm', 'a', 't', ' ', 'c', 'l', 'a', 's', 's', 0}; + static const WCHAR parent_w[] = {'p', 'a', 'r', 'e', 'n', 't', 0}; + static const WCHAR pager_w[] = {'p', 'a', 'g', 'e', 'r', 0}; + static const WCHAR child_w[] = {'c', 'h', 'i', 'l', 'd', 0}; + static const INT formats[] = {NFR_UNICODE, NFR_ANSI}; + HWND parent, pager, child; + LRESULT ret; + INT i; + + ok(register_notifyformat_class(), "Register test class failed, error 0x%08x\n", GetLastError()); + + for (i = 0; i < ARRAY_SIZE(formats); i++) + { + notify_format = formats[i]; + parent = CreateWindowW(class_w, parent_w, WS_OVERLAPPED, 0, 0, 100, 100, 0, 0, GetModuleHandleW(0), 0); + ok(parent != NULL, "CreateWindow failed\n"); + pager = CreateWindowW(WC_PAGESCROLLERW, pager_w, WS_CHILD, 0, 0, 100, 100, parent, 0, GetModuleHandleW(0), 0); + ok(pager != NULL, "CreateWindow failed\n"); + child = CreateWindowW(class_w, child_w, WS_CHILD, 0, 0, 100, 100, pager, 0, GetModuleHandleW(0), 0); + ok(child != NULL, "CreateWindow failed\n"); + SendMessageW(pager, PGM_SETCHILD, 0, (LPARAM)child); + + /* Test parent */ + notify_query_received = FALSE; + ret = SendMessageW(pager, WM_NOTIFYFORMAT, (WPARAM)parent, NF_REQUERY); + todo_wine_if(notify_format == NFR_ANSI) ok(ret == notify_format, "Expect %d, got %ld\n", notify_format, ret); + todo_wine ok(notify_query_received, "Didn't receive notify\n"); + + /* Send NF_QUERY directly to parent */ + notify_query_received = FALSE; + ret = SendMessageW(parent, WM_NOTIFYFORMAT, (WPARAM)pager, NF_QUERY); + ok(ret == notify_format, "Expect %d, got %ld\n", notify_format, ret); + ok(notify_query_received, "Didn't receive notify\n"); + + /* Pager send notifications to its parent regardless of wParam */ + notify_query_received = FALSE; + ret = SendMessageW(pager, WM_NOTIFYFORMAT, (WPARAM)parent_wnd, NF_REQUERY); + todo_wine_if(notify_format == NFR_ANSI) ok(ret == notify_format, "Expect %d, got %ld\n", notify_format, ret); + todo_wine ok(notify_query_received, "Didn't receive notify\n"); + + /* Pager always wants Unicode notifications from children */ + ret = SendMessageW(child, WM_NOTIFYFORMAT, (WPARAM)pager, NF_REQUERY); + ok(ret == NFR_UNICODE, "Expect %d, got %ld\n", NFR_UNICODE, ret); + ret = SendMessageW(pager, WM_NOTIFYFORMAT, (WPARAM)child, NF_QUERY); + ok(ret == NFR_UNICODE, "Expect %d, got %ld\n", NFR_UNICODE, ret); + + DestroyWindow(parent); + } + + UnregisterClassW(class_w, GetModuleHandleW(NULL)); +} + static void init_functions(void) { HMODULE mod = LoadLibraryA("comctl32.dll"); @@ -357,6 +446,7 @@ START_TEST(pager) ok(parent_wnd != NULL, "Failed to create parent window!\n"); test_pager(); + test_wm_notifyformat(); DestroyWindow(parent_wnd); } -- 2.18.0

2 1

[PATCH 1/3] comctl32/tests: Remove pager skip check.
by Zhiyi Zhang 26 Aug '18

26 Aug '18

Pager should be available on all VMs(XP+) on TestBot. So there's no need to check for the availability of pager control. Add calling InitCommonControls() so that pager control can be correctly initialized on XP. Signed-off-by: Zhiyi Zhang <zzhang(a)codeweavers.com> --- dlls/comctl32/tests/pager.c | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/dlls/comctl32/tests/pager.c b/dlls/comctl32/tests/pager.c index 0da396cdd1..fa505dce68 100644 --- a/dlls/comctl32/tests/pager.c +++ b/dlls/comctl32/tests/pager.c @@ -32,6 +32,7 @@ static HWND parent_wnd, child1_wnd, child2_wnd; #define CHILD1_ID 1 #define CHILD2_ID 2 +static BOOL (WINAPI *pInitCommonControlsEx)(const INITCOMMONCONTROLSEX*); static BOOL (WINAPI *pSetWindowSubclass)(HWND, SUBCLASSPROC, UINT_PTR, DWORD_PTR); static struct msg_sequence *sequences[NUM_MSG_SEQUENCES]; @@ -257,11 +258,7 @@ static void test_pager(void) RECT rect, rect2; pager = create_pager_control( PGS_HORZ ); - if (!pager) - { - win_skip( "Pager control not supported\n" ); - return; - } + ok(pager != NULL, "Fail to create pager\n"); register_child_wnd_class(); @@ -333,11 +330,26 @@ static void test_pager(void) DestroyWindow( pager ); } -START_TEST(pager) +static void init_functions(void) { - HMODULE mod = GetModuleHandleA("comctl32.dll"); + HMODULE mod = LoadLibraryA("comctl32.dll"); + +#define X(f) p##f = (void*)GetProcAddress(mod, #f); + X(InitCommonControlsEx); +#undef X pSetWindowSubclass = (void*)GetProcAddress(mod, (LPSTR)410); +} + +START_TEST(pager) +{ + INITCOMMONCONTROLSEX iccex; + + init_functions(); + + iccex.dwSize = sizeof(iccex); + iccex.dwICC = ICC_PAGESCROLLER_CLASS; + pInitCommonControlsEx(&iccex); init_msg_sequences(sequences, NUM_MSG_SEQUENCES); @@ -345,4 +357,6 @@ START_TEST(pager) ok(parent_wnd != NULL, "Failed to create parent window!\n"); test_pager(); + + DestroyWindow(parent_wnd); } -- 2.18.0

2 1

[PATCH 1/2] d3drm: Partially implement IDirect3DRMTexture*::InitFromFile. (v6)
by Aaryaman Vasishta 26 Aug '18

26 Aug '18

v6: Rebased on top of master as of this patch date v5: Redundant newline removed. Signed-off-by: Aaryaman Vasishta <jem456.vasishta(a)gmail.com> --- dlls/d3drm/d3drm_private.h | 1 + dlls/d3drm/tests/d3drm.c | 79 ++++++++- dlls/d3drm/texture.c | 325 ++++++++++++++++++++++++++++++++++++- 3 files changed, 393 insertions(+), 12 deletions(-) diff --git a/dlls/d3drm/d3drm_private.h b/dlls/d3drm/d3drm_private.h index 858911f350..333b809896 100644 --- a/dlls/d3drm/d3drm_private.h +++ b/dlls/d3drm/d3drm_private.h @@ -26,6 +26,7 @@ #define COBJMACROS #include <assert.h> #include <math.h> +#include <limits.h> #include "dxfile.h" #include "d3drmwin.h" #include "rmxfguid.h" diff --git a/dlls/d3drm/tests/d3drm.c b/dlls/d3drm/tests/d3drm.c index f78e098fec..4ed90ca682 100644 --- a/dlls/d3drm/tests/d3drm.c +++ b/dlls/d3drm/tests/d3drm.c @@ -5572,6 +5572,7 @@ static void test_load_texture(void) char *filename; HRESULT hr; BOOL ret; + ULONG ref1, ref2; int i; static const struct @@ -5595,6 +5596,7 @@ static void test_load_texture(void) ok(SUCCEEDED(hr), "Failed to get IDirect3DRM2 interface, hr %#x.\n", hr); hr = IDirect3DRM_QueryInterface(d3drm1, &IID_IDirect3DRM3, (void **)&d3drm3); ok(SUCCEEDED(hr), "Failed to get IDirect3DRM3 interface, hr %#x.\n", hr); + ref1 = get_refcount((IUnknown *)d3drm1); for (i = 0; i < ARRAY_SIZE(tests); ++i) { @@ -5602,24 +5604,74 @@ static void test_load_texture(void) hr = IDirect3DRM_LoadTexture(d3drm1, filename, &texture1); ok(SUCCEEDED(hr), "Test %u: Failed to load texture, hr %#x.\n", i, hr); + ref2 = get_refcount((IUnknown *)d3drm1); + todo_wine ok(ref2 > ref1, "Test %u: expected ref2 > ref1, got ref1 = %u, ref2 = %u.\n", i, ref1, ref2); + + hr = IDirect3DRMTexture_InitFromFile(texture1, filename); + todo_wine ok(hr == D3DRMERR_BADOBJECT, "Test %u: Expected hr == D3DRMERR_BADOBJECT, got %#x.\n", i, hr); + /* InitFromFile seems to AddRef IDirect3DRM even if it fails. */ + if (FAILED(hr)) + IDirect3DRM_Release(d3drm1); d3drm_img = IDirect3DRMTexture_GetImage(texture1); - todo_wine ok(!!d3drm_img, "Test %u: Failed to get image.\n", i); + ok(!!d3drm_img, "Test %u: Failed to get image.\n", i); if (d3drm_img) - test_bitmap_data(i * 4, d3drm_img, FALSE, tests[i].w, tests[i].h, tests[i].palettized); + test_bitmap_data(i * 7, d3drm_img, FALSE, tests[i].w, tests[i].h, tests[i].palettized); + IDirect3DRMTexture_Release(texture1); + ref2 = get_refcount((IUnknown *)d3drm1); + ok(ref1 == ref2, "Test %u: expected ref1 == ref2, got ref1 = %u, ref2 = %u.\n", i, ref1, ref2); + hr = IDirect3DRM_CreateObject(d3drm1, &CLSID_CDirect3DRMTexture, NULL, &IID_IDirect3DRMTexture, (void **)&texture1); + ok(SUCCEEDED(hr), "Test %u: Failed to create texture, hr %#x.\n", i, hr); + hr = IDirect3DRMTexture_InitFromFile(texture1, NULL); + ok(hr == D3DRMERR_BADOBJECT, "Test %u: Expected hr == D3DRMERR_BADOBJECT, got %#x.\n", i, hr); + hr = IDirect3DRMTexture_InitFromFile(texture1, ""); + ok(hr == D3DRMERR_BADOBJECT, "Test %u: Expected hr == D3DRMERR_BADOBJECT, got %#x.\n", i, hr); + hr = IDirect3DRMTexture_InitFromFile(texture1, filename); + ok(SUCCEEDED(hr), "Test %u: Failed to initialize texture from file, hr %#x.\n", i, hr); + d3drm_img = IDirect3DRMTexture_GetImage(texture1); + ok(!!d3drm_img, "Test %u: Failed to get image.\n", i); + test_bitmap_data(i * 7 + 1, d3drm_img, FALSE, tests[i].w, tests[i].h, tests[i].palettized); IDirect3DRMTexture_Release(texture1); hr = IDirect3DRM2_LoadTexture(d3drm2, filename, &texture2); ok(SUCCEEDED(hr), "Test %u: Failed to load texture, hr %#x.\n", i, hr); + ref2 = get_refcount((IUnknown *)d3drm1); + todo_wine ok(ref2 > ref1, "Test %u: expected ref2 > ref1, got ref1 = %u, ref2 = %u.\n", i, ref1, ref2); + + hr = IDirect3DRMTexture2_InitFromFile(texture2, filename); + todo_wine ok(hr == D3DRMERR_BADOBJECT, "Test %u: Expected hr == D3DRMERR_BADOBJECT, got %#x.\n", i, hr); + if (FAILED(hr)) + IDirect3DRM_Release(d3drm1); d3drm_img = IDirect3DRMTexture2_GetImage(texture2); - todo_wine ok(!!d3drm_img, "Test %u: Failed to get image.\n", i); + ok(!!d3drm_img, "Test %u: Failed to get image.\n", i); if (d3drm_img) - test_bitmap_data(i * 4 + 1, d3drm_img, TRUE, tests[i].w, tests[i].h, tests[i].palettized); + test_bitmap_data(i * 7 + 2, d3drm_img, TRUE, tests[i].w, tests[i].h, tests[i].palettized); + IDirect3DRMTexture2_Release(texture2); + ref2 = get_refcount((IUnknown *)d3drm1); + ok(ref1 == ref2, "Test %u: expected ref1 == ref2, got ref1 = %u, ref2 = %u.\n", i, ref1, ref2); + hr = IDirect3DRM2_CreateObject(d3drm2, &CLSID_CDirect3DRMTexture, NULL, &IID_IDirect3DRMTexture2, (void **)&texture2); + ok(SUCCEEDED(hr), "Test %u: Failed to create texture, hr %#x.\n", i, hr); + hr = IDirect3DRMTexture2_InitFromFile(texture2, NULL); + ok(hr == D3DRMERR_BADOBJECT, "Test %u: Expected hr == D3DRMERR_BADOBJECT, got %#x.\n", i, hr); + hr = IDirect3DRMTexture2_InitFromFile(texture2, ""); + ok(hr == D3DRMERR_BADOBJECT, "Test %u: Expected hr == D3DRMERR_BADOBJECT, got %#x.\n", i, hr); + hr = IDirect3DRMTexture2_InitFromFile(texture2, filename); + ok(SUCCEEDED(hr), "Test %u: Failed to initialize texture from file, hr %#x.\n", i, hr); + d3drm_img = IDirect3DRMTexture2_GetImage(texture2); + ok(!!d3drm_img, "Test %u: Failed to get image.\n", i); + test_bitmap_data(i * 7 + 3, d3drm_img, TRUE, tests[i].w, tests[i].h, tests[i].palettized); IDirect3DRMTexture2_Release(texture2); hr = IDirect3DRM3_LoadTexture(d3drm3, filename, &texture3); ok(SUCCEEDED(hr), "Test %u: Failed to load texture, hr %#x.\n", i, hr); + ref2 = get_refcount((IUnknown *)d3drm1); + todo_wine ok(ref2 > ref1, "Test %u: expected ref2 > ref1, got ref1 = %u, ref2 = %u.\n", i, ref1, ref2); + + hr = IDirect3DRMTexture3_InitFromFile(texture3, filename); + todo_wine ok(hr == D3DRMERR_BADOBJECT, "Test %u: Expected hr == D3DRMERR_BADOBJECT, got %#x.\n", i, hr); + if (FAILED(hr)) + IDirect3DRM_Release(d3drm1); d3drm_img = IDirect3DRMTexture3_GetImage(texture3); - todo_wine ok(!!d3drm_img, "Test %u: Failed to get image.\n", i); + ok(!!d3drm_img, "Test %u: Failed to get image.\n", i); if (d3drm_img) test_bitmap_data(i * 4 + 2, d3drm_img, TRUE, tests[i].w, tests[i].h, tests[i].palettized); /* Test whether querying a version 1 texture from version 3 causes a @@ -5627,11 +5679,26 @@ static void test_load_texture(void) hr = IDirect3DRMTexture3_QueryInterface(texture3, &IID_IDirect3DRMTexture, (void **)&texture1); ok(SUCCEEDED(hr), "Failed to get IDirect3DRMTexture interface, hr %#x.\n", hr); d3drm_img = IDirect3DRMTexture_GetImage(texture1); - todo_wine ok(!!d3drm_img, "Test %u: Failed to get image.\n", i); + ok(!!d3drm_img, "Test %u: Failed to get image.\n", i); if (d3drm_img) test_bitmap_data(i * 4 + 3, d3drm_img, TRUE, tests[i].w, tests[i].h, tests[i].palettized); IDirect3DRMTexture_Release(texture1); IDirect3DRMTexture3_Release(texture3); + ref2 = get_refcount((IUnknown *)d3drm1); + ok(ref1 == ref2, "Test %u: expected ref1 == ref2, got ref1 = %u, ref2 = %u.\n", i, ref1, ref2); + + hr = IDirect3DRM3_CreateObject(d3drm3, &CLSID_CDirect3DRMTexture, NULL, &IID_IDirect3DRMTexture3, (void **)&texture3); + ok(SUCCEEDED(hr), "Test %u: Failed to create texture, hr %#x.\n", i, hr); + hr = IDirect3DRMTexture3_InitFromFile(texture3, NULL); + ok(hr == D3DRMERR_BADOBJECT, "Test %u: Expected hr == D3DRMERR_BADOBJECT, got %#x.\n", i, hr); + hr = IDirect3DRMTexture3_InitFromFile(texture3, ""); + ok(hr == D3DRMERR_BADOBJECT, "Test %u: Expected hr == D3DRMERR_BADOBJECT, got %#x.\n", i, hr); + hr = IDirect3DRMTexture3_InitFromFile(texture3, filename); + ok(SUCCEEDED(hr), "Test %u: Failed to initialize texture from file, hr %#x.\n", i, hr); + d3drm_img = IDirect3DRMTexture3_GetImage(texture3); + ok(!!d3drm_img, "Test %u: Failed to get image.\n", i); + test_bitmap_data(i * 7 + 6, d3drm_img, TRUE, tests[i].w, tests[i].h, tests[i].palettized); + IDirect3DRMTexture3_Release(texture3); ret = DeleteFileA(filename); ok(ret, "Test %u: Failed to delete bitmap \"%s\".\n", i, filename); diff --git a/dlls/d3drm/texture.c b/dlls/d3drm/texture.c index fd56e76ff9..4c81c788bf 100644 --- a/dlls/d3drm/texture.c +++ b/dlls/d3drm/texture.c @@ -2,6 +2,7 @@ * Implementation of IDirect3DRMTextureX interfaces * * Copyright 2012 Christian Costa + * Copyright 2016 Aaryaman Vasishta * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -67,6 +68,294 @@ static BOOL d3drm_validate_image(D3DRMIMAGE *image) return TRUE; } +static void CDECL destroy_image_callback(IDirect3DRMObject *obj, void *arg) +{ + D3DRMIMAGE *image = arg; + + TRACE("image %p texture object %p.\n", arg, obj); + + HeapFree(GetProcessHeap(), 0, image->buffer1); + HeapFree(GetProcessHeap(), 0, image); +} + +HRESULT d3drm_texture_load(struct d3drm_texture *texture, const char *path, BOOL load_upside_down, D3DRMIMAGE **image_out) +{ + BITMAPINFO *info; + BITMAPFILEHEADER *bmp_header; + unsigned char *buffer; + DWORD size; + HANDLE hfile, hmapping; + HRESULT hr = D3DRM_OK; + D3DRMPALETTEENTRY *colors = NULL; + D3DRMIMAGE *image = NULL; + BOOL black_used = FALSE, palette_used; + LONG w; + LONG h; + UINT i, j, k; + UINT idx, buffer1_idx; + unsigned char *buffer1; + UINT bpp; + UINT bpl; + UINT num_colors = 0; + struct colors_24bpp + { + BYTE red; + BYTE green; + BYTE blue; + } *color = NULL; + + /* Load the bitmap data */ + hfile = CreateFileA(path, GENERIC_READ, FILE_SHARE_READ, 0, OPEN_EXISTING, 0, 0); + if (hfile == INVALID_HANDLE_VALUE) + return D3DRMERR_BADOBJECT; + + size = GetFileSize(hfile, NULL); + if (size == INVALID_FILE_SIZE) + { + CloseHandle(hfile); + return D3DRMERR_BADVALUE; + } + if (!(hmapping = CreateFileMappingA(hfile, NULL, PAGE_READONLY, 0, 0, NULL))) + { + CloseHandle(hfile); + return D3DRMERR_BADVALUE; + } + if (!(buffer = MapViewOfFile(hmapping, FILE_MAP_READ, 0, 0, 0))) + { + CloseHandle(hmapping); + CloseHandle(hfile); + return D3DRMERR_BADVALUE; + } + + bmp_header = (BITMAPFILEHEADER *)buffer; + if (bmp_header->bfType != 0x4d42) /* BM */ + { + hr = D3DRMERR_BADFILE; + goto cleanup; + } + + info = (BITMAPINFO *)(bmp_header + 1); + /* Only allow version 1 DIB's (BITMAPINFOHEADER) to be loaded */ + if (info->bmiHeader.biSize != sizeof(info->bmiHeader)) + { + hr = D3DRMERR_BADFILE; + goto cleanup; + } + + bpp = info->bmiHeader.biBitCount == 24 ? 32 : info->bmiHeader.biBitCount; + w = info->bmiHeader.biWidth; + h = abs(info->bmiHeader.biHeight); + + if (bpp == 8) + { + buffer += sizeof(BITMAPINFOHEADER) + sizeof(BITMAPFILEHEADER) + 256 * sizeof(RGBQUAD); + palette_used = TRUE; + } + else if (bpp == 32) + { + buffer += sizeof(BITMAPINFOHEADER) + sizeof(BITMAPFILEHEADER); + palette_used = FALSE; + } + else + { + hr = D3DRMERR_BADFILE; + goto cleanup; + } + + /* Create and initialize the image struct. */ + color = (struct colors_24bpp *)buffer; + + if (palette_used) + bpl = w; + else if ((bpl = ((w + 3) & ~3)) > UINT_MAX / (bpp / 8)) + { + hr = D3DRMERR_BADALLOC; + goto cleanup; + } + + bpl = bpl * bpp / 8; + + if (bpl > UINT_MAX / h) + { + hr = D3DRMERR_BADALLOC; + goto cleanup; + } + + if (!(colors = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, 256 * sizeof(*colors)))) + { + WARN("Not enough memory to allocate palette, returning NULL.\n"); + hr = D3DRMERR_BADALLOC; + goto cleanup; + } + + if (!(image = HeapAlloc(GetProcessHeap(), 0, sizeof(*image)))) + { + WARN("Not enough memory to allocate image struct, returning NULL.\n"); + hr = D3DRMERR_BADALLOC; + goto cleanup; + } + image->buffer1 = NULL; + + if (!(image->buffer1 = HeapAlloc(GetProcessHeap(), 0, bpl * h))) + { + WARN("Not enough memory to allocate image buffer, returning NULL.\n"); + hr = D3DRMERR_BADALLOC; + goto cleanup; + } + + buffer1 = image->buffer1; + memset(buffer1, 0xff, bpl * h); + if (!palette_used) + { + for (i = 0; i < h; i++) + { + for (j = 0; j < w; j++) + { + buffer1_idx = ((w + 3) & ~3) * i + j; + idx = load_upside_down ? (h - 1 - i) * w + j : i * w + j; + for (k = 0; k < 256; k++) + { + if (color[idx].blue == colors[k].blue && + color[idx].green == colors[k].green && + color[idx].red == colors[k].red) + { + if (color[idx].blue == 0 && + color[idx].green == 0 && + color[idx].red == 0 && + !black_used) + { + black_used = TRUE; + colors[num_colors++].flags = D3DRMPALETTE_READONLY; + } + buffer1[buffer1_idx] = k; + + break; + } + } + if (k == 256) + { + if (num_colors == 256) + { + num_colors++; + i = h; + break; + } + buffer1[buffer1_idx] = num_colors; + colors[num_colors].red = color[idx].red; + colors[num_colors].green = color[idx].green; + colors[num_colors].blue = color[idx].blue; + colors[num_colors++].flags = D3DRMPALETTE_READONLY; + } + } + } + + if (num_colors <= 256) + { + if (!(image->palette = HeapAlloc(GetProcessHeap(), 0, num_colors * sizeof(*image->palette)))) + { + WARN("Not enough memory to allocate image palette, returning NULL.\n"); + hr = D3DRMERR_BADALLOC; + goto cleanup; + } + image->red_mask = 0xff; + image->green_mask = 0xff; + image->blue_mask = 0xff; + image->rgb = 0; + bpl = bpl / (bpp / 8); + memcpy(image->palette, colors, num_colors * sizeof(D3DRMPALETTEENTRY)); + image->palette_size = num_colors; + palette_used = TRUE; + } + else + { + bpl = w * 4; + image->rgb = 1; + image->palette = NULL; + image->palette_size = 0; + for (i = 0; i < h; ++i) + { + for (j = 0; j < w; ++j) + { + unsigned char *ptr = &buffer1[i * bpl + j * 4]; + idx = load_upside_down ? (h - 1 - i) * w * 3 + j * 3 : i * w * 3 + j * 3; + ptr[0] = buffer[idx]; + ptr[1] = buffer[idx + 1]; + ptr[2] = buffer[idx + 2]; + ptr[3] = 0xff; + } + } + + image->red_mask = 0xff0000; + image->green_mask = 0x00ff00; + image->blue_mask = 0x0000ff; + } + } + else + { + if (!(image->palette = HeapAlloc(GetProcessHeap(), 0, 256 * sizeof(*image->palette)))) + { + WARN("Not enough memory to allocate image palette, returning NULL.\n"); + hr = D3DRMERR_BADALLOC; + goto cleanup; + } + + memcpy(image->palette, info->bmiColors, 256 * sizeof(D3DRMPALETTEENTRY)); + for (i = 0; i < 256; i++) + { + image->palette[i].flags = D3DRMPALETTE_READONLY; + } + if (load_upside_down) + { + for (i = 0; i < h; i++) + { + for (j = 0; j < w; j++) + { + idx = (h - 1 - i) * bpl + j; + buffer1[i * bpl + j] = buffer[idx]; + } + } + } + else + { + memcpy(buffer1, buffer, bpl * h); + } + image->palette_size = 256; + image->red_mask = 0xff; + image->green_mask = 0xff; + image->blue_mask = 0xff; + image->rgb = 0; + } + image->width = w; + image->height = h; + image->aspectx = 1; + image->aspecty = 1; + image->alpha_mask = 0; + image->depth = palette_used ? 8 : bpp; + image->bytes_per_line = bpl; + image->buffer2 = NULL; + + /* Use an internal destroy callback to destroy image struct */ + hr = IDirect3DRMObject_AddDestroyCallback(&texture->IDirect3DRMTexture3_iface, destroy_image_callback, image); + + *image_out = image; + +cleanup: + UnmapViewOfFile(buffer); + CloseHandle(hmapping); + CloseHandle(hfile); + + HeapFree(GetProcessHeap(), 0, colors); + + if (FAILED(hr)) + { + if (image) + HeapFree(GetProcessHeap(), 0, image->buffer1); + HeapFree(GetProcessHeap(), 0, image); + } + + return hr; +} + static HRESULT WINAPI d3drm_texture1_QueryInterface(IDirect3DRMTexture *iface, REFIID riid, void **out) { struct d3drm_texture *texture = impl_from_IDirect3DRMTexture(iface); @@ -171,9 +460,18 @@ static HRESULT WINAPI d3drm_texture1_GetClassName(IDirect3DRMTexture *iface, DWO static HRESULT WINAPI d3drm_texture1_InitFromFile(IDirect3DRMTexture *iface, const char *filename) { - FIXME("iface %p, filename %s stub!\n", iface, debugstr_a(filename)); + struct d3drm_texture *texture = impl_from_IDirect3DRMTexture(iface); + D3DRMIMAGE *image; + HRESULT hr; - return E_NOTIMPL; + TRACE("iface %p, filename %s.\n", iface, debugstr_a(filename)); + + if (FAILED(hr = d3drm_texture_load(texture, filename, FALSE, &image))) + return hr; + + hr = IDirect3DRMTexture3_InitFromImage(&texture->IDirect3DRMTexture3_iface, image); + + return hr; } static HRESULT WINAPI d3drm_texture1_InitFromSurface(IDirect3DRMTexture *iface, @@ -473,9 +771,15 @@ static HRESULT WINAPI d3drm_texture2_GetClassName(IDirect3DRMTexture2 *iface, DW static HRESULT WINAPI d3drm_texture2_InitFromFile(IDirect3DRMTexture2 *iface, const char *filename) { - FIXME("iface %p, filename %s stub!\n", iface, debugstr_a(filename)); + struct d3drm_texture *object = impl_from_IDirect3DRMTexture2(iface); + HRESULT hr; - return E_NOTIMPL; + TRACE("iface %p, filename %s.\n", iface, debugstr_a(filename)); + + if (FAILED(hr = IDirect3DRMTexture3_InitFromFile(&object->IDirect3DRMTexture3_iface, filename))) + return hr; + + return D3DRM_OK; } static HRESULT WINAPI d3drm_texture2_InitFromSurface(IDirect3DRMTexture2 *iface, @@ -833,9 +1137,18 @@ static HRESULT WINAPI d3drm_texture3_GetClassName(IDirect3DRMTexture3 *iface, DW static HRESULT WINAPI d3drm_texture3_InitFromFile(IDirect3DRMTexture3 *iface, const char *filename) { - FIXME("iface %p, filename %s stub!\n", iface, debugstr_a(filename)); + struct d3drm_texture *texture = impl_from_IDirect3DRMTexture3(iface); + D3DRMIMAGE *image; + HRESULT hr; - return E_NOTIMPL; + TRACE("iface %p, filename %s.\n", iface, debugstr_a(filename)); + + if (FAILED(hr = d3drm_texture_load(texture, filename, TRUE, &image))) + return hr; + + hr = IDirect3DRMTexture3_InitFromImage(iface, image); + + return hr; } static HRESULT WINAPI d3drm_texture3_InitFromSurface(IDirect3DRMTexture3 *iface, -- 2.17.1

2 3

[PATCH 4/4] testbot: Detect the TestBot scripts usage errors in LogUtils.
by Francois Gouget 26 Aug '18

26 Aug '18

They indicate a TestBot error obviously. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/lib/WineTestBot/LogUtils.pm | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/testbot/lib/WineTestBot/LogUtils.pm b/testbot/lib/WineTestBot/LogUtils.pm index a9ab20b5f..17b9093aa 100644 --- a/testbot/lib/WineTestBot/LogUtils.pm +++ b/testbot/lib/WineTestBot/LogUtils.pm @@ -119,7 +119,11 @@ sub GetLogLineCategory($) { return "diag"; } - if ($Line =~ /^BotError:/ or + if (# TestBot script error messages + $Line =~ /^[a-zA-Z.]+:error: / or + # TestBot error + $Line =~ /^BotError:/ or + # X errors $Line =~ /^X Error of failed request: / or $Line =~ / opcode of failed request: /) { -- 2.18.0

1 0

[PATCH 3/4] testbot: Fix detection of new exceptions and too much data errors.
by Francois Gouget 26 Aug '18

26 Aug '18

The address of the crashing code changes with each patch and should thus be ignored. The exact amount of data printed does not change the error. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/lib/WineTestBot/LogUtils.pm | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/testbot/lib/WineTestBot/LogUtils.pm b/testbot/lib/WineTestBot/LogUtils.pm index 19a39aaf5..a9ab20b5f 100644 --- a/testbot/lib/WineTestBot/LogUtils.pm +++ b/testbot/lib/WineTestBot/LogUtils.pm @@ -756,6 +756,12 @@ sub _GetLineKey($) # Remove the line number $Line =~ s/^([_a-z0-9]+\.c:)\d+:( Test (?:failed|succeeded inside todo block): )/$1$2/; + # Remove the crash code address: it changes whenever the test is recompiled + $Line =~ s/^(Unhandled exception: .* code) $0x[0-9a-fA-F]{8,16}$\.$/$1/; + + # The exact amount of data printed does not change the error + $Line =~ s/^([_.a-z0-9-]+:[_a-z0-9]* prints too much data )$[0-9]+ bytes$$/$1/; + # Note: Only the 'done (258)' lines are reported as errors and they are # modified by GetLogErrors() so that they no longer contain the pid. # So there is no need to remove the pid from the done lines. -- 2.18.0

1 0

[PATCH 2/4] testbot: Add a note regarding the detection of new errors.
by Francois Gouget 26 Aug '18

26 Aug '18

The detection of new errors may be based on a WineTest report which is not up-to-date. However this should not be a real issue in practice. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- This is not a new issue. testbot/bin/WineSendLog.pl | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/testbot/bin/WineSendLog.pl b/testbot/bin/WineSendLog.pl index f3b56d8c9..b0efa8c33 100755 --- a/testbot/bin/WineSendLog.pl +++ b/testbot/bin/WineSendLog.pl @@ -266,6 +266,12 @@ EOF # Build a job summary with only the new errors # + # Note that this may be a bit inaccurate right after a Wine commit if this + # job's patch got compiled on top of the new Wine before all the reference + # WineTest results were updated. This is made more likely by the job + # priorities: high for Wine updates, and low for WineTest runs. + # However in practice this would only be an issue if the patch reintroduced + # an error that just disappeared in the latest Wine which is highly unlikely. my @Messages; foreach my $Key (@SortedKeys) { -- 2.18.0

1 0

[PATCH 1/4] testbot/WineSendLog: Use LogUtils to identify (new) errors.
by Francois Gouget 26 Aug '18

26 Aug '18

Added LogUtils::GetNewLogErrors() to identify new errors in test reports and their associated extra errors files. Note that unlike WineSendLog.pl's old CompareLogs() function GetNewLogErrors() can deal with reports containing the errors for multiple modules, which is a requirement for running the tests on Wine. WineSendLog.pl now uses LogUtils instead of duplicating the code. Besides reaping the benefits from the improved GetNewLogErrors() this also improves error detection and ensures it is consistent with JobDetails.pl. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/bin/WineSendLog.pl | 380 ++++++---------------------- testbot/lib/WineTestBot/LogUtils.pm | 106 +++++++- 2 files changed, 183 insertions(+), 303 deletions(-) diff --git a/testbot/bin/WineSendLog.pl b/testbot/bin/WineSendLog.pl index 2127eadb7..f3b56d8c9 100755 --- a/testbot/bin/WineSendLog.pl +++ b/testbot/bin/WineSendLog.pl @@ -78,129 +78,47 @@ sub Error(@) # -# Log analysis +# Reporting # -sub IsBotFailure($) +sub GetTitle($$) { - my ($ErrLine) = @_; + my ($StepTask, $LogName) = @_; - return ($ErrLine =~ m/Can't set VM status to running/ || - $ErrLine =~ m/Can't copy exe to VM/ || - $ErrLine =~ m/Can't copy log from VM/ || - $ErrLine =~ m/Can't copy generated executable from VM/); -} - -sub CheckErrLog($) -{ - my ($ErrLogFileName) = @_; - - my $BotFailure = !1; - my $Messages = ""; - if (open ERRFILE, "$ErrLogFileName") - { - my $Line; - while (defined($Line = <ERRFILE>)) - { - if (IsBotFailure($Line)) - { - if (! $Messages) - { - $BotFailure = 1; - } - } - else - { - $Messages .= $Line; - } - } - close ERRFILE; - } - - return ($BotFailure, $Messages); -} - -sub ReadLog($$$) -{ - my ($LogName, $BaseName, $TestSet) = @_; - - my @Messages; - if (open LOG, "<$LogName") - { - my $Line; - my $Found = !1; - while (! $Found && defined($Line = <LOG>)) - { - $Found = ($Line =~ m/${BaseName}:${TestSet} start/); - } - if ($Found) - { - $Found = !1; - while (! $Found && defined($Line = <LOG>)) - { - $Line =~ s/[\r\n]*$//; - if ($Line =~ m/${BaseName}:${TestSet}(?::[0-9a-f]+)? done/) - { - if ($Line =~ m/${BaseName}:${TestSet}(?::[0-9a-f]+)? done $258$/) - { - push @Messages, "The test timed out"; - } - $Found = 1; - } - else - { - push @Messages, $Line; - } - } - } - - close LOG; - } - else + my $Label = GetLogLabel($LogName); + if ($LogName !~ /\.report$/ and + ($StepTask->Type eq "build" or $StepTask->VM->Type eq "wine")) { - Error "Unable to open '$LogName' for reading: $!\n"; + $Label = "build log"; } - return \@Messages; + return $StepTask->VM->Name . " ($Label)"; } -sub GetLineKey($) +sub DumpLogAndErr($$) { - my ($Line) = @_; - - $Line =~ s/^([\w_.]+:)\d+(:.*)$/$1$2/; - - return $Line; -} - -sub CompareLogs($$$$) -{ - my ($SuiteLog, $TaskLog, $BaseName, $TestSet) = @_; - - my $Messages = ""; + my ($File, $Path) = @_; - my $SuiteMessages = ReadLog($SuiteLog, $BaseName, $TestSet); - my $TaskMessages = ReadLog($TaskLog, $BaseName, $TestSet); - - my $Diff = Algorithm::Diff->new($SuiteMessages, $TaskMessages, - { keyGen => \&GetLineKey }); - while ($Diff->Next()) + my $PrintSeparator; + foreach my $FileName ($Path, "$Path.err") { - if (! $Diff->Same()) + if (open(my $LogFile, "<", $FileName)) { - foreach my $Line ($Diff->Items(2)) + my $First = 1; + foreach my $Line (<$LogFile>) { - if ($Line =~ m/: Test failed: / || - $Line =~ m/: unhandled exception [0-9a-fA-F]{8} at / || - $Line =~ m/The test timed out/) + $Line =~ s/\s*$//; + if ($First and $PrintSeparator) { - $Messages .= "$Line\n"; + print $File "\n"; + $First = 0; } + print $File "$Line\n"; } + close($LogFile); + $PrintSeparator = 1; } } - - return $Messages; } sub SendLog($) @@ -279,110 +197,28 @@ EOF print $Sendmail "\nYou can also see the results at:\n$JobURL\n\n"; # Print the job summary - my @FailureKeys; + my ($JobErrors, $ReportCounts); foreach my $Key (@SortedKeys) { my $StepTask = $StepsTasks->GetItem($Key); my $TaskDir = $StepTask->GetTaskDir(); - print $Sendmail "\n=== ", $StepTask->GetTitle(), " ===\n"; - - my $LogFiles = GetLogFileNames($TaskDir); - my $LogName = $LogFiles->[0] || "log"; - if (open LOGFILE, "<$TaskDir/$LogName") + my $LogNames = GetLogFileNames($TaskDir); + $JobErrors->{$Key}->{LogNames} = $LogNames; + foreach my $LogName (@$LogNames) { - my $HasLogEntries = !1; - my $PrintedSomething = !1; - my $CurrentDll = ""; - my $PrintedDll = ""; - my $Line; - while (defined($Line = <LOGFILE>)) - { - $HasLogEntries = 1; - $Line =~ s/\s*$//; - if ($Line =~ m/^([^:]+):[^ ]+ start [^ ]+ -$/) - { - $CurrentDll = $1; - } - my $Category = $LogName =~ /\.report$/ ? - GetReportLineCategory($Line) : - GetLogLineCategory($Line); - if ($Category eq "error") - { - if ($PrintedDll ne $CurrentDll) - { - print $Sendmail "\n$CurrentDll:\n"; - $PrintedDll = $CurrentDll; - } - if ($Line =~ m/^[^:]+:([^ ]+)(?::[0-9a-f]+)? done $258$/) - { - print $Sendmail "$1: The test timed out\n"; - } - else - { - print $Sendmail "$Line\n"; - } - $PrintedSomething = 1; - } - } - close LOGFILE; + my ($Groups, $Errors) = GetLogErrors("$TaskDir/$LogName"); + next if (!$Groups or !@$Groups); + $JobErrors->{$Key}->{HasErrors} = 1; + $JobErrors->{$Key}->{$LogName}->{Groups} = $Groups; + $JobErrors->{$Key}->{$LogName}->{Errors} = $Errors; - if (open ERRFILE, "<$TaskDir/log.err") - { - my $First = 1; - while (defined($Line = <ERRFILE>)) - { - if ($First) - { - print $Sendmail "\n"; - $First = !1; - } - $HasLogEntries = 1; - $Line =~ s/\s*$//; - print $Sendmail "$Line\n"; - $PrintedSomething = 1; - } - close ERRFILE; - } + print $Sendmail "\n=== ", GetTitle($StepTask, $LogName), " ===\n"; - if (! $PrintedSomething) - { - if (! $HasLogEntries) - { - print $Sendmail "Empty test log and no error message\n"; - } - elsif ($StepTask->Type eq "build") - { - print $Sendmail "No build failures found\n"; - } - else - { - print $Sendmail "No test failures found\n"; - } - } - else + foreach my $GroupName (@$Groups) { - push @FailureKeys, $Key; - } - } - elsif (open ERRFILE, "<$TaskDir/log.err") - { - my $HasErrEntries = !1; - my $Line; - while (defined($Line = <ERRFILE>)) - { - $HasErrEntries = 1; - $Line =~ s/\s*$//; - print $Sendmail "$Line\n"; - } - close ERRFILE; - if (! $HasErrEntries) - { - print $Sendmail "No test log and no error message"; - } - else - { - push @FailureKeys, $Key; + print $Sendmail ($GroupName ? "\n$GroupName:\n" : "\n"); + print $Sendmail "$_\n" for (@{$Errors->{$GroupName}}); } } } @@ -393,46 +229,25 @@ EOF my $StepTask = $StepsTasks->GetItem($Key); my $TaskDir = $StepTask->GetTaskDir(); - print $Sendmail <<"EOF"; + foreach my $LogName (@{$JobErrors->{$Key}->{LogNames}}) + { + print $Sendmail <<"EOF"; --$PART_BOUNDARY Content-Type: text/plain; charset="UTF-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit EOF - print $Sendmail "Content-Disposition: attachment; filename=", - $StepTask->VM->Name, ".log\n\n"; - print $Sendmail "Not dumping logs in debug mode\n" if ($Debug); - - my $LogFiles = GetLogFileNames($TaskDir); - my $LogName = $LogFiles->[0] || "log"; - my $PrintSeparator = !1; - if (open LOGFILE, "<$TaskDir/$LogName") - { - my $Line; - while (defined($Line = <LOGFILE>)) + print $Sendmail "Content-Disposition: attachment; filename=", + $StepTask->VM->Name, "_$LogName\n\n"; + if ($Debug) { - $Line =~ s/\s*$//; - print $Sendmail "$Line\n" if (!$Debug); - $PrintSeparator = 1; + print $Sendmail "Not dumping logs in debug mode\n"; } - close LOGFILE; - } - - if (open ERRFILE, "<$TaskDir/log.err") - { - my $Line; - while (defined($Line = <ERRFILE>)) + else { - if ($PrintSeparator) - { - print $Sendmail "\n" if (!$Debug); - $PrintSeparator = !1; - } - $Line =~ s/\s*$//; - print $Sendmail "$Line\n" if (!$Debug); + DumpLogAndErr($Sendmail, "$TaskDir/$LogName"); } - close ERRFILE; } } @@ -446,75 +261,62 @@ EOF return; } + # # Build a job summary with only the new errors # - my $Messages = ""; - foreach my $Key (@FailureKeys) + my @Messages; + foreach my $Key (@SortedKeys) { + next if (!$JobErrors->{$Key}->{HasErrors}); my $StepTask = $StepsTasks->GetItem($Key); my $TaskDir = $StepTask->GetTaskDir(); - my ($BotFailure, $MessagesFromErr) = CheckErrLog("$TaskDir/log.err"); - if ($BotFailure) + if ($StepTask->Status eq "boterror") { - # TestBot errors are not the developer's fault and prevent us from doing - # any meaningful analysis. So skip. - Error "A TestBot error was found in $TaskDir/log.err\n"; + # TestBot errors are not the developer's fault and prevent us from + # identifying new errors. So skip. + Error "A Task error was found in '$TaskDir/log.err'\n"; next; } - my $MessagesFromLog = ""; - my $LogFiles = GetLogFileNames($TaskDir); - my $LogName = $LogFiles->[0] || "log"; - if ($LogName =~ /\.report$/) + foreach my $LogName (@{$JobErrors->{$Key}->{LogNames}}) { - my $LatestName = "$DataDir/latest/". $StepTask->VM->Name ."_$LogName"; - my ($LatestBotFailure, $Dummy) = CheckErrLog("$LatestName.err"); - if (! $LatestBotFailure) + my $LogErrors = $JobErrors->{$Key}->{$LogName}; + my $RefFileName = "$DataDir/latest". $StepTask->VM->Name ."_$LogName"; + my ($NewGroups, $NewErrors) = GetNewLogErrors($RefFileName, $LogErrors->{Groups}, $LogErrors->{Errors}); + if (!$NewGroups) { - if (defined($StepTask->CmdLineArg)) - { - # Filter out failures that happened in the full test suite: - # the test suite is run against code which is already in Wine - # so any failure it reported is not caused by this patch. - $StepTask->FileName =~ m/^(.*)_test/; - my $BaseName = $1; - $MessagesFromLog = CompareLogs($LatestName, "$TaskDir/$LogName", - $BaseName, $StepTask->CmdLineArg); - } + # There was no reference log (typical of build logs) + # so every error is new + $NewGroups = $LogErrors->{Groups}; + $NewErrors = $LogErrors->{Errors}; } - else + elsif (!@$NewGroups) { - Error "BotFailure found in $LatestName.err\n"; + # There is no new error + next; } - } - elsif (open(my $LogFile, "<", "$TaskDir/$LogName")) - { - foreach my $Line (<$LogFile>) + + push @Messages, "\n=== ". GetTitle($StepTask, $LogName) ." ===\n"; + + foreach my $GroupName (@$NewGroups) { - my $Category = $LogName =~ /\.report$/ ? - GetReportLineCategory($Line) : - GetLogLineCategory($Line); - $MessagesFromLog .= $Line if ($Category eq "error"); + push @Messages, ($GroupName ? "\n$GroupName:\n" : "\n"); + push @Messages, "$_\n" for (@{$NewErrors->{$GroupName}}); } - close($LogFile); - } - if ($MessagesFromErr || $MessagesFromLog) - { - $Messages .= "\n=== " . $StepTask->GetTitle() . " ===\n" . - $MessagesFromLog . $MessagesFromErr; } } + # # Send a summary of the new errors to the mailing list # Debug("\n-------------------- Mailing list email --------------------\n"); - if ($Messages) + if (@Messages) { if ($Debug) { @@ -548,7 +350,7 @@ Your paranoid android. EOF - print $Sendmail $Messages; + print $Sendmail $_ for (@Messages); close($Sendmail); } else @@ -556,6 +358,7 @@ EOF Debug("Found no error to report to the mailing list\n"); } + # # Create a .testbot file for the patches website # @@ -570,7 +373,7 @@ EOF { # Only take into account new errors to decide whether the job was # successful or not. - DebugTee($Result, "Status: ". ($Messages ? "Failed" : "OK") ."\n"); + DebugTee($Result, "Status: ". (@Messages ? "Failed" : "OK") ."\n"); DebugTee($Result, "Job-ID: ". $Job->Id ."\n"); DebugTee($Result, "URL: $JobURL\n"); @@ -579,37 +382,10 @@ EOF my $StepTask = $StepsTasks->GetItem($Key); my $TaskDir = $StepTask->GetTaskDir(); - print $Result "\n=== ", $StepTask->GetTitle(), " ===\n"; - - my $LogFiles = GetLogFileNames($TaskDir); - my $LogName = $LogFiles->[0] || "log"; - my $PrintSeparator = !1; - if (open(my $logfile, "<", "$TaskDir/$LogName")) - { - my $Line; - while (defined($Line = <$logfile>)) - { - $Line =~ s/\s*$//; - print $Result "$Line\n"; - $PrintSeparator = 1; - } - close($logfile); - } - - if (open(my $errfile, "<", "$TaskDir/log.err")) + foreach my $LogName (@{$JobErrors->{$Key}->{LogNames}}) { - my $Line; - while (defined($Line = <$errfile>)) - { - if ($PrintSeparator) - { - print $Result "\n"; - $PrintSeparator = !1; - } - $Line =~ s/\s*$//; - print $Result "$Line\n"; - } - close($errfile); + print $Result "=== ", GetTitle($StepTask, $LogName), " ===\n"; + DumpLogAndErr($Result, "$TaskDir/$LogName"); } } print $Result "--- END FULL_LOGS ---\n"; diff --git a/testbot/lib/WineTestBot/LogUtils.pm b/testbot/lib/WineTestBot/LogUtils.pm index a5a1a68b5..19a39aaf5 100644 --- a/testbot/lib/WineTestBot/LogUtils.pm +++ b/testbot/lib/WineTestBot/LogUtils.pm @@ -27,11 +27,12 @@ WineTestBot::LogUtils - Provides functions to parse task logs use Exporter 'import'; -our @EXPORT = qw(GetLogFileNames GetLogLabel GetLogErrors +our @EXPORT = qw(GetLogFileNames GetLogLabel GetLogErrors GetNewLogErrors GetLogLineCategory GetReportLineCategory RenameReferenceLogs RenameTaskLogs ParseTaskLog ParseWineTestReport); +use Algorithm::Diff; use File::Basename; use WineTestBot::Config; # For $MaxUnitSize @@ -697,6 +698,8 @@ sub GetLogErrors($) $Line =~ s/\s*$//; if (!$CurrentGroup) { + # Note: $GroupName must not depend on the previous content as this + # would break diffs. my $GroupName = $IsReport ? "Report errors" : "Task errors"; $CurrentGroup = _AddErrorGroup($Groups, $Errors, $GroupName); } @@ -714,4 +717,105 @@ sub GetLogErrors($) return $NoLog ? (undef, undef) : ($Groups, $Errors); } +sub _DumpDiff($$) +{ + my ($Label, $Diff) = @_; + + print STDERR "$Label:\n"; + $Diff = $Diff->Copy(); + while ($Diff->Next()) + { + if ($Diff->Same()) + { + print STDERR " $_\n" for ($Diff->Same()); + } + else + { + print STDERR "-$_\n" for ($Diff->Items(1)); + print STDERR "+$_\n" for ($Diff->Items(2)); + } + } +} + +=pod +=over 12 + +=item C<_GetLineKey()> + +This is a helper for GetNewLogErrors(). It reformats the log lines so they can +meaningfully be compared to the reference log even if line numbers change, etc. + +=back +=cut + +sub _GetLineKey($) +{ + my ($Line) = @_; + return undef if (!defined $Line); + + # Remove the line number + $Line =~ s/^([_a-z0-9]+\.c:)\d+:( Test (?:failed|succeeded inside todo block): )/$1$2/; + + # Note: Only the 'done (258)' lines are reported as errors and they are + # modified by GetLogErrors() so that they no longer contain the pid. + # So there is no need to remove the pid from the done lines. + + return $Line; +} + +=pod +=over 12 + +=item C<GetNewLogErrors()> + +Compares the specified errors to the reference log and returns only the ones +that are new. + +Returns a list of error groups containing new errors and a hashtable containing +the list of new errors for each group. + +=back +=cut + +sub GetNewLogErrors($$$) +{ + my ($RefFileName, $Groups, $Errors) = @_; + + my ($_Dummy, $RefErrors, $NoLog) = GetLogErrors($RefFileName); + return (undef, undef) if ($NoLog); + + my (@NewGroups, %NewErrors); + foreach my $GroupName (@$Groups) + { + if ($RefErrors->{$GroupName}) + { + my $Diff = Algorithm::Diff->new($RefErrors->{$GroupName}, + $Errors->{$GroupName}, + { keyGen => \&_GetLineKey }); + my $CurrentGroup; + while ($Diff->Next()) + { + # Skip if there are no new lines + next if ($Diff->Same() or !$Diff->Items(2)); + + if (!$CurrentGroup) + { + push @NewGroups, $GroupName; + $CurrentGroup = $NewErrors{$GroupName} = []; + } + push @$CurrentGroup, $Diff->Items(2); + } + } + else + { + # This module did not have errors before, so every error is new + push @NewGroups, $GroupName; + $NewErrors{$GroupName} = $Errors->{$GroupName}; + my $Last = @{$Errors->{$GroupName}} - 1; + } + } + + return (\@NewGroups, \%NewErrors); +} + 1; -- 2.18.0

1 0