[PATCH 0/1] MR4060: winegstreamer: Remove redundant null check before wcsdup in object_context_create.

[Alex Henrie (＠alexhenrie)]

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4060

[PATCH 1/1] winegstreamer: Remove redundant null check before wcsdup in object_context_create.

[Alex Henrie]

From: Alex Henrie <alexhenrie24(a)gmail.com>

---
 dlls/winegstreamer/media_source.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c
index 8b9d42ea3f0..a84dc0a206f 100644
--- a/dlls/winegstreamer/media_source.c
+++ b/dlls/winegstreamer/media_source.c
@@ -97,7 +97,7 @@ static const IUnknownVtbl object_context_vtbl =
 static HRESULT object_context_create(DWORD flags, IMFByteStream *stream, const WCHAR *url,
         QWORD file_size, IMFAsyncResult *result, IUnknown **out)
 {
-    WCHAR *tmp_url = url ? wcsdup(url) : NULL;
+    WCHAR *tmp_url = wcsdup(url);
     struct object_context *context;
 
     if (!(context = calloc(1, sizeof(*context))))
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/4060

[PATCH 1/1] winegstreamer: Remove redundant null check before wcsdup in object_context_create.

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=138566

Your paranoid android.


=== debian11b (64 bit WoW report) ===

wmvcore:
wmvcore.c:1711: Test failed: Stream 0: Format 1: Got hr 0xc00d0041.
wmvcore.c:1728: Test failed: Stream 0: Format 1: Media types didn't match.

[PATCH 0/1] MR4060: winegstreamer: Remove redundant null check before wcsdup in object_context_create. - approved

[Rémi Bernon]

This merge request was approved by Rémi Bernon.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4060

[Zebediah Figura (＠zfigura)]

This isn't a documented feature of strdup()/wcsdup(), which makes me a little doubtful of whether it's really a good idea, both in terms of functionality (what if it's supposed to trigger an invalid parameter error?) and clarity.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4060#note_48218

[Alex Henrie (＠alexhenrie)]

On Tue Oct 10 18:21:12 2023 +0000, Zebediah Figura wrote:
> This isn't a documented feature of strdup()/wcsdup(), which makes me a
> little doubtful of whether it's really a good idea, both in terms of
> functionality (what if it's supposed to trigger an invalid parameter
> error?) and clarity.
> We have a [test for strdup](https://gitlab.winehq.org/wine/wine/-/blob/eb06f991725d21e1ce4e3353a377e8708...) that shows that strdup(NULL) == NULL on Windows, but it appears we don't have a similar test for wcsdup. I'd be happy to add that in a separate MR to reassure everyone that we can count on this behavior. It sounds like we should test the value of errno too.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4060#note_48221

[Zebediah Figura (＠zfigura)]

On Tue Oct 10 18:21:12 2023 +0000, Alex Henrie wrote:
> We have a [test for
> strdup](https://gitlab.winehq.org/wine/wine/-/blob/eb06f991725d21e1ce4e3353a377e8708...)
> that shows that strdup(NULL) == NULL on Windows, but it appears we don't
> have a similar test for wcsdup. I'd be happy to add that in a separate
> MR to reassure everyone that we can count on this behavior. It sounds
> like we should test the value of errno too.
> Despite the test I'm still not convinced this is a good idea. If the code doesn't check for a NULL pointer and just passes directly to wcsdup(), it makes me think that it will never be NULL, which isn't the case.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4060#note_50681