[PATCH v2 0/2] MR5139: qwave: Add QOSCloseHandle() stub.

[Aida Jonikienė]

Discord calls this function when hanging up a call.

--
  v2: qwave: Add tests for QOSCloseHandle().
      qwave: Add QOSCloseHandle() stub.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5139

[PATCH v2 1/2] qwave: Add QOSCloseHandle() stub.

[Aida Jonikienė]

From: Aida Jonikienė <aidas957(a)gmail.com>

Discord calls this function when hanging up a call.
---
 dlls/qwave/main.c     | 12 ++++++++++++
 dlls/qwave/qwave.spec |  2 +-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/dlls/qwave/main.c b/dlls/qwave/main.c
index a8755d993d1..82d1926d649 100644
--- a/dlls/qwave/main.c
+++ b/dlls/qwave/main.c
@@ -43,3 +43,15 @@ BOOL WINAPI QOSAddSocketToFlow(HANDLE handle, SOCKET socket, PSOCKADDR addr,
     SetLastError(ERROR_NOT_SUPPORTED);
     return FALSE;
 }
+
+BOOL WINAPI QOSCloseHandle(HANDLE handle)
+{
+    DWORD unused;
+
+    FIXME("%p stub!\n", handle);
+    if (!GetHandleInformation(handle, &unused))
+        SetLastError(ERROR_INVALID_HANDLE);
+    else
+        SetLastError(ERROR_NOT_SUPPORTED);
+    return FALSE;
+}
diff --git a/dlls/qwave/qwave.spec b/dlls/qwave/qwave.spec
index 83dfc6787cc..ff2994d11fb 100644
--- a/dlls/qwave/qwave.spec
+++ b/dlls/qwave/qwave.spec
@@ -2,7 +2,7 @@
 @ stub QDLHStartDiagnosingPath
 @ stdcall QOSAddSocketToFlow(ptr long ptr long long ptr)
 @ stub QOSCancel
-@ stub QOSCloseHandle
+@ stdcall QOSCloseHandle(ptr)
 @ stdcall QOSCreateHandle(ptr ptr)
 @ stub QOSEnumerateFlows
 @ stub QOSNotifyFlow
-- 
GitLab


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

[PATCH v2 2/2] qwave: Add tests for QOSCloseHandle().

[Aida Jonikienė]

From: Aida Jonikienė <aidas957(a)gmail.com>

---
 dlls/qwave/tests/qos.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/dlls/qwave/tests/qos.c b/dlls/qwave/tests/qos.c
index 7e8fddf64c4..0071e2827d5 100644
--- a/dlls/qwave/tests/qos.c
+++ b/dlls/qwave/tests/qos.c
@@ -64,7 +64,35 @@ static void test_QOSCreateHandle(void)
     ok(GetLastError() == ERROR_INVALID_PARAMETER, "Expected ERROR_INVALID_PARAMETER, got %ld\n", GetLastError());
 }
 
+static void test_QOSCloseHandle(void)
+{
+    QOS_VERSION ver;
+    HANDLE h;
+    BOOL ret;
+
+    SetLastError(0xdeadbeef);
+    ret = QOSCloseHandle(NULL);
+    ok(ret == FALSE, "Expected FALSE, got %d\n", ret);
+    ok(GetLastError() == ERROR_INVALID_HANDLE, "Expected ERROR_INVALID_HANDLE, got %ld\n", GetLastError());
+
+    ver.MajorVersion = 1;
+    ver.MinorVersion = 0;
+    ret = QOSCreateHandle(&ver, &h);
+    todo_wine ok(ret == TRUE, "Expected TRUE, got %d\n", ret);
+
+    SetLastError(0xdeadbeef);
+    ret = QOSCloseHandle(h);
+    todo_wine ok(ret == TRUE, "Expected TRUE, got %d\n", ret);
+    todo_wine ok(GetLastError() == 0xdeadbeef, "Expected 0xdeadbeef, got %ld\n", GetLastError());
+
+    SetLastError(0xdeadbeef);
+    ret = QOSCloseHandle((HANDLE)0xdeadbeef);
+    ok(ret == FALSE, "Expected FALSE, got %d\n", ret);
+    ok(GetLastError() == ERROR_INVALID_HANDLE, "Expected ERROR_INVALID_HANDLE, got %ld\n", GetLastError());
+}
+
 START_TEST(qos)
 {
     test_QOSCreateHandle();
+    test_QOSCloseHandle();
 }
-- 
GitLab

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

[Nikolay Sivov (＠nsivov)]

If it's all the same for the application, just return FALSE, no need for all this validation that won't work anyway.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5139#note_62476

[Aida Jonikienė]

On Sat Feb 24 07:24:42 2024 +0000, Nikolay Sivov wrote:
> If it's all the same for the application, just return FALSE, no need for
> all this validation that won't work anyway.
> That means I have to add a lot more `todo_wine`s for no reason

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5139#note_62503

[Zebediah Figura (＠zfigura)]

> That means I have to add a lot more `todo_wine`s for no reason

It's not no reason.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5139#note_62522

[Aida Jonikienė]

On Sat Feb 24 18:11:00 2024 +0000, Zebediah Figura wrote:
> > That means I have to add a lot more `todo_wine`s for no reason
> > It's not no reason.
> > I can simplify the function to just a single SetLastError (or even none) if it means I can get this MR approved (and eventually merged)

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5139#note_62526

[Zebediah Figura (＠zfigura)]

> I can simplify the function to just a single SetLastError (or even none) if it means I can get this MR approved (and eventually merged)

Well, that is what was requested. More generally, though, it's not about getting one's patches merged, it's about committing correct code, and improving the project.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5139#note_62527