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

[PATCH v2] shell32/autocomplete: Revamp pwzsRegKeyPath handling so it can deal with arbitrary sizes and make it more robust
by Gabriel Ivăncescu Aug. 30, 2018

Aug. 30, 2018

Handle heap_alloc failure, reg strings without a \ character at all, try harder to find the reg path (if only value fails the lookup), and read the registry value with any size. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- v2: Retrieve the registry value without failing in rare race conditions. dlls/shell32/autocomplete.c | 93 +++++++++++++++++++++++++++++---------------- 1 file changed, 61 insertions(+), 32 deletions(-) diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 5fe48e9..99d5ae7 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -486,40 +486,69 @@ static HRESULT WINAPI IAutoComplete2_fnInit( if (This->options & ACO_AUTOSUGGEST) create_listbox(This); - if (pwzsRegKeyPath) { - WCHAR *key; - WCHAR result[MAX_PATH]; - WCHAR *value; - HKEY hKey = 0; - LONG res; - LONG len; - - /* pwszRegKeyPath contains the key as well as the value, so we split */ - key = heap_alloc((lstrlenW(pwzsRegKeyPath)+1)*sizeof(WCHAR)); - strcpyW(key, pwzsRegKeyPath); - value = strrchrW(key, '\\'); - *value = 0; - value++; - /* Now value contains the value and buffer the key */ - res = RegOpenKeyExW(HKEY_CURRENT_USER, key, 0, KEY_READ, &hKey); - if (res != ERROR_SUCCESS) { - /* if the key is not found, MSDN states we must seek in HKEY_LOCAL_MACHINE */ - res = RegOpenKeyExW(HKEY_LOCAL_MACHINE, key, 0, KEY_READ, &hKey); - } - if (res == ERROR_SUCCESS) { - res = RegQueryValueW(hKey, value, result, &len); - if (res == ERROR_SUCCESS) { - This->quickComplete = heap_alloc(len*sizeof(WCHAR)); - strcpyW(This->quickComplete, result); - } - RegCloseKey(hKey); - } - heap_free(key); + if (pwzsRegKeyPath) + { + WCHAR *key, *value; + DWORD type, sz = MAX_PATH * sizeof(WCHAR); + BYTE *qc; + HKEY hKey; + LSTATUS res; + size_t len; + + /* pwszRegKeyPath contains the key as well as the value, so split it */ + value = strrchrW(pwzsRegKeyPath, '\\'); + len = value - pwzsRegKeyPath; + + if (value && (key = heap_alloc((len+1) * sizeof(*key))) != NULL) { + memcpy(key, pwzsRegKeyPath, len * sizeof(*key)); + key[len] = '\0'; + value++; + + res = RegOpenKeyExW(HKEY_CURRENT_USER, key, 0, KEY_READ, &hKey); + + /* if not found, MSDN states we must seek in HKEY_LOCAL_MACHINE */ + if (res != ERROR_SUCCESS) + res = RegOpenKeyExW(HKEY_LOCAL_MACHINE, key, 0, KEY_READ, &hKey); + + if (res == ERROR_SUCCESS && (qc = heap_alloc(sz)) != NULL) { + res = RegQueryValueExW(hKey, value, NULL, &type, qc, &sz); + + if ((res != ERROR_SUCCESS && res != ERROR_MORE_DATA) || type != REG_SZ) + { + RegCloseKey(hKey); + res = RegOpenKeyExW(HKEY_LOCAL_MACHINE, key, 0, KEY_READ, &hKey); + if (res != ERROR_SUCCESS) { + heap_free(qc); + goto free_key_str; + } + sz = MAX_PATH * sizeof(WCHAR); + res = RegQueryValueExW(hKey, value, NULL, &type, qc, &sz); + } + + for (;;) { + if (res == ERROR_SUCCESS && type == REG_SZ) { + This->quickComplete = heap_realloc(qc, sz); + break; + } + heap_free(qc); + if (res != ERROR_MORE_DATA || type != REG_SZ) + break; + if ((qc = heap_alloc(sz)) == NULL) + break; + res = RegQueryValueExW(hKey, value, NULL, &type, qc, &sz); + } + RegCloseKey(hKey); + } + free_key_str: + heap_free(key); + } } - if ((pwszQuickComplete) && (!This->quickComplete)) { - This->quickComplete = heap_alloc((lstrlenW(pwszQuickComplete)+1)*sizeof(WCHAR)); - lstrcpyW(This->quickComplete, pwszQuickComplete); + if (!This->quickComplete && pwszQuickComplete) + { + size_t len = strlenW(pwszQuickComplete)+1; + if ((This->quickComplete = heap_alloc(len * sizeof(WCHAR))) != NULL) + memcpy(This->quickComplete, pwszQuickComplete, len * sizeof(WCHAR)); } return S_OK; -- 1.9.1

1 0

[PATCH 1/3] shell32/autocomplete: Don't crash when there's another AutoComplete object on the same edit control
by Gabriel Ivăncescu Aug. 30, 2018

Aug. 30, 2018

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=22333 Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/shell32/autocomplete.c | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 99ce23d..5fe48e9 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -93,6 +93,14 @@ static inline IAutoCompleteImpl *impl_from_IAutoCompleteDropDown(IAutoCompleteDr return CONTAINING_RECORD(iface, IAutoCompleteImpl, IAutoCompleteDropDown_iface); } +static void destroy_autocomplete_object(IAutoCompleteImpl *ac) +{ + ac->hwndEdit = NULL; + if (ac->hwndListBox) + DestroyWindow(ac->hwndListBox); + IAutoComplete2_Release(&ac->IAutoComplete2_iface); +} + /* Window procedure for autocompletion */ @@ -276,10 +284,7 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, RemovePropW(hwnd, autocomplete_propertyW); SetWindowLongPtrW(hwnd, GWLP_WNDPROC, (LONG_PTR)proc); - This->hwndEdit = NULL; - if (This->hwndListBox) - DestroyWindow(This->hwndListBox); - IAutoComplete2_Release(&This->IAutoComplete2_iface); + destroy_autocomplete_object(This); return CallWindowProcW(proc, hwnd, uMsg, wParam, lParam); } default: @@ -434,7 +439,7 @@ static HRESULT WINAPI IAutoComplete2_fnInit( LPCOLESTR pwzsRegKeyPath, LPCOLESTR pwszQuickComplete) { - IAutoCompleteImpl *This = impl_from_IAutoComplete2(iface); + IAutoCompleteImpl *Other, *This = impl_from_IAutoComplete2(iface); TRACE("(%p)->(%p, %p, %s, %s)\n", This, hwndEdit, punkACL, debugstr_w(pwzsRegKeyPath), debugstr_w(pwszQuickComplete)); @@ -461,10 +466,22 @@ static HRESULT WINAPI IAutoComplete2_fnInit( This->initialized = TRUE; This->hwndEdit = hwndEdit; - This->wpOrigEditProc = (WNDPROC) SetWindowLongPtrW( hwndEdit, GWLP_WNDPROC, (LONG_PTR) ACEditSubclassProc); - /* Keep at least one reference to the object until the edit window is destroyed. */ + + /* If another AutoComplete object was previously assigned to this edit control, + release it but keep the same callback on the control, to avoid an infinite + recursive loop in ACEditSubclassProc while the property is set to this object */ + Other = GetPropW(hwndEdit, autocomplete_propertyW); + SetPropW(hwndEdit, autocomplete_propertyW, This); + + if (Other && Other->initialized) { + This->wpOrigEditProc = Other->wpOrigEditProc; + destroy_autocomplete_object(Other); + } + else + This->wpOrigEditProc = (WNDPROC) SetWindowLongPtrW(hwndEdit, GWLP_WNDPROC, (LONG_PTR) ACEditSubclassProc); + + /* Keep at least one reference to the object until the edit window is destroyed */ IAutoComplete2_AddRef(&This->IAutoComplete2_iface); - SetPropW( hwndEdit, autocomplete_propertyW, This ); if (This->options & ACO_AUTOSUGGEST) create_listbox(This); -- 1.9.1

2 7

[v2 PATCH 1/2] include: Add IOpcFactory and related definitions.
by Nikolay Sivov Aug. 30, 2018

Aug. 30, 2018

Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- v2: fixed up includes in main idl. include/Makefile.in | 4 ++ include/msopc.idl | 89 ++++++++++++++++++++++++++++++++++++++ include/opcbase.idl | 28 ++++++++++++ include/opcobjectmodel.idl | 34 +++++++++++++++ include/opcparturi.idl | 71 ++++++++++++++++++++++++++++++ 5 files changed, 226 insertions(+) create mode 100644 include/msopc.idl create mode 100644 include/opcbase.idl create mode 100644 include/opcobjectmodel.idl create mode 100644 include/opcparturi.idl diff --git a/include/Makefile.in b/include/Makefile.in index a89fef6eac..ffecb98d9e 100644 --- a/include/Makefile.in +++ b/include/Makefile.in @@ -399,6 +399,7 @@ SOURCES = \ msident.idl \ msinkaut.idl \ msiquery.h \ + msopc.idl \ mssip.h \ msstkppg.h \ mstask.idl \ @@ -493,6 +494,9 @@ SOURCES = \ oledberr.h \ oledlg.h \ oleidl.idl \ + opcbase.idl \ + opcobjectmodel.idl \ + opcparturi.idl \ opnrst.idl \ optary.idl \ patchapi.h \ diff --git a/include/msopc.idl b/include/msopc.idl new file mode 100644 index 0000000000..12404e5a12 --- /dev/null +++ b/include/msopc.idl @@ -0,0 +1,89 @@ +/* + * Copyright 2018 Nikolay Sivov for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +import "unknwn.idl"; +import "ocidl.idl"; + +#include "opcbase.idl" +#include "opcobjectmodel.idl" +#include "opcparturi.idl" + +#ifndef __WIDL__ +#define threading(model) +#endif + +interface IOpcUri; +interface IOpcPartUri; +interface IOpcPackage; +interface IOpcDigitalSignatureManager; + +[ + object, + uuid(6d0b4446-cd73-4ab3-94f4-8ccdf6116154), + pointer_default(ref) +] +interface IOpcFactory : IUnknown +{ + HRESULT CreatePackageRootUri( + [out, retval] IOpcUri **uri + ); + + HRESULT CreatePartUri( + [in, string] LPCWSTR uri, + [out, retval] IOpcPartUri **part_uri + ); + + [local] + HRESULT CreateStreamOnFile( + [in, string] LPCWSTR filename, + [in] OPC_STREAM_IO_MODE io_mode, + [in, unique] SECURITY_ATTRIBUTES *sa, + [in] DWORD flags, + [out, retval] IStream **stream + ); + + HRESULT CreatePackage( + [out, retval] IOpcPackage **package + ); + + HRESULT ReadPackageFromStream( + [in] IStream *stream, + [in] OPC_READ_FLAGS flags, + [out, retval] IOpcPackage **package + ); + + HRESULT WritePackageToStream( + [in] IOpcPackage *package, + [in] OPC_WRITE_FLAGS flags, + [in] IStream *stream + ); + + HRESULT CreateDigitalSignatureManager( + [in] IOpcPackage *package, + [out, retval] IOpcDigitalSignatureManager **signature_manager + ); +} + +[ + threading(both), + uuid(6b2d6Ba0-9f3e-4f27-920b-313cc426a39e) +] +coclass OpcFactory +{ + interface IOpcFactory; +}; diff --git a/include/opcbase.idl b/include/opcbase.idl new file mode 100644 index 0000000000..2cfb885c12 --- /dev/null +++ b/include/opcbase.idl @@ -0,0 +1,28 @@ +/* + * Copyright 2018 Nikolay Sivov for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#if 0 +#pragma makedep install +#endif + +typedef [v1_enum] enum +{ + OPC_STREAM_IO_READ = 1, + OPC_STREAM_IO_WRITE = 2, +} OPC_STREAM_IO_MODE; + diff --git a/include/opcobjectmodel.idl b/include/opcobjectmodel.idl new file mode 100644 index 0000000000..e5370d2e54 --- /dev/null +++ b/include/opcobjectmodel.idl @@ -0,0 +1,34 @@ +/* + * Copyright 2018 Nikolay Sivov for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#if 0 +#pragma makedep install +#endif + +typedef [v1_enum] enum +{ + OPC_READ_DEFAULT = 0, + OPC_VALIDATE_ON_LOAD = 1, + OPC_CACHE_ON_ACCESS = 2, +} OPC_READ_FLAGS; + +typedef [v1_enum] enum +{ + OPC_WRITE_DEFAULT = 0, + OPC_WRITE_FORCE_ZIP32 = 1, +} OPC_WRITE_FLAGS; diff --git a/include/opcparturi.idl b/include/opcparturi.idl new file mode 100644 index 0000000000..00b21b9598 --- /dev/null +++ b/include/opcparturi.idl @@ -0,0 +1,71 @@ +/* + * Copyright 2018 Nikolay Sivov for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +import "unknwn.idl"; +import "ocidl.idl"; + +#if 0 +#pragma makedep install +#endif + +interface IOpcPartUri; + +[ + object, + uuid(bc9c1b9b-d62c-49eb-aef0-3b4e0b28ebed), + nonextensible, + pointer_default(ref) +] +interface IOpcUri : IUri +{ + HRESULT GetRelationshipsPartUri( + [out, retval] IOpcPartUri **part_uri + ); + + HRESULT GetRelativeUri( + [in] IOpcPartUri *part_uri, + [out, retval] IUri **relative_uri + ); + + HRESULT CombinePartUri( + [in] IUri *relative_uri, + [out, retval] IOpcPartUri **combined + ); +} + +[ + object, + uuid(7d3babe7-88b2-46ba-85cb-4203cb016c87), + nonextensible, + pointer_default(ref) +] +interface IOpcPartUri : IOpcUri +{ + HRESULT ComparePartUri( + [in] IOpcPartUri *part_uri, + [out, retval] INT32 *result + ); + + HRESULT GetSourceUri( + [out, retval] IOpcUri **source_uri + ); + + HRESULT IsRelationshipsPartUri( + [out, retval] BOOL *result + ); +} -- 2.18.0

1 1

[PATCH 4/4] winevulkan: Free all command buffers when vkAllocateCommandBuffers() fails.
by Józef Kucia Aug. 30, 2018

Aug. 30, 2018

Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com> --- dlls/winevulkan/vulkan.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index f500602e96d5..4e0d999ddb41 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -540,25 +540,24 @@ VkResult WINAPI wine_vkAllocateCommandBuffers(VkDevice device, buffers[i]->base.loader_magic = VULKAN_ICD_MAGIC_VALUE; buffers[i]->device = device; + list_add_tail(&pool->command_buffers, &buffers[i]->pool_link); res = device->funcs.p_vkAllocateCommandBuffers(device->device, &allocate_info_host, &buffers[i]->command_buffer); if (res != VK_SUCCESS) { ERR("Failed to allocate command buffer, res=%d.\n", res); + buffers[i]->command_buffer = VK_NULL_HANDLE; break; } - - list_add_tail(&pool->command_buffers, &buffers[i]->pool_link); } if (res != VK_SUCCESS) { - wine_vk_free_command_buffers(device, pool, i, buffers); + wine_vk_free_command_buffers(device, pool, i + 1, buffers); memset(buffers, 0, allocate_info->commandBufferCount * sizeof(*buffers)); - return res; } - return VK_SUCCESS; + return res; } void WINAPI wine_vkCmdExecuteCommands(VkCommandBuffer buffer, uint32_t count, -- 2.16.4

1 0

[PATCH 3/4] vulkan-1/tests: Add a test for destroying a command pool.
by Józef Kucia Aug. 30, 2018

Aug. 30, 2018

The Vulkan spec says: "When a pool is destroyed, all command buffers allocated from the pool are implicitly freed and become invalid. Command buffers allocated from a given pool do not need to be freed before destroying that command pool." Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com> --- dlls/vulkan-1/tests/vulkan.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/dlls/vulkan-1/tests/vulkan.c b/dlls/vulkan-1/tests/vulkan.c index 1d2f66f5f901..20a245238b02 100644 --- a/dlls/vulkan-1/tests/vulkan.c +++ b/dlls/vulkan-1/tests/vulkan.c @@ -232,6 +232,45 @@ static void test_physical_device_groups(void) vkDestroyInstance(vk_instance, NULL); } +static void test_destroy_command_pool(VkPhysicalDevice vk_physical_device) +{ + VkCommandBufferAllocateInfo allocate_info; + VkCommandPoolCreateInfo pool_info; + VkCommandBuffer vk_cmd_buffers[4]; + uint32_t queue_family_index; + VkCommandPool vk_cmd_pool; + VkDevice vk_device; + VkResult vr; + + if ((vr = create_device(vk_physical_device, 0, NULL, NULL, &vk_device)) < 0) + { + skip("Failed to create device, vr %d.\n", vr); + return; + } + + find_queue_family(vk_physical_device, VK_QUEUE_GRAPHICS_BIT, &queue_family_index); + + pool_info.sType = VK_STRUCTURE_TYPE_COMMAND_POOL_CREATE_INFO; + pool_info.pNext = NULL; + pool_info.flags = 0; + pool_info.queueFamilyIndex = queue_family_index; + vr = vkCreateCommandPool(vk_device, &pool_info, NULL, &vk_cmd_pool); + ok(vr == VK_SUCCESS, "Got unexpected VkResult %d.\n", vr); + + allocate_info.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO; + allocate_info.pNext = NULL; + allocate_info.commandPool = vk_cmd_pool; + allocate_info.level = VK_COMMAND_BUFFER_LEVEL_PRIMARY; + allocate_info.commandBufferCount = ARRAY_SIZE(vk_cmd_buffers); + vr = vkAllocateCommandBuffers(vk_device, &allocate_info, vk_cmd_buffers); + ok(vr == VK_SUCCESS, "Got unexpected VkResult %d.\n", vr); + + vkDestroyCommandPool(vk_device, vk_cmd_pool, NULL); + vkDestroyCommandPool(vk_device, VK_NULL_HANDLE, NULL); + + vkDestroyDevice(vk_device, NULL); +} + static void for_each_device(void (*test_func)(VkPhysicalDevice)) { VkPhysicalDevice *vk_physical_devices; @@ -272,4 +311,5 @@ START_TEST(vulkan) for_each_device(enumerate_physical_device); for_each_device(enumerate_device_queues); test_physical_device_groups(); + for_each_device(test_destroy_command_pool); } -- 2.16.4

1 0

[PATCH 2/4] winevulkan: Fix potential memory leaks when a command pool is destroyed.
by Józef Kucia Aug. 30, 2018

Aug. 30, 2018

Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com> --- dlls/winevulkan/vulkan.c | 22 ++++++++++++++++++---- dlls/winevulkan/vulkan_private.h | 5 +++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index 886ef0b47344..f500602e96d5 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -165,6 +165,7 @@ static void wine_vk_free_command_buffers(struct VkDevice_T *device, continue; device->funcs.p_vkFreeCommandBuffers(device->device, pool->command_pool, 1, &buffers[i]->command_buffer); + list_remove(&buffers[i]->pool_link); heap_free(buffers[i]); } } @@ -528,9 +529,8 @@ VkResult WINAPI wine_vkAllocateCommandBuffers(VkDevice device, allocate_info_host.level = allocate_info->level; allocate_info_host.commandBufferCount = 1; - TRACE("Creating command buffer %u, pool 0x%s, level %#x\n", i, - wine_dbgstr_longlong(allocate_info_host.commandPool), - allocate_info_host.level); + TRACE("Allocating command buffer %u from pool 0x%s.\n", + i, wine_dbgstr_longlong(allocate_info_host.commandPool)); if (!(buffers[i] = heap_alloc_zero(sizeof(**buffers)))) { @@ -544,9 +544,11 @@ VkResult WINAPI wine_vkAllocateCommandBuffers(VkDevice device, &allocate_info_host, &buffers[i]->command_buffer); if (res != VK_SUCCESS) { - ERR("Failed to allocate command buffer, res=%d\n", res); + ERR("Failed to allocate command buffer, res=%d.\n", res); break; } + + list_add_tail(&pool->command_buffers, &buffers[i]->pool_link); } if (res != VK_SUCCESS) @@ -1132,6 +1134,8 @@ VkResult WINAPI wine_vkCreateCommandPool(VkDevice device, const VkCommandPoolCre if (!(object = heap_alloc_zero(sizeof(*object)))) return VK_ERROR_OUT_OF_HOST_MEMORY; + list_init(&object->command_buffers); + res = device->funcs.p_vkCreateCommandPool(device->device, info, NULL, &object->command_pool); if (res == VK_SUCCESS) @@ -1146,6 +1150,7 @@ void WINAPI wine_vkDestroyCommandPool(VkDevice device, VkCommandPool handle, const VkAllocationCallbacks *allocator) { struct wine_cmd_pool *pool = wine_cmd_pool_from_handle(handle); + struct VkCommandBuffer_T *buffer, *cursor; TRACE("%p, 0x%s, %p\n", device, wine_dbgstr_longlong(handle), allocator); @@ -1155,6 +1160,15 @@ void WINAPI wine_vkDestroyCommandPool(VkDevice device, VkCommandPool handle, if (allocator) FIXME("Support for allocation callbacks not implemented yet\n"); + /* The Vulkan spec says: + * + * "When a pool is destroyed, all command buffers allocated from the pool are freed." + */ + LIST_FOR_EACH_ENTRY_SAFE(buffer, cursor, &pool->command_buffers, struct VkCommandBuffer_T, pool_link) + { + heap_free(buffer); + } + device->funcs.p_vkDestroyCommandPool(device->device, pool->command_pool, NULL); heap_free(pool); } diff --git a/dlls/winevulkan/vulkan_private.h b/dlls/winevulkan/vulkan_private.h index f31d4d5d8f52..17072d234193 100644 --- a/dlls/winevulkan/vulkan_private.h +++ b/dlls/winevulkan/vulkan_private.h @@ -27,6 +27,7 @@ #include "wine/debug.h" #include "wine/heap.h" +#include "wine/list.h" #define VK_NO_PROTOTYPES #include "wine/vulkan.h" #include "wine/vulkan_driver.h" @@ -62,6 +63,8 @@ struct VkCommandBuffer_T struct wine_vk_base base; struct VkDevice_T *device; /* parent */ VkCommandBuffer command_buffer; /* native command buffer */ + + struct list pool_link; }; struct VkDevice_T @@ -113,6 +116,8 @@ struct VkQueue_T struct wine_cmd_pool { VkCommandPool command_pool; + + struct list command_buffers; }; static inline struct wine_cmd_pool *wine_cmd_pool_from_handle(VkCommandPool handle) -- 2.16.4

1 0

[PATCH 1/4] winevulkan: Wrap VkCommandPools.
by Józef Kucia Aug. 30, 2018

Aug. 30, 2018

We need to wrap VkCommandPools to track allocated VkCommandBuffers. Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com> --- dlls/winevulkan/make_vulkan | 50 +++++++++++++++-------------- dlls/winevulkan/vulkan.c | 68 +++++++++++++++++++++++++++++++++------- dlls/winevulkan/vulkan_private.h | 15 +++++++++ dlls/winevulkan/vulkan_thunks.c | 18 ++--------- dlls/winevulkan/vulkan_thunks.h | 2 ++ 5 files changed, 103 insertions(+), 50 deletions(-) diff --git a/dlls/winevulkan/make_vulkan b/dlls/winevulkan/make_vulkan index 5b0f82ce355a..8e7d88ad7dec 100755 --- a/dlls/winevulkan/make_vulkan +++ b/dlls/winevulkan/make_vulkan @@ -157,6 +157,8 @@ FUNCTION_OVERRIDES = { # Device functions "vkAllocateCommandBuffers" : {"dispatch" : True, "driver" : False, "thunk" : False}, "vkCmdExecuteCommands" : {"dispatch" : True, "driver" : False, "thunk" : False}, + "vkCreateCommandPool" : {"dispatch": True, "driver" : False, "thunk" : False}, + "vkDestroyCommandPool" : {"dispatch": True, "driver" : False, "thunk" : False}, "vkDestroyDevice" : {"dispatch" : True, "driver" : False, "thunk" : False}, "vkFreeCommandBuffers" : {"dispatch" : True, "driver" : False, "thunk" : False}, "vkGetDeviceProcAddr" : {"dispatch" : False, "driver" : True, "thunk" : False}, @@ -838,27 +840,31 @@ class VkHandle(object): def is_required(self): return self.required - def native_handle(self): - """ Provide access to the native handle of a dispatchable object. + def native_handle(self, name): + """ Provide access to the native handle of a wrapped object. """ - Dispatchable objects wrap an underlying 'native' object. - This method provides access to the native object. - """ - if not self.is_dispatchable(): - return None + if self.name == "VkCommandPool": + return "wine_cmd_pool_from_handle({0})->command_pool".format(name) + + native_handle_name = None if self.name == "VkCommandBuffer": - return "command_buffer" - elif self.name == "VkDevice": - return "device" - elif self.name == "VkInstance": - return "instance" - elif self.name == "VkPhysicalDevice": - return "phys_dev" - elif self.name == "VkQueue": - return "queue" - else: + native_handle_name = "command_buffer" + if self.name == "VkDevice": + native_handle_name = "device" + if self.name == "VkInstance": + native_handle_name = "instance" + if self.name == "VkPhysicalDevice": + native_handle_name = "phys_dev" + if self.name == "VkQueue": + native_handle_name = "queue" + + if native_handle_name: + return "{0}->{1}".format(name, native_handle_name) + + if self.is_dispatchable(): LOGGER.error("Unhandled native handle for: {0}".format(self.name)) + return None class VkMember(object): @@ -1471,17 +1477,15 @@ class VkParam(object): LOGGER.debug("TODO: setting NULL VkAllocationCallbacks for {0}".format(self.name)) return "NULL" - # Dispatchable objects wrap the native handle. For thunk generation we - # need to pass the native handle to the native Vulkan calls. - if self.is_dispatchable(): - return "{0}->{1}".format(self.name, self.handle.native_handle()) - elif conv and self.needs_conversion(): + if conv and self.needs_conversion(): if self.is_dynamic_array(): return "{0}_host".format(self.name) else: return "&{0}_host".format(self.name) else: - return self.name + # We need to pass the native handle to the native Vulkan calls. + native_handle = self.handle.native_handle(self.name) if self.is_handle() else None + return native_handle if native_handle else self.name class VkStruct(Sequence): diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index 89df838f0499..886ef0b47344 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -154,9 +154,8 @@ err: return NULL; } -/* Helper function to release command buffers. */ -static void wine_vk_command_buffers_free(struct VkDevice_T *device, VkCommandPool pool, - uint32_t count, const VkCommandBuffer *buffers) +static void wine_vk_free_command_buffers(struct VkDevice_T *device, + struct wine_cmd_pool *pool, uint32_t count, const VkCommandBuffer *buffers) { unsigned int i; @@ -165,12 +164,11 @@ static void wine_vk_command_buffers_free(struct VkDevice_T *device, VkCommandPoo if (!buffers[i]) continue; - device->funcs.p_vkFreeCommandBuffers(device->device, pool, 1, &buffers[i]->command_buffer); + device->funcs.p_vkFreeCommandBuffers(device->device, pool->command_pool, 1, &buffers[i]->command_buffer); heap_free(buffers[i]); } } -/* Helper function to create queues for a given family index. */ static struct VkQueue_T *wine_vk_device_alloc_queues(struct VkDevice_T *device, uint32_t family_index, uint32_t queue_count, VkDeviceQueueCreateFlags flags) { @@ -506,10 +504,13 @@ static void wine_vk_instance_free(struct VkInstance_T *instance) VkResult WINAPI wine_vkAllocateCommandBuffers(VkDevice device, const VkCommandBufferAllocateInfo *allocate_info, VkCommandBuffer *buffers) { + struct wine_cmd_pool *pool; VkResult res = VK_SUCCESS; unsigned int i; - TRACE("%p %p %p\n", device, allocate_info, buffers); + TRACE("%p, %p, %p\n", device, allocate_info, buffers); + + pool = wine_cmd_pool_from_handle(allocate_info->commandPool); memset(buffers, 0, allocate_info->commandBufferCount * sizeof(*buffers)); @@ -523,7 +524,7 @@ VkResult WINAPI wine_vkAllocateCommandBuffers(VkDevice device, /* TODO: future extensions (none yet) may require pNext conversion. */ allocate_info_host.pNext = allocate_info->pNext; allocate_info_host.sType = allocate_info->sType; - allocate_info_host.commandPool = allocate_info->commandPool; + allocate_info_host.commandPool = pool->command_pool; allocate_info_host.level = allocate_info->level; allocate_info_host.commandBufferCount = 1; @@ -550,7 +551,7 @@ VkResult WINAPI wine_vkAllocateCommandBuffers(VkDevice device, if (res != VK_SUCCESS) { - wine_vk_command_buffers_free(device, allocate_info->commandPool, i, buffers); + wine_vk_free_command_buffers(device, pool, i, buffers); memset(buffers, 0, allocate_info->commandBufferCount * sizeof(*buffers)); return res; } @@ -920,12 +921,14 @@ VkResult WINAPI wine_vkEnumeratePhysicalDevices(VkInstance instance, uint32_t *c return *count < instance->phys_dev_count ? VK_INCOMPLETE : VK_SUCCESS; } -void WINAPI wine_vkFreeCommandBuffers(VkDevice device, VkCommandPool pool, uint32_t count, - const VkCommandBuffer *buffers) +void WINAPI wine_vkFreeCommandBuffers(VkDevice device, VkCommandPool pool_handle, + uint32_t count, const VkCommandBuffer *buffers) { - TRACE("%p 0x%s %u %p\n", device, wine_dbgstr_longlong(pool), count, buffers); + struct wine_cmd_pool *pool = wine_cmd_pool_from_handle(pool_handle); - wine_vk_command_buffers_free(device, pool, count, buffers); + TRACE("%p, 0x%s, %u, %p\n", device, wine_dbgstr_longlong(pool_handle), count, buffers); + + wine_vk_free_command_buffers(device, pool, count, buffers); } PFN_vkVoidFunction WINAPI wine_vkGetDeviceProcAddr(VkDevice device, const char *name) @@ -1115,6 +1118,47 @@ err: return res; } +VkResult WINAPI wine_vkCreateCommandPool(VkDevice device, const VkCommandPoolCreateInfo *info, + const VkAllocationCallbacks *allocator, VkCommandPool *command_pool) +{ + struct wine_cmd_pool *object; + VkResult res; + + TRACE("%p, %p, %p, %p\n", device, info, allocator, command_pool); + + if (allocator) + FIXME("Support for allocation callbacks not implemented yet\n"); + + if (!(object = heap_alloc_zero(sizeof(*object)))) + return VK_ERROR_OUT_OF_HOST_MEMORY; + + res = device->funcs.p_vkCreateCommandPool(device->device, info, NULL, &object->command_pool); + + if (res == VK_SUCCESS) + *command_pool = wine_cmd_pool_to_handle(object); + else + heap_free(object); + + return res; +} + +void WINAPI wine_vkDestroyCommandPool(VkDevice device, VkCommandPool handle, + const VkAllocationCallbacks *allocator) +{ + struct wine_cmd_pool *pool = wine_cmd_pool_from_handle(handle); + + TRACE("%p, 0x%s, %p\n", device, wine_dbgstr_longlong(handle), allocator); + + if (!handle) + return; + + if (allocator) + FIXME("Support for allocation callbacks not implemented yet\n"); + + device->funcs.p_vkDestroyCommandPool(device->device, pool->command_pool, NULL); + heap_free(pool); +} + static VkResult wine_vk_enumerate_physical_device_groups(struct VkInstance_T *instance, VkResult (*p_vkEnumeratePhysicalDeviceGroups)(VkInstance, uint32_t *, VkPhysicalDeviceGroupProperties *), uint32_t *count, VkPhysicalDeviceGroupProperties *properties) diff --git a/dlls/winevulkan/vulkan_private.h b/dlls/winevulkan/vulkan_private.h index 2e071a524b1c..f31d4d5d8f52 100644 --- a/dlls/winevulkan/vulkan_private.h +++ b/dlls/winevulkan/vulkan_private.h @@ -110,6 +110,21 @@ struct VkQueue_T VkDeviceQueueCreateFlags flags; }; +struct wine_cmd_pool +{ + VkCommandPool command_pool; +}; + +static inline struct wine_cmd_pool *wine_cmd_pool_from_handle(VkCommandPool handle) +{ + return (struct wine_cmd_pool *)(uintptr_t)handle; +} + +static inline VkCommandPool wine_cmd_pool_to_handle(struct wine_cmd_pool *cmd_pool) +{ + return (VkCommandPool)(uintptr_t)cmd_pool; +} + void *wine_vk_get_device_proc_addr(const char *name) DECLSPEC_HIDDEN; void *wine_vk_get_instance_proc_addr(const char *name) DECLSPEC_HIDDEN; diff --git a/dlls/winevulkan/vulkan_thunks.c b/dlls/winevulkan/vulkan_thunks.c index 9d637bf9eb92..f5cadd8c72b6 100644 --- a/dlls/winevulkan/vulkan_thunks.c +++ b/dlls/winevulkan/vulkan_thunks.c @@ -1744,12 +1744,6 @@ VkResult WINAPI wine_vkCreateBufferView(VkDevice device, const VkBufferViewCreat #endif } -VkResult WINAPI wine_vkCreateCommandPool(VkDevice device, const VkCommandPoolCreateInfo *pCreateInfo, const VkAllocationCallbacks *pAllocator, VkCommandPool *pCommandPool) -{ - TRACE("%p, %p, %p, %p\n", device, pCreateInfo, pAllocator, pCommandPool); - return device->funcs.p_vkCreateCommandPool(device->device, pCreateInfo, NULL, pCommandPool); -} - VkResult WINAPI wine_vkCreateComputePipelines(VkDevice device, VkPipelineCache pipelineCache, uint32_t createInfoCount, const VkComputePipelineCreateInfo *pCreateInfos, const VkAllocationCallbacks *pAllocator, VkPipeline *pPipelines) { #if defined(USE_STRUCT_CONVERSION) @@ -1979,12 +1973,6 @@ void WINAPI wine_vkDestroyBufferView(VkDevice device, VkBufferView bufferView, c device->funcs.p_vkDestroyBufferView(device->device, bufferView, NULL); } -void WINAPI wine_vkDestroyCommandPool(VkDevice device, VkCommandPool commandPool, const VkAllocationCallbacks *pAllocator) -{ - TRACE("%p, 0x%s, %p\n", device, wine_dbgstr_longlong(commandPool), pAllocator); - device->funcs.p_vkDestroyCommandPool(device->device, commandPool, NULL); -} - void WINAPI wine_vkDestroyDescriptorPool(VkDevice device, VkDescriptorPool descriptorPool, const VkAllocationCallbacks *pAllocator) { TRACE("%p, 0x%s, %p\n", device, wine_dbgstr_longlong(descriptorPool), pAllocator); @@ -2744,7 +2732,7 @@ VkResult WINAPI wine_vkResetCommandBuffer(VkCommandBuffer commandBuffer, VkComma VkResult WINAPI wine_vkResetCommandPool(VkDevice device, VkCommandPool commandPool, VkCommandPoolResetFlags flags) { TRACE("%p, 0x%s, %#x\n", device, wine_dbgstr_longlong(commandPool), flags); - return device->funcs.p_vkResetCommandPool(device->device, commandPool, flags); + return device->funcs.p_vkResetCommandPool(device->device, wine_cmd_pool_from_handle(commandPool)->command_pool, flags); } VkResult WINAPI wine_vkResetDescriptorPool(VkDevice device, VkDescriptorPool descriptorPool, VkDescriptorPoolResetFlags flags) @@ -2774,13 +2762,13 @@ VkResult WINAPI wine_vkSetEvent(VkDevice device, VkEvent event) void WINAPI wine_vkTrimCommandPool(VkDevice device, VkCommandPool commandPool, VkCommandPoolTrimFlags flags) { TRACE("%p, 0x%s, %#x\n", device, wine_dbgstr_longlong(commandPool), flags); - device->funcs.p_vkTrimCommandPool(device->device, commandPool, flags); + device->funcs.p_vkTrimCommandPool(device->device, wine_cmd_pool_from_handle(commandPool)->command_pool, flags); } static void WINAPI wine_vkTrimCommandPoolKHR(VkDevice device, VkCommandPool commandPool, VkCommandPoolTrimFlags flags) { TRACE("%p, 0x%s, %#x\n", device, wine_dbgstr_longlong(commandPool), flags); - device->funcs.p_vkTrimCommandPoolKHR(device->device, commandPool, flags); + device->funcs.p_vkTrimCommandPoolKHR(device->device, wine_cmd_pool_from_handle(commandPool)->command_pool, flags); } void WINAPI wine_vkUnmapMemory(VkDevice device, VkDeviceMemory memory) diff --git a/dlls/winevulkan/vulkan_thunks.h b/dlls/winevulkan/vulkan_thunks.h index 94e56bb97300..21f9b4817800 100644 --- a/dlls/winevulkan/vulkan_thunks.h +++ b/dlls/winevulkan/vulkan_thunks.h @@ -43,7 +43,9 @@ /* Functions for which we have custom implementations outside of the thunks. */ VkResult WINAPI wine_vkAllocateCommandBuffers(VkDevice device, const VkCommandBufferAllocateInfo *pAllocateInfo, VkCommandBuffer *pCommandBuffers); void WINAPI wine_vkCmdExecuteCommands(VkCommandBuffer commandBuffer, uint32_t commandBufferCount, const VkCommandBuffer *pCommandBuffers); +VkResult WINAPI wine_vkCreateCommandPool(VkDevice device, const VkCommandPoolCreateInfo *pCreateInfo, const VkAllocationCallbacks *pAllocator, VkCommandPool *pCommandPool); VkResult WINAPI wine_vkCreateDevice(VkPhysicalDevice physicalDevice, const VkDeviceCreateInfo *pCreateInfo, const VkAllocationCallbacks *pAllocator, VkDevice *pDevice); +void WINAPI wine_vkDestroyCommandPool(VkDevice device, VkCommandPool commandPool, const VkAllocationCallbacks *pAllocator); void WINAPI wine_vkDestroyDevice(VkDevice device, const VkAllocationCallbacks *pAllocator); void WINAPI wine_vkDestroyInstance(VkInstance instance, const VkAllocationCallbacks *pAllocator); VkResult WINAPI wine_vkEnumerateDeviceExtensionProperties(VkPhysicalDevice physicalDevice, const char *pLayerName, uint32_t *pPropertyCount, VkExtensionProperties *pProperties); -- 2.16.4

1 0

[PATCH 1/2] testbot/web: Make it possible to link to the top and bottom of the page.
by Francois Gouget Aug. 30, 2018

Aug. 30, 2018

The PageTitle anchor identifies the page's title while the PageEnd one points to the bottom of the page. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/lib/ObjectModel/CGI/CollectionPage.pm | 2 +- testbot/lib/ObjectModel/CGI/FormPage.pm | 2 +- testbot/lib/WineTestBot/CGI/PageBase.pm | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/testbot/lib/ObjectModel/CGI/CollectionPage.pm b/testbot/lib/ObjectModel/CGI/CollectionPage.pm index 6593afcd2..194379f40 100644 --- a/testbot/lib/ObjectModel/CGI/CollectionPage.pm +++ b/testbot/lib/ObjectModel/CGI/CollectionPage.pm @@ -63,7 +63,7 @@ sub GenerateTitle($) my $Title = $self->GetTitle(); if ($Title) { - print "<h1>", $self->escapeHTML($Title), "</h1>\n"; + print "<h1 id='PageTitle'>", $self->escapeHTML($Title), "</h1>\n"; } } diff --git a/testbot/lib/ObjectModel/CGI/FormPage.pm b/testbot/lib/ObjectModel/CGI/FormPage.pm index 3a41fe0e6..01f789489 100644 --- a/testbot/lib/ObjectModel/CGI/FormPage.pm +++ b/testbot/lib/ObjectModel/CGI/FormPage.pm @@ -86,7 +86,7 @@ sub GenerateTitle($) my $Title = $self->GetTitle(); if ($Title) { - print "<h1>", $self->CGI->escapeHTML($Title), "</h1>\n"; + print "<h1 id='PageTitle'>", $self->CGI->escapeHTML($Title), "</h1>\n"; } } diff --git a/testbot/lib/WineTestBot/CGI/PageBase.pm b/testbot/lib/WineTestBot/CGI/PageBase.pm index e5b2f2d3b..0eac6b17b 100644 --- a/testbot/lib/WineTestBot/CGI/PageBase.pm +++ b/testbot/lib/WineTestBot/CGI/PageBase.pm @@ -328,7 +328,7 @@ sub GenerateFooter($) print <<EOF; </div> - <b class="rbottom"><b class="r4"> </b><b class="r3"> </b><b class="r2"> </b><b class="r1"> </b></b> + <b id="PageEnd" class="rbottom"><b class="r4"> </b><b class="r3"> </b><b class="r2"> </b><b class="r1"> </b></b> </div> </div> -- 2.18.0

1 1

[PATCH 1/2] testbot/web: Highlight new errors on the JobDetails page.
by Francois Gouget Aug. 30, 2018

Aug. 30, 2018

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/bin/WineRunBuild.pl | 31 +++++++++++++++++++++++++++ testbot/bin/WineSendLog.pl | 2 +- testbot/lib/WineTestBot/LogUtils.pm | 21 +++++++++++------- testbot/lib/WineTestBot/Steps.pm | 16 +++++++++----- testbot/lib/WineTestBot/StepsTasks.pm | 16 +++++++++----- testbot/web/JobDetails.pl | 24 ++++++++++++++++++++- testbot/web/WineTestBot.css | 1 + 7 files changed, 91 insertions(+), 20 deletions(-) diff --git a/testbot/bin/WineRunBuild.pl b/testbot/bin/WineRunBuild.pl index 117b06551..11b41c67a 100755 --- a/testbot/bin/WineRunBuild.pl +++ b/testbot/bin/WineRunBuild.pl @@ -471,6 +471,37 @@ foreach my $TestInfo (values %{$Impacts->{Tests}}) $TA->Disconnect(); +# +# Grab a copy of the reference logs +# + +# Note that this may be a bit inaccurate right after a Wine commit. +# See WineSendLog.pl for more details. +my $LatestDir = "$DataDir/latest"; +foreach my $TestStep (@{$Job->Steps->GetItems()}) +{ + if (($TestStep->PreviousNo || 0) == $Step->No and + $TestStep->FileType =~ /^exe/) + { + foreach my $TestTask (@{$TestStep->Tasks->GetItems()}) + { + my $RefReport = $TestTask->VM->Name ."_". $TestStep->FileType .".report"; + for my $Suffix ("", ".err") + { + if (-f "$LatestDir/$RefReport$Suffix") + { + unlink "$StepDir/$RefReport$Suffix"; + if (!link "$LatestDir/$RefReport$Suffix", "$StepDir/$RefReport$Suffix") + { + Error "Could not link '$RefReport$Suffix': $!\n"; + } + } + } + } + } +} + + # # Wrap up # diff --git a/testbot/bin/WineSendLog.pl b/testbot/bin/WineSendLog.pl index a4b1c823d..36d47c3a6 100755 --- a/testbot/bin/WineSendLog.pl +++ b/testbot/bin/WineSendLog.pl @@ -288,7 +288,7 @@ EOF { my $LogErrors = $JobErrors->{$Key}->{$LogName}; my $RefFileName = "$DataDir/latest". $StepTask->VM->Name ."_$LogName"; - my ($NewGroups, $NewErrors) = GetNewLogErrors($RefFileName, $LogErrors->{Groups}, $LogErrors->{Errors}); + my ($NewGroups, $NewErrors, $_NewIndices) = GetNewLogErrors($RefFileName, $LogErrors->{Groups}, $LogErrors->{Errors}); if (!$NewGroups) { # There was no reference log (typical of build logs) diff --git a/testbot/lib/WineTestBot/LogUtils.pm b/testbot/lib/WineTestBot/LogUtils.pm index 17b9093aa..5120217c0 100644 --- a/testbot/lib/WineTestBot/LogUtils.pm +++ b/testbot/lib/WineTestBot/LogUtils.pm @@ -781,8 +781,9 @@ sub _GetLineKey($) 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. +Returns a list of error groups containing new errors, a hashtable containing +the list of new errors for each group, and a hashtable containing the indices +of the new errors in the input errors list for each group. =back =cut @@ -791,10 +792,10 @@ sub GetNewLogErrors($$$) { my ($RefFileName, $Groups, $Errors) = @_; - my ($_Dummy, $RefErrors, $NoLog) = GetLogErrors($RefFileName); - return (undef, undef) if ($NoLog); + my ($RefGroups, $RefErrors) = GetLogErrors($RefFileName); + return (undef, undef) if (!$RefGroups); - my (@NewGroups, %NewErrors); + my (@NewGroups, %NewErrors, %NewIndices); foreach my $GroupName (@$Groups) { if ($RefErrors->{$GroupName}) @@ -802,7 +803,7 @@ sub GetNewLogErrors($$$) my $Diff = Algorithm::Diff->new($RefErrors->{$GroupName}, $Errors->{$GroupName}, { keyGen => \&_GetLineKey }); - my $CurrentGroup; + my ($CurrentGroup, $CurrentIndices); while ($Diff->Next()) { # Skip if there are no new lines @@ -812,20 +813,24 @@ sub GetNewLogErrors($$$) { push @NewGroups, $GroupName; $CurrentGroup = $NewErrors{$GroupName} = []; + $CurrentIndices = $NewIndices{$GroupName} = {}; } push @$CurrentGroup, $Diff->Items(2); + $CurrentIndices->{$_} = 1 for ($Diff->Range(2)); } } else { - # This module did not have errors before, so every error is new + # This group did not have errors before, so every error is new push @NewGroups, $GroupName; $NewErrors{$GroupName} = $Errors->{$GroupName}; + $NewIndices{$GroupName} = {}; my $Last = @{$Errors->{$GroupName}} - 1; + $NewIndices{$GroupName}->{$_} = 1 for (0..$Last); } } - return (\@NewGroups, \%NewErrors); + return (\@NewGroups, \%NewErrors, \%NewIndices); } 1; diff --git a/testbot/lib/WineTestBot/Steps.pm b/testbot/lib/WineTestBot/Steps.pm index 2f5bdb070..ae1180c66 100644 --- a/testbot/lib/WineTestBot/Steps.pm +++ b/testbot/lib/WineTestBot/Steps.pm @@ -146,16 +146,22 @@ sub RmTree($) rmtree($Dir); } -sub GetFullFileName($) +sub GetFullFileName($;$) { - my ($self) = @_; + my ($self, $FileName) = @_; - return undef if (!defined $self->FileName); + $FileName = $self->FileName if (!defined $FileName); + return undef if (!defined $FileName); my ($JobId, $StepNo) = @{$self->GetMasterKey()}; my $Path = "$DataDir/jobs/$JobId/"; - $Path .= $self->PreviousNo ."/" if ($self->PreviousNo); - return $Path . $self->FileName; + foreach my $StepNo ($StepNo, $self->PreviousNo) + { + next if (!$StepNo); + my $Full = "$Path$StepNo/$FileName"; + return $Full if (-f $Full); + } + return $Path . $FileName; } sub UpdateStatus($$) diff --git a/testbot/lib/WineTestBot/StepsTasks.pm b/testbot/lib/WineTestBot/StepsTasks.pm index 4675194e6..a50f61ada 100644 --- a/testbot/lib/WineTestBot/StepsTasks.pm +++ b/testbot/lib/WineTestBot/StepsTasks.pm @@ -40,16 +40,22 @@ sub GetStepDir($) } # See WineTestBot::Step::GetFullFileName() -sub GetFullFileName($) +sub GetFullFileName($;$) { - my ($self) = @_; + my ($self, $FileName) = @_; - return undef if (!defined $self->FileName); + $FileName = $self->FileName if (!defined $FileName); + return undef if (!defined $FileName); my ($JobId, $_StepTaskId) = @{$self->GetMasterKey()}; my $Path = "$DataDir/jobs/$JobId/"; - $Path .= $self->PreviousNo ."/" if ($self->PreviousNo); - return $Path . $self->FileName; + foreach my $StepNo ($self->StepNo, $self->PreviousNo) + { + next if (!$StepNo); + my $Full = "$Path$StepNo/$FileName"; + return $Full if (-f $Full); + } + return $Path . $FileName; } sub GetTaskDir($) diff --git a/testbot/web/JobDetails.pl b/testbot/web/JobDetails.pl index 7dd6aa5e9..d8e55296d 100644 --- a/testbot/web/JobDetails.pl +++ b/testbot/web/JobDetails.pl @@ -25,6 +25,7 @@ package JobDetailsPage; use ObjectModel::CGI::CollectionPage; our @ISA = qw(ObjectModel::CGI::CollectionPage); +use File::Basename; use URI::Escape; use WineTestBot::Config; @@ -490,11 +491,32 @@ EOF } my $Summary = $LogSummaries->{$LogName}; + my $New; + if ($LogName =~ /\.report$/) + { + # Identify new errors in test reports + my $RefFileName = $StepTask->GetFullFileName($VM->Name ."_$LogName"); + (my $_NewGroups, my $_NewErrors, $New) = GetNewLogErrors($RefFileName, $Summary->{Groups}, $Summary->{Errors}); + } + foreach my $GroupName (@{$Summary->{Groups}}) { print "<div class='LogDllName'>$GroupName:</div>\n" if ($GroupName); + print "<pre><code>"; - print $self->escapeHTML($_), "\n" for (@{$Summary->{Errors}->{$GroupName}}); + my $ErrIndex = 0; + foreach my $Line (@{$Summary->{Errors}->{$GroupName}}) + { + if ($New and $New->{$GroupName}->{$ErrIndex}) + { + print "<span class='log-new'>", $self->escapeHTML($Line), "</span>\n"; + } + else + { + print $self->escapeHTML($Line), "\n"; + } + $ErrIndex++; + } print "</code></pre>\n"; } } diff --git a/testbot/web/WineTestBot.css b/testbot/web/WineTestBot.css index 55c61a300..df683f863 100644 --- a/testbot/web/WineTestBot.css +++ b/testbot/web/WineTestBot.css @@ -381,6 +381,7 @@ pre .log-error { color: red; } .log-boterror { color: #cc0052; } .log-diag { color: #e56300; } +.log-new { color: #e56e00; font-weight: bold; } a.title { color:inherit; text-decoration: none; } -- 2.18.0

1 1

[PATCH] testbot/WineSendLog: Report TestBot errors to the mailing list too.
by Francois Gouget Aug. 29, 2018

Aug. 29, 2018

TestBot errors may or may not be caused by the patch but either way they make it impossible to know if there are new errors. Thus the patch requires extra scrutiny anyway. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/bin/WineSendLog.pl | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/testbot/bin/WineSendLog.pl b/testbot/bin/WineSendLog.pl index b0efa8c33..a4b1c823d 100755 --- a/testbot/bin/WineSendLog.pl +++ b/testbot/bin/WineSendLog.pl @@ -279,13 +279,10 @@ EOF my $StepTask = $StepsTasks->GetItem($Key); my $TaskDir = $StepTask->GetTaskDir(); - if ($StepTask->Status eq "boterror") - { - # 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; - } + # Note: We could check $StepTask->Status for TestBot errors. However, + # whether they are caused by the patch or not, they prevent the TestBot + # from checking for new errors which justifies sending an email to the + # mailing list so that the patch receives greater scrutiny. foreach my $LogName (@{$JobErrors->{$Key}->{LogNames}}) { -- 2.18.0

1 0