April 2021 - Wine-Devel - list.winehq.org

[PATCH 1/4] shell32/tests: add more tests for Shell_MergeMenus()

by Damjan Jovanovic

Show Shell_MergeMenus() really adds the offset instead of changing IDs to start with it, and that it really honours the maximum allowed value. Signed-off-by: Damjan Jovanovic <damjan.jov(a)gmail.com> --- dlls/shell32/tests/shlfileop.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)

April 30, 2021

[PATCH] winegstreamer: Delay duration query until caps have been acquired.

by Zebediah Figura

From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/quartz/tests/waveparser.c | 2 +- dlls/winegstreamer/wg_parser.c | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/dlls/quartz/tests/waveparser.c b/dlls/quartz/tests/waveparser.c index f3ea8f2b482..f78e15e5c19 100644 --- a/dlls/quartz/tests/waveparser.c +++ b/dlls/quartz/tests/waveparser.c @@ -929,7 +929,7 @@ static void test_seeking(void) duration = 0xdeadbeef; hr = IMediaSeeking_GetDuration(seeking, &duration); ok(hr == S_OK, "Got hr %#x.\n", hr); - todo_wine ok(duration > 0, "Got duration %s.\n", wine_dbgstr_longlong(duration)); + ok(duration > 0, "Got duration %s.\n", wine_dbgstr_longlong(duration)); stop = current = 0xdeadbeef; hr = IMediaSeeking_GetStopPosition(seeking, &stop); diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index d6aceff8cfe..c06e5af8160 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -1553,7 +1553,6 @@ static HRESULT CDECL wg_parser_connect(struct wg_parser *parser, uint64_t file_s { struct wg_parser_stream *stream = parser->streams[i]; - stream->duration = query_duration(stream->their_src); while (!stream->has_caps && !parser->error) pthread_cond_wait(&parser->init_cond, &parser->mutex); if (parser->error) @@ -1561,6 +1560,12 @@ static HRESULT CDECL wg_parser_connect(struct wg_parser *parser, uint64_t file_s pthread_mutex_unlock(&parser->mutex); return E_FAIL; } + /* GStreamer doesn't actually provide any guarantees about when duration + * is available, even for seekable streams. However, many elements (e.g. + * avidemux, wavparse, qtdemux) in practice record duration before + * fixing caps, so as a heuristic, wait until we get caps before trying + * to query for duration. */ + stream->duration = query_duration(stream->their_src); } pthread_mutex_unlock(&parser->mutex); -- 2.30.2

April 29, 2021

Re: [PATCH 3/4] d3d11/tests: Initialise "constant" in test_device_context_state() (Valgrind).

by Marvin

Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=89538 Your paranoid android. === w1064v1809 (32 bit report) === d3d11: d3d11.c:5811: Test failed: Got unexpected IAVertices count: 0. d3d11.c:5812: Test failed: Got unexpected IAPrimitives count: 0. d3d11.c:5813: Test failed: Got unexpected VSInvocations count: 0. d3d11.c:5816: Test failed: Got unexpected CInvocations count: 0. d3d11.c:5817: Test failed: Got unexpected CPrimitives count: 0. === w1064 (32 bit report) === d3d11: d3d11.c:5811: Test failed: Got unexpected IAVertices count: 0. d3d11.c:5812: Test failed: Got unexpected IAPrimitives count: 0. d3d11.c:5813: Test failed: Got unexpected VSInvocations count: 0. d3d11.c:5816: Test failed: Got unexpected CInvocations count: 0. d3d11.c:5817: Test failed: Got unexpected CPrimitives count: 0. === w10pro64 (32 bit report) === d3d11: d3d11.c:5811: Test failed: Got unexpected IAVertices count: 0. d3d11.c:5812: Test failed: Got unexpected IAPrimitives count: 0. d3d11.c:5813: Test failed: Got unexpected VSInvocations count: 0. d3d11.c:5816: Test failed: Got unexpected CInvocations count: 0. d3d11.c:5817: Test failed: Got unexpected CPrimitives count: 0. === debiant2 (32 bit Chinese:China report) === d3d11: d3d11.c:9661: Test failed: d3d11.c:15027: Test marked todo: Got hr 0 for WRITE_DISCARD.

April 29, 2021

[PATCH 4/4] wined3d: Only read "*rect_count" when "rects" is non-NULL in wined3d_device_context_get_scissor_rects() (Valgrind).

by Henri Verbeet

When "rects" is NULL, "*rect_count" is potentially uninitialised. That's fairly benign because the resulting "count" value is inconsequential in that case, but it's also unnecessary, and easy to avoid. Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com> --- dlls/wined3d/device.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c index f3d7cff74bf..813ee773478 100644 --- a/dlls/wined3d/device.c +++ b/dlls/wined3d/device.c @@ -1722,8 +1722,7 @@ void CDECL wined3d_device_context_get_scissor_rects(const struct wined3d_device_ TRACE("context %p, rect_count %p, rects %p.\n", context, rect_count, rects); - count = rect_count ? min(*rect_count, state->scissor_rect_count) : 1; - if (count && rects) + if (rects && (count = rect_count ? min(*rect_count, state->scissor_rect_count) : 1)) memcpy(rects, state->scissor_rects, count * sizeof(*rects)); if (rect_count) *rect_count = state->scissor_rect_count; -- 2.20.1

April 29, 2021

[PATCH 3/4] d3d11/tests: Initialise "constant" in test_device_context_state() (Valgrind).

by Henri Verbeet

We're mostly just using this to make sure the value we read back is that same as the one we set, so in that sense the actual value is inconsequential. Nevertheless, randomness is generally undesirable in the tests. Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com> --- dlls/d3d11/tests/d3d11.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/d3d11/tests/d3d11.c b/dlls/d3d11/tests/d3d11.c index ca60170d080..191df67a41d 100644 --- a/dlls/d3d11/tests/d3d11.c +++ b/dlls/d3d11/tests/d3d11.c @@ -6775,6 +6775,7 @@ static void test_device_context_state(void) 0x00000008, 0x00000000, 0x00000008, 0x58454853, 0x00000020, 0x00050050, 0x00000008, 0x0100086a, 0x0400009b, 0x00000001, 0x00000001, 0x00000001, 0x0100003e, }; + static const struct vec4 constant = {1.257f, 1.885f, 2.513f, 3.770f}; ID3DDeviceContextState *context_state, *previous_context_state, *tmp_context_state, *context_state2; UINT ib_offset, vb_offset, vb_stride, so_offset, offset, stride, sample_mask, stencil_ref, count; @@ -6814,7 +6815,6 @@ static void test_device_context_state(void) ID3D11Texture2D *texture; enum DXGI_FORMAT format; float blend_factor[4]; - struct vec4 constant; DWORD data_size; BOOL pred_value; ULONG refcount; -- 2.20.1

April 29, 2021

[PATCH 2/4] wined3d: Free the program variants array in shader_spirv_destroy() (Valgrind).

by Henri Verbeet

Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com> --- dlls/wined3d/shader_spirv.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/wined3d/shader_spirv.c b/dlls/wined3d/shader_spirv.c index eb2647db12d..7617ee9d6ef 100644 --- a/dlls/wined3d/shader_spirv.c +++ b/dlls/wined3d/shader_spirv.c @@ -1019,6 +1019,7 @@ static void shader_spirv_destroy(struct wined3d_shader *shader) shader_spirv_invalidate_contexts_graphics_program_variant(&device_vk->d, variant_vk); VK_CALL(vkDestroyShaderModule(device_vk->vk_device, variant_vk->vk_module, NULL)); } + heap_free(program_vk->variants); vkd3d_shader_free_scan_descriptor_info(&program_vk->descriptor_info); shader->backend_data = NULL; -- 2.20.1

April 29, 2021

[PATCH 1/4] wined3d: Avoid accessing the bo_user list from outside the command stream thread (Valgrind).

by Henri Verbeet

Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com> --- dlls/wined3d/adapter_gl.c | 69 ++++++++++++++++------------------- dlls/wined3d/adapter_vk.c | 77 ++++++++++++++++++--------------------- 2 files changed, 68 insertions(+), 78 deletions(-) diff --git a/dlls/wined3d/adapter_gl.c b/dlls/wined3d/adapter_gl.c index 990991e2e3d..8787324571a 100644 --- a/dlls/wined3d/adapter_gl.c +++ b/dlls/wined3d/adapter_gl.c @@ -4755,6 +4755,7 @@ struct wined3d_view_gl_destroy_ctx { struct wined3d_device *device; const struct wined3d_gl_view *gl_view; + struct wined3d_bo_user *bo_user; struct wined3d_bo_gl *counter_bo; void *object; struct wined3d_view_gl_destroy_ctx *free; @@ -4787,13 +4788,15 @@ static void wined3d_view_gl_destroy_object(void *object) checkGLcall("delete resources"); context_release(context); } + if (ctx->bo_user) + list_remove(&ctx->bo_user->entry); heap_free(ctx->object); heap_free(ctx->free); } -static void wined3d_view_gl_destroy(struct wined3d_device *device, - const struct wined3d_gl_view *gl_view, struct wined3d_bo_gl *counter_bo, void *object) +static void wined3d_view_gl_destroy(struct wined3d_device *device, const struct wined3d_gl_view *gl_view, + struct wined3d_bo_user *bo_user, struct wined3d_bo_gl *counter_bo, void *object) { struct wined3d_view_gl_destroy_ctx *ctx, c; @@ -4801,6 +4804,7 @@ static void wined3d_view_gl_destroy(struct wined3d_device *device, ctx = &c; ctx->device = device; ctx->gl_view = gl_view; + ctx->bo_user = bo_user; ctx->counter_bo = counter_bo; ctx->object = object; ctx->free = ctx != &c ? ctx : NULL; @@ -4813,21 +4817,16 @@ static void wined3d_view_gl_destroy(struct wined3d_device *device, static void adapter_gl_destroy_rendertarget_view(struct wined3d_rendertarget_view *view) { struct wined3d_rendertarget_view_gl *view_gl = wined3d_rendertarget_view_gl(view); - struct wined3d_device *device = view_gl->v.resource->device; - unsigned int swapchain_count = device->swapchain_count; + struct wined3d_resource *resource = view_gl->v.resource; TRACE("view_gl %p.\n", view_gl); - /* Take a reference to the device, in case releasing the view's resource - * would cause the device to be destroyed. However, swapchain resources - * don't take a reference to the device, and we wouldn't want to increment - * the refcount on a device that's in the process of being destroyed. */ - if (swapchain_count) - wined3d_device_incref(device); + /* Take a reference to the resource, in case releasing the resource + * would cause the device to be destroyed. */ + wined3d_resource_incref(resource); wined3d_rendertarget_view_cleanup(&view_gl->v); - wined3d_view_gl_destroy(device, &view_gl->gl_view, NULL, view_gl); - if (swapchain_count) - wined3d_device_decref(device); + wined3d_view_gl_destroy(resource->device, &view_gl->gl_view, NULL, NULL, view_gl); + wined3d_resource_decref(resource); } static HRESULT adapter_gl_create_shader_resource_view(const struct wined3d_view_desc *desc, @@ -4859,22 +4858,20 @@ static HRESULT adapter_gl_create_shader_resource_view(const struct wined3d_view_ static void adapter_gl_destroy_shader_resource_view(struct wined3d_shader_resource_view *view) { struct wined3d_shader_resource_view_gl *view_gl = wined3d_shader_resource_view_gl(view); - struct wined3d_device *device = view_gl->v.resource->device; - unsigned int swapchain_count = device->swapchain_count; + struct wined3d_resource *resource = view_gl->v.resource; TRACE("view_gl %p.\n", view_gl); - /* Take a reference to the device, in case releasing the view's resource - * would cause the device to be destroyed. However, swapchain resources - * don't take a reference to the device, and we wouldn't want to increment - * the refcount on a device that's in the process of being destroyed. */ - if (swapchain_count) - wined3d_device_incref(device); - list_remove(&view_gl->bo_user.entry); + /* Take a reference to the resource. There are two reasons for this: + * - Releasing the resource could in turn cause the device to be + * destroyed, but we still need the device for + * wined3d_view_vk_destroy(). + * - We shouldn't free buffer resources until after we've removed the + * view from its bo_user list. */ + wined3d_resource_incref(resource); wined3d_shader_resource_view_cleanup(&view_gl->v); - wined3d_view_gl_destroy(device, &view_gl->gl_view, NULL, view_gl); - if (swapchain_count) - wined3d_device_decref(device); + wined3d_view_gl_destroy(resource->device, &view_gl->gl_view, &view_gl->bo_user, NULL, view_gl); + wined3d_resource_decref(resource); } static HRESULT adapter_gl_create_unordered_access_view(const struct wined3d_view_desc *desc, @@ -4906,22 +4903,20 @@ static HRESULT adapter_gl_create_unordered_access_view(const struct wined3d_view static void adapter_gl_destroy_unordered_access_view(struct wined3d_unordered_access_view *view) { struct wined3d_unordered_access_view_gl *view_gl = wined3d_unordered_access_view_gl(view); - struct wined3d_device *device = view_gl->v.resource->device; - unsigned int swapchain_count = device->swapchain_count; + struct wined3d_resource *resource = view_gl->v.resource; TRACE("view_gl %p.\n", view_gl); - /* Take a reference to the device, in case releasing the view's resource - * would cause the device to be destroyed. However, swapchain resources - * don't take a reference to the device, and we wouldn't want to increment - * the refcount on a device that's in the process of being destroyed. */ - if (swapchain_count) - wined3d_device_incref(device); - list_remove(&view_gl->bo_user.entry); + /* Take a reference to the resource. There are two reasons for this: + * - Releasing the resource could in turn cause the device to be + * destroyed, but we still need the device for + * wined3d_view_vk_destroy(). + * - We shouldn't free buffer resources until after we've removed the + * view from its bo_user list. */ + wined3d_resource_incref(resource); wined3d_unordered_access_view_cleanup(&view_gl->v); - wined3d_view_gl_destroy(device, &view_gl->gl_view, &view_gl->counter_bo, view_gl); - if (swapchain_count) - wined3d_device_decref(device); + wined3d_view_gl_destroy(resource->device, &view_gl->gl_view, &view_gl->bo_user, &view_gl->counter_bo, view_gl); + wined3d_resource_decref(resource); } static HRESULT adapter_gl_create_sampler(struct wined3d_device *device, const struct wined3d_sampler_desc *desc, diff --git a/dlls/wined3d/adapter_vk.c b/dlls/wined3d/adapter_vk.c index 0abae24fd1f..99d7547b834 100644 --- a/dlls/wined3d/adapter_vk.c +++ b/dlls/wined3d/adapter_vk.c @@ -1304,6 +1304,7 @@ struct wined3d_view_vk_destroy_ctx struct wined3d_device_vk *device_vk; VkBufferView *vk_buffer_view; VkImageView *vk_image_view; + struct wined3d_bo_user *bo_user; struct wined3d_bo_vk *vk_counter_bo; VkBufferView *vk_counter_view; uint64_t *command_buffer_id; @@ -1350,6 +1351,8 @@ static void wined3d_view_vk_destroy_object(void *object) TRACE("Destroyed image view 0x%s.\n", wine_dbgstr_longlong(*ctx->vk_image_view)); } } + if (ctx->bo_user) + list_remove(&ctx->bo_user->entry); if (ctx->vk_counter_bo && ctx->vk_counter_bo->vk_buffer) wined3d_context_vk_destroy_bo(wined3d_context_vk(context), ctx->vk_counter_bo); if (ctx->vk_counter_view) @@ -1374,7 +1377,7 @@ static void wined3d_view_vk_destroy_object(void *object) } static void wined3d_view_vk_destroy(struct wined3d_device *device, VkBufferView *vk_buffer_view, - VkImageView *vk_image_view, struct wined3d_bo_vk *vk_counter_bo, + VkImageView *vk_image_view, struct wined3d_bo_user *bo_user, struct wined3d_bo_vk *vk_counter_bo, VkBufferView *vk_counter_view, uint64_t *command_buffer_id, void *view_vk) { struct wined3d_view_vk_destroy_ctx *ctx, c; @@ -1384,6 +1387,7 @@ static void wined3d_view_vk_destroy(struct wined3d_device *device, VkBufferView ctx->device_vk = wined3d_device_vk(device); ctx->vk_buffer_view = vk_buffer_view; ctx->vk_image_view = vk_image_view; + ctx->bo_user = bo_user; ctx->vk_counter_bo = vk_counter_bo; ctx->vk_counter_view = vk_counter_view; ctx->command_buffer_id = command_buffer_id; @@ -1398,22 +1402,17 @@ static void wined3d_view_vk_destroy(struct wined3d_device *device, VkBufferView static void adapter_vk_destroy_rendertarget_view(struct wined3d_rendertarget_view *view) { struct wined3d_rendertarget_view_vk *view_vk = wined3d_rendertarget_view_vk(view); - struct wined3d_device *device = view_vk->v.resource->device; - unsigned int swapchain_count = device->swapchain_count; + struct wined3d_resource *resource = view_vk->v.resource; TRACE("view_vk %p.\n", view_vk); - /* Take a reference to the device, in case releasing the view's resource - * would cause the device to be destroyed. However, swapchain resources - * don't take a reference to the device, and we wouldn't want to increment - * the refcount on a device that's in the process of being destroyed. */ - if (swapchain_count) - wined3d_device_incref(device); + /* Take a reference to the resource, in case releasing the resource + * would cause the device to be destroyed. */ + wined3d_resource_incref(resource); wined3d_rendertarget_view_cleanup(&view_vk->v); - wined3d_view_vk_destroy(device, NULL, &view_vk->vk_image_view, - NULL, NULL, &view_vk->command_buffer_id, view_vk); - if (swapchain_count) - wined3d_device_decref(device); + wined3d_view_vk_destroy(resource->device, NULL, &view_vk->vk_image_view, + NULL, NULL, NULL, &view_vk->command_buffer_id, view_vk); + wined3d_resource_decref(resource); } static HRESULT adapter_vk_create_shader_resource_view(const struct wined3d_view_desc *desc, @@ -1445,30 +1444,28 @@ static HRESULT adapter_vk_create_shader_resource_view(const struct wined3d_view_ static void adapter_vk_destroy_shader_resource_view(struct wined3d_shader_resource_view *view) { struct wined3d_shader_resource_view_vk *srv_vk = wined3d_shader_resource_view_vk(view); - struct wined3d_device *device = srv_vk->v.resource->device; - unsigned int swapchain_count = device->swapchain_count; + struct wined3d_resource *resource = srv_vk->v.resource; struct wined3d_view_vk *view_vk = &srv_vk->view_vk; VkBufferView *vk_buffer_view = NULL; VkImageView *vk_image_view = NULL; TRACE("srv_vk %p.\n", srv_vk); - /* Take a reference to the device, in case releasing the view's resource - * would cause the device to be destroyed. However, swapchain resources - * don't take a reference to the device, and we wouldn't want to increment - * the refcount on a device that's in the process of being destroyed. */ - if (swapchain_count) - wined3d_device_incref(device); - if (srv_vk->v.resource->type == WINED3D_RTYPE_BUFFER) + /* Take a reference to the resource. There are two reasons for this: + * - Releasing the resource could in turn cause the device to be + * destroyed, but we still need the device for + * wined3d_view_vk_destroy(). + * - We shouldn't free buffer resources until after we've removed the + * view from its bo_user list. */ + wined3d_resource_incref(resource); + if (resource->type == WINED3D_RTYPE_BUFFER) vk_buffer_view = &view_vk->u.vk_buffer_view; else vk_image_view = &view_vk->u.vk_image_info.imageView; - list_remove(&view_vk->bo_user.entry); wined3d_shader_resource_view_cleanup(&srv_vk->v); - wined3d_view_vk_destroy(device, vk_buffer_view, vk_image_view, - NULL, NULL, &view_vk->command_buffer_id, srv_vk); - if (swapchain_count) - wined3d_device_decref(device); + wined3d_view_vk_destroy(resource->device, vk_buffer_view, vk_image_view, + &view_vk->bo_user, NULL, NULL, &view_vk->command_buffer_id, srv_vk); + wined3d_resource_decref(resource); } static HRESULT adapter_vk_create_unordered_access_view(const struct wined3d_view_desc *desc, @@ -1500,30 +1497,28 @@ static HRESULT adapter_vk_create_unordered_access_view(const struct wined3d_view static void adapter_vk_destroy_unordered_access_view(struct wined3d_unordered_access_view *view) { struct wined3d_unordered_access_view_vk *uav_vk = wined3d_unordered_access_view_vk(view); - struct wined3d_device *device = uav_vk->v.resource->device; - unsigned int swapchain_count = device->swapchain_count; + struct wined3d_resource *resource = uav_vk->v.resource; struct wined3d_view_vk *view_vk = &uav_vk->view_vk; VkBufferView *vk_buffer_view = NULL; VkImageView *vk_image_view = NULL; TRACE("uav_vk %p.\n", uav_vk); - /* Take a reference to the device, in case releasing the view's resource - * would cause the device to be destroyed. However, swapchain resources - * don't take a reference to the device, and we wouldn't want to increment - * the refcount on a device that's in the process of being destroyed. */ - if (swapchain_count) - wined3d_device_incref(device); - if (uav_vk->v.resource->type == WINED3D_RTYPE_BUFFER) + /* Take a reference to the resource. There are two reasons for this: + * - Releasing the resource could in turn cause the device to be + * destroyed, but we still need the device for + * wined3d_view_vk_destroy(). + * - We shouldn't free buffer resources until after we've removed the + * view from its bo_user list. */ + wined3d_resource_incref(resource); + if (resource->type == WINED3D_RTYPE_BUFFER) vk_buffer_view = &view_vk->u.vk_buffer_view; else vk_image_view = &view_vk->u.vk_image_info.imageView; - list_remove(&view_vk->bo_user.entry); wined3d_unordered_access_view_cleanup(&uav_vk->v); - wined3d_view_vk_destroy(device, vk_buffer_view, vk_image_view, &uav_vk->counter_bo, - &uav_vk->vk_counter_view, &view_vk->command_buffer_id, uav_vk); - if (swapchain_count) - wined3d_device_decref(device); + wined3d_view_vk_destroy(resource->device, vk_buffer_view, vk_image_view, &view_vk->bo_user, + &uav_vk->counter_bo, &uav_vk->vk_counter_view, &view_vk->command_buffer_id, uav_vk); + wined3d_resource_decref(resource); } static HRESULT adapter_vk_create_sampler(struct wined3d_device *device, const struct wined3d_sampler_desc *desc, -- 2.20.1

April 29, 2021

Re: [PATCH v9 1/2] ntdll: Strip trailing slashes after looking up unix name.

by Alexandre Julliard

Gabriel Ivăncescu <gabrielopcode(a)gmail.com> writes: > As they are useless anyway, and have inconsistencies, plus not useful > to new code that assumes there's not any on the unix side. See: > https://www.winehq.org/pipermail/wine-devel/2021-April/185850.html Actually, trailing slashes are in general not allowed on NT names, we have some todo tests about that. A single trailing slash on a directory name is OK, and I think your code should deal with that. -- Alexandre Julliard julliard(a)winehq.org

April 29, 2021

Re: [PATCH v2 2/2] winegstreamer: Wait for duration-changed message before duration query.

by Connor McAdams

On Thu, Apr 29, 2021 at 10:36:39PM +0700, Anton Baskanov wrote: > Unfortunately, this breaks WMA playback. Looks like GstASFDemux does not send > duration-changed, and the duration is available immediately. Yes, this was something I was afraid of. I guess the more correct behavior would be to check duration, and if it fails, then have the duration_changed check. Or, only enable the check depending on the format type. I will ask Zebediah for her opinion on this in IRC, if she doesn't get to this before then. > > On среда, 28 апреля 2021 г. 23:15:26 +07 you wrote: > > Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> > > --- > > dlls/winegstreamer/wg_parser.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c > > index f90362981df..2041cf713fc 100644 > > --- a/dlls/winegstreamer/wg_parser.c > > +++ b/dlls/winegstreamer/wg_parser.c > > @@ -1553,7 +1553,7 @@ static HRESULT CDECL wg_parser_connect(struct > > wg_parser *parser, uint64_t file_s { > > struct wg_parser_stream *stream = parser->streams[i]; > > > > - while (!stream->has_caps && !parser->error) > > + while ((!stream->has_caps || !parser->has_duration) && > > !parser->error) pthread_cond_wait(&parser->init_cond, &parser->mutex); > > stream->duration = query_duration(stream->their_src); > > if (parser->error) > > > >

April 29, 2021

[PATCH v9 2/2] ntdll: Allow renaming a file/directory to a different case of itself.

by Gabriel Ivăncescu

Renaming a file or directory from e.g. foobar to FooBar (or any other case change) should work, like on Windows, instead of being a no-op. Clobbering an existing file must also respect the new case. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46203 Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- Added special case for '/' root component, as it has no named component. Otherwise, \\?\unix\ for example would cause the case component to be 'unix' even though it refers to / dlls/kernel32/tests/file.c | 4 +- dlls/ntdll/tests/file.c | 4 +- dlls/ntdll/unix/file.c | 69 +++++++++++++++++++++++++-- server/fd.c | 98 +++++++++++++++++++++++++------------- server/protocol.def | 2 + 5 files changed, 137 insertions(+), 40 deletions(-) diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c index 8560524..9ca56e1 100644 --- a/dlls/kernel32/tests/file.c +++ b/dlls/kernel32/tests/file.c @@ -2040,7 +2040,7 @@ static void test_MoveFileA(void) ok(hfile != INVALID_HANDLE_VALUE, "FindFirstFileA: failed, error %d\n", GetLastError()); if (hfile != INVALID_HANDLE_VALUE) { - todo_wine ok(!lstrcmpA(strrchr(tempdir, '\\') + 1, find_data.cFileName), + ok(!lstrcmpA(strrchr(tempdir, '\\') + 1, find_data.cFileName), "MoveFile failed to change casing on same file: got %s\n", find_data.cFileName); } CloseHandle(hfile); @@ -2085,7 +2085,7 @@ static void test_MoveFileA(void) ok(hfile != INVALID_HANDLE_VALUE, "FindFirstFileA: failed, error %d\n", GetLastError()); if (hfile != INVALID_HANDLE_VALUE) { - todo_wine ok(!lstrcmpA(strrchr(tempdir, '\\') + 1, find_data.cFileName), + ok(!lstrcmpA(strrchr(tempdir, '\\') + 1, find_data.cFileName), "MoveFile failed to change casing on same directory: got %s\n", find_data.cFileName); } CloseHandle(hfile); diff --git a/dlls/ntdll/tests/file.c b/dlls/ntdll/tests/file.c index d469b44..49f4e34 100644 --- a/dlls/ntdll/tests/file.c +++ b/dlls/ntdll/tests/file.c @@ -2315,7 +2315,7 @@ static void test_file_link_information(void) ok(handle != INVALID_HANDLE_VALUE, "FindFirstFileW: failed, error %d\n", GetLastError()); if (handle != INVALID_HANDLE_VALUE) { - todo_wine ok(!lstrcmpW(wcsrchr(newpath, '\\') + 1, find_data.cFileName), + ok(!lstrcmpW(wcsrchr(newpath, '\\') + 1, find_data.cFileName), "Link did not change casing on existing target file: got %s\n", wine_dbgstr_w(find_data.cFileName)); } @@ -2900,7 +2900,7 @@ static void test_file_link_information(void) ok(handle != INVALID_HANDLE_VALUE, "FindFirstFileW: failed, error %d\n", GetLastError()); if (handle != INVALID_HANDLE_VALUE) { - todo_wine ok(!lstrcmpW(wcsrchr(oldpath, '\\') + 1, find_data.cFileName), + ok(!lstrcmpW(wcsrchr(oldpath, '\\') + 1, find_data.cFileName), "Link did not change casing on same file: got %s\n", wine_dbgstr_w(find_data.cFileName)); } diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index ec0beaa..21d26be 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -3408,6 +3408,61 @@ NTSTATUS nt_to_unix_file_name( const OBJECT_ATTRIBUTES *attr, char **name_ret, U } +/*********************************************************************** + * nt_to_unix_file_name_with_actual_case + * + * Same as nt_to_unix_file_name, but additionally return unix file name + * without path, with the actual case from the NT file name. + */ +static NTSTATUS nt_to_unix_file_name_with_actual_case( const OBJECT_ATTRIBUTES *attr, char **name_ret, + char **actual_case_ret, UINT disposition ) +{ + const WCHAR *nt_filename = attr->ObjectName->Buffer; + char *actual_case; + NTSTATUS status; + int len; + + /* strip off trailing backslashes; we also accept '/' for unix namespaces */ + for (len = attr->ObjectName->Length / sizeof(WCHAR); len != 0; len--) + if (nt_filename[len - 1] != '\\' && nt_filename[len - 1] != '/') + break; + + /* get the last component */ + for (nt_filename += len; nt_filename != attr->ObjectName->Buffer; nt_filename--) + if (nt_filename[-1] == '\\' || nt_filename[-1] == '/') + break; + len = attr->ObjectName->Buffer + len - nt_filename; + + if (!(actual_case = malloc( len * 3 + 1 ))) return STATUS_NO_MEMORY; + + status = nt_to_unix_file_name( attr, name_ret, disposition ); + if (status != STATUS_SUCCESS && status != STATUS_NO_SUCH_FILE) + { + free( actual_case ); + return status; + } + + /* special case for '/' root itself, as it has no named components */ + if ((*name_ret)[0] == '/' && !(*name_ret)[1]) + actual_case[0] = 0; + else + { + len = ntdll_wcstoumbs( nt_filename, len, actual_case, len * 3, TRUE ); + if (len > 0) + actual_case[len] = 0; + else + { + char *p = strrchr( *name_ret, '/' ); + p = p ? p + 1 : *name_ret; + strcpy( actual_case, p ); + } + } + + *actual_case_ret = actual_case; + return status; +} + + /****************************************************************************** * wine_nt_to_unix_file_name * @@ -4530,8 +4585,8 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, { FILE_RENAME_INFORMATION *info = ptr; UNICODE_STRING name_str, redir; + char *unix_name, *file_case; OBJECT_ATTRIBUTES attr; - char *unix_name; name_str.Buffer = info->FileName; name_str.Length = info->FileNameLength; @@ -4539,7 +4594,7 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, InitializeObjectAttributes( &attr, &name_str, OBJ_CASE_INSENSITIVE, info->RootDirectory, NULL ); get_redirect( &attr, &redir ); - io->u.Status = nt_to_unix_file_name( &attr, &unix_name, FILE_OPEN_IF ); + io->u.Status = nt_to_unix_file_name_with_actual_case( &attr, &unix_name, &file_case, FILE_OPEN_IF ); if (io->u.Status == STATUS_SUCCESS || io->u.Status == STATUS_NO_SUCH_FILE) { SERVER_START_REQ( set_fd_name_info ) @@ -4547,15 +4602,18 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, req->handle = wine_server_obj_handle( handle ); req->rootdir = wine_server_obj_handle( attr.RootDirectory ); req->namelen = attr.ObjectName->Length; + req->caselen = strlen( file_case ); req->link = FALSE; req->replace = info->ReplaceIfExists; wine_server_add_data( req, attr.ObjectName->Buffer, attr.ObjectName->Length ); + wine_server_add_data( req, file_case, req->caselen ); wine_server_add_data( req, unix_name, strlen(unix_name) ); io->u.Status = wine_server_call( req ); } SERVER_END_REQ; free( unix_name ); + free( file_case ); } free( redir.Buffer ); } @@ -4567,8 +4625,8 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, { FILE_LINK_INFORMATION *info = ptr; UNICODE_STRING name_str, redir; + char *unix_name, *file_case; OBJECT_ATTRIBUTES attr; - char *unix_name; name_str.Buffer = info->FileName; name_str.Length = info->FileNameLength; @@ -4576,7 +4634,7 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, InitializeObjectAttributes( &attr, &name_str, OBJ_CASE_INSENSITIVE, info->RootDirectory, NULL ); get_redirect( &attr, &redir ); - io->u.Status = nt_to_unix_file_name( &attr, &unix_name, FILE_OPEN_IF ); + io->u.Status = nt_to_unix_file_name_with_actual_case( &attr, &unix_name, &file_case, FILE_OPEN_IF ); if (io->u.Status == STATUS_SUCCESS || io->u.Status == STATUS_NO_SUCH_FILE) { SERVER_START_REQ( set_fd_name_info ) @@ -4584,15 +4642,18 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, req->handle = wine_server_obj_handle( handle ); req->rootdir = wine_server_obj_handle( attr.RootDirectory ); req->namelen = attr.ObjectName->Length; + req->caselen = strlen( file_case ); req->link = TRUE; req->replace = info->ReplaceIfExists; wine_server_add_data( req, attr.ObjectName->Buffer, attr.ObjectName->Length ); + wine_server_add_data( req, file_case, req->caselen ); wine_server_add_data( req, unix_name, strlen(unix_name) ); io->u.Status = wine_server_call( req ); } SERVER_END_REQ; free( unix_name ); + free( file_case ); } free( redir.Buffer ); } diff --git a/server/fd.c b/server/fd.c index 481e9a8..3e51528 100644 --- a/server/fd.c +++ b/server/fd.c @@ -2488,11 +2488,14 @@ static void set_fd_disposition( struct fd *fd, int unlink ) /* set new name for the fd */ static void set_fd_name( struct fd *fd, struct fd *root, const char *nameptr, data_size_t len, - struct unicode_str nt_name, int create_link, int replace ) + const char *file_case, data_size_t caselen, struct unicode_str nt_name, + int create_link, int replace ) { + size_t pathlen, filenamelen; struct inode *inode; struct stat st, st2; - char *name; + int different_case; + char *name, *tmp; if (!fd->inode || !fd->unix_name) { @@ -2526,6 +2529,23 @@ static void set_fd_name( struct fd *fd, struct fd *root, const char *nameptr, da name = combined_name; } + tmp = strrchr( name, '/' ); + tmp = tmp ? tmp + 1 : name; + pathlen = tmp - name; + filenamelen = strlen( tmp ); + different_case = (filenamelen != caselen || memcmp( tmp, file_case, caselen )); + + if (filenamelen < caselen) + { + tmp = realloc( name, pathlen + caselen + 1 ); + if (!tmp) + { + set_error( STATUS_NO_MEMORY ); + goto failed; + } + name = tmp; + } + /* when creating a hard link, source cannot be a dir */ if (create_link && !fstat( fd->unix_fd, &st ) && S_ISDIR( st.st_mode )) { @@ -2538,47 +2558,58 @@ static void set_fd_name( struct fd *fd, struct fd *root, const char *nameptr, da if (!fstat( fd->unix_fd, &st2 ) && st.st_ino == st2.st_ino && st.st_dev == st2.st_dev) { if (create_link && !replace) set_error( STATUS_OBJECT_NAME_COLLISION ); - free( name ); - return; - } + if (!different_case) + { + free( name ); + return; + } - if (!replace) - { - set_error( STATUS_OBJECT_NAME_COLLISION ); - goto failed; + /* creating a link with a different case on itself renames the file */ + create_link = 0; } - - /* can't replace directories or special files */ - if (!S_ISREG( st.st_mode )) + else { - set_error( STATUS_ACCESS_DENIED ); - goto failed; - } + if (!replace) + { + set_error( STATUS_OBJECT_NAME_COLLISION ); + goto failed; + } - /* can't replace an opened file */ - if ((inode = get_inode( st.st_dev, st.st_ino, -1 ))) - { - int is_empty = list_empty( &inode->open ); - release_object( inode ); - if (!is_empty) + /* can't replace directories or special files */ + if (!S_ISREG( st.st_mode )) { set_error( STATUS_ACCESS_DENIED ); goto failed; } - } - /* link() expects that the target doesn't exist */ - /* rename() cannot replace files with directories */ - if (create_link || S_ISDIR( st2.st_mode )) - { - if (unlink( name )) + /* can't replace an opened file */ + if ((inode = get_inode( st.st_dev, st.st_ino, -1 ))) { - file_set_error(); - goto failed; + int is_empty = list_empty( &inode->open ); + release_object( inode ); + if (!is_empty) + { + set_error( STATUS_ACCESS_DENIED ); + goto failed; + } + } + + /* link() expects that the target doesn't exist */ + /* rename() cannot replace files with directories */ + if (create_link || S_ISDIR( st2.st_mode ) || different_case) + { + if (unlink( name )) + { + file_set_error(); + goto failed; + } } } } + memcpy( name + pathlen, file_case, caselen ); + name[pathlen + caselen] = 0; + if (create_link) { if (link( fd->unix_name, name )) @@ -2879,16 +2910,19 @@ DECL_HANDLER(set_fd_disp_info) /* set fd name information */ DECL_HANDLER(set_fd_name_info) { + const char *fullname, *file_case; struct fd *fd, *root_fd = NULL; struct unicode_str nt_name; - if (req->namelen > get_req_data_size()) + if (req->namelen > get_req_data_size() || get_req_data_size() - req->namelen < req->caselen) { set_error( STATUS_INVALID_PARAMETER ); return; } nt_name.str = get_req_data(); nt_name.len = (req->namelen / sizeof(WCHAR)) * sizeof(WCHAR); + file_case = (const char *)get_req_data() + req->namelen; + fullname = file_case + req->caselen; if (req->rootdir) { @@ -2902,8 +2936,8 @@ DECL_HANDLER(set_fd_name_info) if ((fd = get_handle_fd_obj( current->process, req->handle, 0 ))) { - set_fd_name( fd, root_fd, (const char *)get_req_data() + req->namelen, - get_req_data_size() - req->namelen, nt_name, req->link, req->replace ); + set_fd_name( fd, root_fd, fullname, (const char *)get_req_data() + get_req_data_size() - fullname, + file_case, req->caselen, nt_name, req->link, req->replace ); release_object( fd ); } if (root_fd) release_object( root_fd ); diff --git a/server/protocol.def b/server/protocol.def index d061fca..77bfdaf 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -3519,9 +3519,11 @@ struct handle_info obj_handle_t handle; /* handle to a file or directory */ obj_handle_t rootdir; /* root directory */ data_size_t namelen; /* length of NT name in bytes */ + data_size_t caselen; /* length of the actual case filename */ int link; /* link instead of renaming */ int replace; /* replace an existing file? */ VARARG(name,unicode_str,namelen); /* NT name */ + VARARG(actual_case,string,caselen); /* new file name's actual case (without path) */ VARARG(filename,string); /* new file name */ @END -- 2.30.0

April 29, 2021