From d640c733b68bfac3c4c09f8718aa88b065ecf7c4 Mon Sep 17 00:00:00 2001 From: junkmd Date: Sat, 10 Jan 2026 13:39:27 +0900 Subject: [PATCH 1/9] test: Unskip `test_c_char` in `test_outparam` as it now passes. The `test_c_char` in `test_outparam.py` was previously skipped due to "failing for mysterious reasons". It now passes without modifications. This suggests that underlying issues might have been resolved by this package's community or within CPython over the past few years. --- comtypes/test/test_outparam.py | 1 - 1 file changed, 1 deletion(-) diff --git a/comtypes/test/test_outparam.py b/comtypes/test/test_outparam.py index 26a8949ff..defc58125 100644 --- a/comtypes/test/test_outparam.py +++ b/comtypes/test/test_outparam.py @@ -76,7 +76,6 @@ def comstring(text, typ=c_wchar_p): class Test(unittest.TestCase): - @unittest.skip("This fails for reasons I don't understand yet") # TODO untested changes; this was modified because it had global effects on other tests @patch.object(c_wchar_p, "__ctypes_from_outparam__", from_outparm) def test_c_char(self): From 5578f74552e21997dc7b982ea084e9096f3f23b4 Mon Sep 17 00:00:00 2001 From: junkmd Date: Sat, 10 Jan 2026 13:39:27 +0900 Subject: [PATCH 2/9] test: Remove redundant `str` conversion from `comstring` in `test_outparam`. Removed the `text_type = str` assignment and its usage in `comstring` function. This change eliminates a redundant `str` conversion that remained from Python 2/3 version bridging. --- comtypes/test/test_outparam.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/comtypes/test/test_outparam.py b/comtypes/test/test_outparam.py index defc58125..46b2b99e0 100644 --- a/comtypes/test/test_outparam.py +++ b/comtypes/test/test_outparam.py @@ -22,8 +22,6 @@ from comtypes import COMMETHOD, GUID, IUnknown from comtypes.GUID import _CoTaskMemFree -text_type = str - class IMalloc(IUnknown): _iid_ = GUID("{00000002-0000-0000-C000-000000000046}") @@ -66,7 +64,6 @@ def from_outparm(self): def comstring(text, typ=c_wchar_p): - text = text_type(text) size = (len(text) + 1) * sizeof(c_wchar) mem = _CoTaskMemAlloc(size) print("malloc'd 0x%x, %d bytes" % (mem, size)) From fc3a38e89f35844390462a2a8bf24f1e66492598 Mon Sep 17 00:00:00 2001 From: junkmd Date: Sat, 10 Jan 2026 13:39:27 +0900 Subject: [PATCH 3/9] refactor: Use `logging` module for debug output in `test_outparam`. Replaced `print` statements with `logger.debug` in `test_outparam.py` to allow for more granular control and suppression of debug messages. --- comtypes/test/test_outparam.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/comtypes/test/test_outparam.py b/comtypes/test/test_outparam.py index 46b2b99e0..608b81bbb 100644 --- a/comtypes/test/test_outparam.py +++ b/comtypes/test/test_outparam.py @@ -1,3 +1,4 @@ +import logging import unittest from ctypes import ( HRESULT, @@ -22,6 +23,8 @@ from comtypes import COMMETHOD, GUID, IUnknown from comtypes.GUID import _CoTaskMemFree +logger = logging.getLogger(__name__) + class IMalloc(IUnknown): _iid_ = GUID("{00000002-0000-0000-C000-000000000046}") @@ -66,7 +69,7 @@ def from_outparm(self): def comstring(text, typ=c_wchar_p): size = (len(text) + 1) * sizeof(c_wchar) mem = _CoTaskMemAlloc(size) - print("malloc'd 0x%x, %d bytes" % (mem, size)) + logger.debug("malloc'd 0x%x, %d bytes" % (mem, size)) ptr = cast(mem, typ) memmove(mem, text, size) return ptr @@ -87,7 +90,9 @@ def test_c_char(self): z = comstring("spam, spam, and spam") # (x.__ctypes_from_outparam__(), x.__ctypes_from_outparam__()) - print((x.__ctypes_from_outparam__(), None)) # x.__ctypes_from_outparam__()) + logger.debug( + (x.__ctypes_from_outparam__(), None) + ) # x.__ctypes_from_outparam__()) # print comstring("Hello, World", c_wchar_p).__ctypes_from_outparam__() # print comstring("Hello, World", c_wchar_p).__ctypes_from_outparam__() From 28ba9075e50549f37efebdd0e5e8d3529388028c Mon Sep 17 00:00:00 2001 From: junkmd Date: Sat, 10 Jan 2026 13:39:27 +0900 Subject: [PATCH 4/9] test: Replace `ValueError` with `assert` for memory allocation check in `test_outparam`. Changed the memory allocation check in `test_outparam.py` from raising a `ValueError` to using an `assert` statement. In test code, avoiding `if` branches is preferred. --- comtypes/test/test_outparam.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/comtypes/test/test_outparam.py b/comtypes/test/test_outparam.py index 608b81bbb..3b47e19b3 100644 --- a/comtypes/test/test_outparam.py +++ b/comtypes/test/test_outparam.py @@ -60,8 +60,12 @@ def from_outparm(self): if not self: return None result = wstring_at(self) - if not malloc.DidAlloc(self): - raise ValueError("memory was NOT allocated by CoTaskMemAlloc") + # `DidAlloc` method returns; + # * 1 (allocated) + # * 0 (not allocated) + # * -1 (cannot determine or NULL) + # https://learn.microsoft.com/en-us/windows/win32/api/objidl/nf-objidl-imalloc-didalloc + assert malloc.DidAlloc(self), "memory was NOT allocated by CoTaskMemAlloc" _CoTaskMemFree(self) return result From c77aaab2bb4b64ccafa57b7ee48cc60aed81d103 Mon Sep 17 00:00:00 2001 From: junkmd Date: Sat, 10 Jan 2026 13:39:27 +0900 Subject: [PATCH 5/9] test: Verify `c_wchar_p` constructor does not allocate memory via COM allocator. Added an assertion to `test_outparam.py` to verify that `c_wchar_p`'s constructor does not allocate memory using `CoTaskMemAlloc`. This ensures the memory is not identified as COM-allocated. This check is vital to prevent the patched `__ctypes_from_outparam__` from attempting to free unmanaged memory, which it expects to be COM-allocated. --- comtypes/test/test_outparam.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/comtypes/test/test_outparam.py b/comtypes/test/test_outparam.py index 3b47e19b3..0ee712930 100644 --- a/comtypes/test/test_outparam.py +++ b/comtypes/test/test_outparam.py @@ -83,9 +83,11 @@ class Test(unittest.TestCase): # TODO untested changes; this was modified because it had global effects on other tests @patch.object(c_wchar_p, "__ctypes_from_outparam__", from_outparm) def test_c_char(self): - # ptr = c_wchar_p("abc") - # self.failUnlessEqual(ptr.__ctypes_from_outparam__(), - # "abc") + ptr = c_wchar_p("abc") + # The normal constructor does not allocate memory using `CoTaskMemAlloc`. + # Therefore, calling the patched `ptr.__ctypes_from_outparam__()` would + # attempt to free invalid memory, potentially leading to a crash. + self.assertEqual(malloc.DidAlloc(ptr), 0) # p = BSTR("foo bar spam") From d516af9a88cc3d103888cf5144857c0374ac0ba8 Mon Sep 17 00:00:00 2001 From: junkmd Date: Sat, 10 Jan 2026 13:39:27 +0900 Subject: [PATCH 6/9] test: Remove irrelevant `BSTR` comment from `test_outparam`. Removed a commented-out `BSTR` instantiation from `test_outparam.py`. While its presence might have been intended to prevent some type definition regression, it felt largely unrelated to the core purpose of this specific test and was therefore removed for clarity. --- comtypes/test/test_outparam.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/comtypes/test/test_outparam.py b/comtypes/test/test_outparam.py index 0ee712930..938f5b9c4 100644 --- a/comtypes/test/test_outparam.py +++ b/comtypes/test/test_outparam.py @@ -89,8 +89,6 @@ def test_c_char(self): # attempt to free invalid memory, potentially leading to a crash. self.assertEqual(malloc.DidAlloc(ptr), 0) - # p = BSTR("foo bar spam") - x = comstring("Hello, World") y = comstring("foo bar") z = comstring("spam, spam, and spam") From 2f780e5ba6dc77b5907ec29d0ccc4b1ee75f6216 Mon Sep 17 00:00:00 2001 From: junkmd Date: Sat, 10 Jan 2026 13:39:27 +0900 Subject: [PATCH 7/9] test: Refactor `test_c_char` to use `subTest` and verify memory deallocation. Refactored `test_c_char` in `test_outparam.py` to utilize `self.subTest` for improved test granularity. The test now explicitly verifies memory allocation status using `malloc.DidAlloc` before and after calling `__ctypes_from_outparam__`, ensuring correct memory deallocation behavior. Old commented-out debug statements were also removed. --- comtypes/test/test_outparam.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/comtypes/test/test_outparam.py b/comtypes/test/test_outparam.py index 938f5b9c4..761ca1b4f 100644 --- a/comtypes/test/test_outparam.py +++ b/comtypes/test/test_outparam.py @@ -93,15 +93,20 @@ def test_c_char(self): y = comstring("foo bar") z = comstring("spam, spam, and spam") - # (x.__ctypes_from_outparam__(), x.__ctypes_from_outparam__()) - logger.debug( - (x.__ctypes_from_outparam__(), None) - ) # x.__ctypes_from_outparam__()) - - # print comstring("Hello, World", c_wchar_p).__ctypes_from_outparam__() - # print comstring("Hello, World", c_wchar_p).__ctypes_from_outparam__() - # print comstring("Hello, World", c_wchar_p).__ctypes_from_outparam__() - # print comstring("Hello, World", c_wchar_p).__ctypes_from_outparam__() + # The `__ctypes_from_outparam__` method is called to convert an output + # parameter into a Python object. In this test, the custom + # `from_outparm` function not only converts the `c_wchar_p` to a Python + # string but also frees the associated memory. Therefore, it can only + # be called once for each allocated memory block. + for wchar_ptr, expected in [ + (x, "Hello, World"), + (y, "foo bar"), + (z, "spam, spam, and spam"), + ]: + with self.subTest(wchar_ptr=wchar_ptr, expected=expected): + self.assertEqual(malloc.DidAlloc(wchar_ptr), 1) + self.assertEqual(wchar_ptr.__ctypes_from_outparam__(), expected) + self.assertEqual(malloc.DidAlloc(wchar_ptr), 0) if __name__ == "__main__": From 0df0bc51d0909e49014e21aa891ac7cd59ddf036 Mon Sep 17 00:00:00 2001 From: junkmd Date: Sat, 10 Jan 2026 13:39:27 +0900 Subject: [PATCH 8/9] docs: Remove outdated TODO comment from `test_outparam`. Removed the `TODO` comment in `comtypes/test/test_outparam.py` regarding "untested changes" and "global effects on other tests". This comment is no longer relevant due to recent improvements and the unskipping of `test_c_char`. --- comtypes/test/test_outparam.py | 1 - 1 file changed, 1 deletion(-) diff --git a/comtypes/test/test_outparam.py b/comtypes/test/test_outparam.py index 761ca1b4f..a7929bf1a 100644 --- a/comtypes/test/test_outparam.py +++ b/comtypes/test/test_outparam.py @@ -80,7 +80,6 @@ def comstring(text, typ=c_wchar_p): class Test(unittest.TestCase): - # TODO untested changes; this was modified because it had global effects on other tests @patch.object(c_wchar_p, "__ctypes_from_outparam__", from_outparm) def test_c_char(self): ptr = c_wchar_p("abc") From e7415557b1acbc34e083ea3842bc112f500cebce Mon Sep 17 00:00:00 2001 From: junkmd Date: Sat, 10 Jan 2026 13:39:27 +0900 Subject: [PATCH 9/9] fix: Rename `from_outparm` to `from_outparam` in `test_outparam.py`. Corrected a typo in the function name `from_outparm` to `from_outparam` within `test_outparam.py`. --- comtypes/test/test_outparam.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/comtypes/test/test_outparam.py b/comtypes/test/test_outparam.py index a7929bf1a..bd4c05a87 100644 --- a/comtypes/test/test_outparam.py +++ b/comtypes/test/test_outparam.py @@ -56,7 +56,7 @@ class IMalloc(IUnknown): assert bool(malloc) -def from_outparm(self): +def from_outparam(self): if not self: return None result = wstring_at(self) @@ -80,7 +80,7 @@ def comstring(text, typ=c_wchar_p): class Test(unittest.TestCase): - @patch.object(c_wchar_p, "__ctypes_from_outparam__", from_outparm) + @patch.object(c_wchar_p, "__ctypes_from_outparam__", from_outparam) def test_c_char(self): ptr = c_wchar_p("abc") # The normal constructor does not allocate memory using `CoTaskMemAlloc`. @@ -94,9 +94,9 @@ def test_c_char(self): # The `__ctypes_from_outparam__` method is called to convert an output # parameter into a Python object. In this test, the custom - # `from_outparm` function not only converts the `c_wchar_p` to a Python - # string but also frees the associated memory. Therefore, it can only - # be called once for each allocated memory block. + # `from_outparam` function not only converts the `c_wchar_p` to a + # Python string but also frees the associated memory. Therefore, it can + # only be called once for each allocated memory block. for wchar_ptr, expected in [ (x, "Hello, World"), (y, "foo bar"),