diff --git a/comtypes/test/test_cpp_byte_compatibility.py b/comtypes/test/test_cpp_byte_compatibility.py new file mode 100644 index 000000000..bde368c97 --- /dev/null +++ b/comtypes/test/test_cpp_byte_compatibility.py @@ -0,0 +1,149 @@ +"""Byte and string compatibility through generated interfaces and native COM.""" + +import unittest +from ctypes import ( + byref, + c_byte, + c_char_p, + c_void_p, + cast, + create_string_buffer, + pointer, +) + +from comtypes import CLSCTX_LOCAL_SERVER +from comtypes.client import CreateObject, GetModule +from comtypes.malloc import _CoTaskMemFree + +try: + GetModule(("{07D2AEE5-1DF8-4D2C-953A-554ADFD25F99}", 1, 0, 0)) + from comtypes.gen.ComtypesCppTestSrvLib import ( + CoByteCompatibilityTest, + IByteCompatibilityTest, + ) + + IMPORT_SERVER_FAILED = False +except (ImportError, OSError): + IMPORT_SERVER_FAILED = True + + +@unittest.skipIf(IMPORT_SERVER_FAILED, "Requires the C++ out-of-process COM server.") +class _NativeByteTest(unittest.TestCase): + def setUp(self): + self.server = CreateObject( + CoByteCompatibilityTest, + clsctx=CLSCTX_LOCAL_SERVER, + interface=IByteCompatibilityTest, + ) + + +class ByteCompatibilityTest(_NativeByteTest): + def test_negative_int8(self): + for value in (-128, -7, -1): + with self.subTest(value=value): + self.assertEqual(self.server.EchoInt8(value), value) + + def test_nonnegative_int8(self): + for value in (0, 1, 127): + with self.subTest(value=value): + self.assertEqual(self.server.EchoInt8(value), value) + + def test_uint8(self): + for value in (0, 1, 42, 128, 255): + with self.subTest(value=value): + self.assertEqual(self.server.EchoUint8(value), value) + + def test_lpstr(self): + for value in (b"", b"hello", b"\x80\xff"): + with self.subTest(value=value): + result = c_char_p() + try: + # Use the generated raw method to retain the allocated address. + # The high-level result is bytes, which loses that address. + self.server._IByteCompatibilityTest__com_EchoLpStr( + value, byref(result) + ) + self.assertEqual(result.value, value) + finally: + _CoTaskMemFree(cast(result, c_void_p)) + + +class LegacyMappingCompatibilityTest(_NativeByteTest): + """Calls accepted by the old mapping that regress with the proposed mapping.""" + + @unittest.expectedFailure + def test_char_pointer_bytes_input(self): + # Plain char* was previously inferred as STRING. Changing VT_I1 to + # signed char loses this implicit bytes input, unlike explicit LPSTR. + # Keep the compatibility regression visible for the review decision. + self.assertEqual(self.server.ReadCharPointer(b"h"), 104) + + @unittest.expectedFailure + def test_character_bytes_output_buffer(self): + # Old STRING output accepts a char buffer. Reserve the terminator + # locally because a plain char* marshals only one character. + result = create_string_buffer(2) + self.server._IByteCompatibilityTest__com_GetCharacterBytes(104, result) + self.assertEqual(result.value, b"h") + + +class ProposedMappingCompatibilityTest(_NativeByteTest): + """Numeric arguments and results supported by the proposed signed mapping.""" + + def test_char_pointer_input(self): + for value, expected in ((0, 0), (104, 104), (-128, 128), (-1, 255)): + with self.subTest(value=value): + self.assertEqual( + self.server.ReadCharPointer(pointer(c_byte(value))), expected + ) + + def test_char_reference_input(self): + for value, expected in ((0, 0), (104, 104), (-128, 128), (-1, 255)): + with self.subTest(value=value): + self.assertEqual( + self.server.ReadCharPointer(byref(c_byte(value))), expected + ) + + def test_char_array_input(self): + for value, expected in ((0, 0), (104, 104), (-128, 128), (-1, 255)): + with self.subTest(value=value): + self.assertEqual( + self.server.ReadCharPointer((c_byte * 1)(value)), expected + ) + + def test_character_pointer_output(self): + for value, expected in ((0, 0), (1, 1), (104, 104), (128, -128), (255, -1)): + with self.subTest(value=value): + # Plain char* output uses the signed numeric mapping as well. + self.assertEqual(self.server.GetCharacterBytes(value), expected) + + def test_character_pointer_output_buffer(self): + for value, expected in ((0, 0), (104, 104), (128, -128), (255, -1)): + with self.subTest(value=value): + result = pointer(c_byte()) + self.server._IByteCompatibilityTest__com_GetCharacterBytes( + value, result + ) + self.assertEqual(result.contents.value, expected) + + def test_character_reference_output_buffer(self): + for value, expected in ((0, 0), (104, 104), (128, -128), (255, -1)): + with self.subTest(value=value): + result = c_byte() + self.server._IByteCompatibilityTest__com_GetCharacterBytes( + value, byref(result) + ) + self.assertEqual(result.value, expected) + + def test_character_array_output_buffer(self): + for value, expected in ((0, 0), (104, 104), (128, -128), (255, -1)): + with self.subTest(value=value): + result = (c_byte * 1)() + self.server._IByteCompatibilityTest__com_GetCharacterBytes( + value, result + ) + self.assertEqual(result[0], expected) + + +if __name__ == "__main__": + unittest.main() diff --git a/comtypes/test/test_cpp_safearray.py b/comtypes/test/test_cpp_safearray.py index 1c8e77d2f..f39d5b575 100644 --- a/comtypes/test/test_cpp_safearray.py +++ b/comtypes/test/test_cpp_safearray.py @@ -35,6 +35,7 @@ def setUp(self): ) # Fixed expected values come from the C++ fixture, not Python input. self.cases_1d = [ + (self.server.GetInt8Array, "int8", (-7, 0, 42, -7, 0, 42)), (self.server.GetUint8Array, "uint8", (0, 42, 255, 0, 42, 255)), (self.server.GetInt16Array, "int16", (-7, 0, 42, -7, 0, 42)), (self.server.GetUint16Array, "uint16", (0, 42, 65535, 0, 42, 65535)), @@ -63,6 +64,7 @@ def setUp(self): ] # SAFEARRAY storage is column-major; the fixture has two rows. self.cases_2d = [ + (self.server.GetInt8Array, "int8", ((-7, 42, 0), (0, -7, 42))), (self.server.GetUint8Array, "uint8", ((0, 255, 42), (42, 0, 255))), (self.server.GetInt16Array, "int16", ((-7, 42, 0), (0, -7, 42))), (self.server.GetUint16Array, "uint16", ((0, 65535, 42), (42, 0, 65535))), @@ -140,18 +142,6 @@ def test_ndarray_values_2d(self): numpy.testing.assert_array_equal(result, expected) self.assertEqual(method(2, 6), expected) - def test_int8_codegen_1d(self): - # VT_I1 is currently parsed as c_char (see #935), whose pointer slice - # returns bytes. Check the real COM/code-generator path and raw bytes; - # signed int8/NumPy dtype assertions await the separate c_char/c_byte fix. - result = self.server.GetInt8Array(1, 6) - self.assertEqual(result, (249, 0, 42, 249, 0, 42)) - - def test_int8_codegen_2d(self): - # As above, 249 is the raw byte for -7, not a signed dtype assertion. - result = self.server.GetInt8Array(2, 6) - self.assertEqual(result, ((249, 42, 0), (0, 249, 42))) - if __name__ == "__main__": unittest.main() diff --git a/comtypes/tools/tlbparser.py b/comtypes/tools/tlbparser.py index cad940d7f..3bc194e76 100644 --- a/comtypes/tools/tlbparser.py +++ b/comtypes/tools/tlbparser.py @@ -21,6 +21,7 @@ def PTR(typ): # basic C data types, with size and alignment in bits char_type = typedesc.FundamentalType("char", 8, 8) +schar_type = typedesc.FundamentalType("signed char", 8, 8) uchar_type = typedesc.FundamentalType("unsigned char", 8, 8) wchar_t_type = typedesc.FundamentalType("wchar_t", 16, 16) short_type = typedesc.FundamentalType("short int", 16, 16) @@ -81,7 +82,7 @@ def midlSAFEARRAY(typ): automation.VT_VARIANT: VARIANT_type, # 12 automation.VT_UNKNOWN: PTR(IUNKNOWN_type), # 13 automation.VT_DECIMAL: DECIMAL_type, # 14 - automation.VT_I1: char_type, # 16 + automation.VT_I1: schar_type, # 16 automation.VT_UI1: uchar_type, # 17 automation.VT_UI2: ushort_type, # 18 automation.VT_UI4: ulong_type, # 19 diff --git a/docs/source/byte_compatibility.rst b/docs/source/byte_compatibility.rst new file mode 100644 index 000000000..173660390 --- /dev/null +++ b/docs/source/byte_compatibility.rst @@ -0,0 +1,76 @@ +Signed 8-bit values from type libraries +======================================= + +This page describes the compatibility impact of the proposed ``VT_I1`` +type-library mapping change. The change is under review; the examples below +describe regenerated modules using that mapping. + +Generated types +--------------- + +``GetModule`` now represents ``VT_I1`` with ``ctypes.c_byte`` (a signed 8-bit +integer) instead of ``ctypes.c_char``. This affects scalar values, pointers, +and SAFEARRAY elements described by ``VT_I1`` in a type library. For example, +an IDL ``small`` scalar and a plain ``char*`` pointer use this mapping. +Explicit string types such as ``LPSTR`` still generate ``STRING`` +(``ctypes.c_char_p``); ``VT_UI1`` still generates ``ctypes.c_ubyte``. + +The change applies when modules are generated again. Existing generated +source files and interfaces defined directly in Python retain their declared +types. Mixing old generated modules with new ones can therefore produce +different Python argument and return types for the same type library. + +Arguments +--------- + +Pass a Python integer in the range -128 to 127, or a ``ctypes.c_byte`` value, +to a signed 8-bit scalar argument. The generated retval for a signed scalar +is also numeric: an ``EchoInt8(-7)`` call returns the integer ``-7``. +Single-character ``bytes`` and ``ctypes.c_char`` arguments do not match the +new scalar type. + +A plain ``char*`` argument previously generated as ``STRING`` now generates +as ``POINTER(c_byte)``. Existing calls that pass ``b"h"`` to such an argument +raise ``ctypes.ArgumentError``. Use a pointer, a reference, or an array of +``c_byte`` values instead, according to the native method's buffer contract:: + + from ctypes import byref, c_byte, pointer + + value = c_byte(ord("h")) + server.ReadCharPointer(pointer(value)) + server.ReadCharPointer(byref(value)) + server.ReadCharPointer((c_byte * 1)(ord("h"))) + +These examples refer to a method that reads exactly one character. For +methods that read more data, provide the required buffer size and lifetime. +The pointer type alone does not describe a string's length or termination. +Explicit ``LPSTR`` arguments continue to accept ``bytes`` as before. + +Return values +------------- + +A plain ``char*`` output also changes from ``STRING`` to ``POINTER(c_byte)``. +For a method returning one character through that parameter, a generated +high-level call now returns an integer. The value 104 returns as 104, and +255 returns as -1. Under the old mapping the automatic output allocation +could instead fail with ``TypeError: c_char_p 'out' parameter must be passed +as default value``. + +Calls to the generated raw COM method previously accepted a writable +``ctypes.create_string_buffer`` for that output. The new pointer type rejects +that character array. Supply ``byref(c_byte())`` or a ``c_byte`` array for a +single-character output, and read the resulting signed number. To recover +the corresponding one-byte ``bytes`` value, use ``bytes([value & 0xff])``. + +For larger returned buffers, read the result using the native method's +documented length. ``ctypes.string_at(result, length)`` makes a ``bytes`` copy +from a pointer of known length. Use NUL-terminated string conversion only +when the native contract guarantees termination. A plain character pointer +does not guarantee a terminator. Free or release native allocations as +specified by that contract after copying the data. + +Explicit ``LPSTR`` output types retain their existing string conversion. +Changing interface declarations by hand should likewise follow the actual +native contract. Type libraries describe both numeric ``small`` and plain +character data using ``VT_I1``, so this mapping change cannot by itself +preserve every existing implicit string conversion. diff --git a/docs/source/index.rst b/docs/source/index.rst index b7c54777e..ff403eb71 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -30,6 +30,7 @@ Functionalities npsupport threading type_checking + byte_compatibility Links diff --git a/source/CppTestSrv/CoByteCompatibilityTest.cpp b/source/CppTestSrv/CoByteCompatibilityTest.cpp new file mode 100644 index 000000000..3f92b4c64 --- /dev/null +++ b/source/CppTestSrv/CoByteCompatibilityTest.cpp @@ -0,0 +1,84 @@ +#include +#include +#include "CoByteCompatibilityTest.h" + +HRESULT CByteCompatibilityTest::CreateInstance(IUnknown* pUnknownOuter, CUnknown** ppNewComponent) +{ + if (pUnknownOuter != NULL) + { + return CLASS_E_NOAGGREGATION ; + } + *ppNewComponent = new CByteCompatibilityTest(pUnknownOuter) ; + return S_OK ; +} + +HRESULT __stdcall CByteCompatibilityTest::NondelegatingQueryInterface(const IID& iid, void** ppv) +{ + if (iid == IID_IByteCompatibilityTest) + { + return FinishQI(static_cast(this), ppv) ; + } + return CUnknown::NondelegatingQueryInterface(iid, ppv) ; +} + +HRESULT __stdcall CByteCompatibilityTest::EchoInt8(char value, char* result) +{ + if (result == NULL) + { + return E_POINTER ; + } + *result = value ; + return S_OK ; +} + +HRESULT __stdcall CByteCompatibilityTest::EchoUint8(byte value, byte* result) +{ + if (result == NULL) + { + return E_POINTER ; + } + *result = value ; + return S_OK ; +} + +HRESULT __stdcall CByteCompatibilityTest::EchoLpStr(LPSTR value, LPSTR* result) +{ + if (result == NULL) + { + return E_POINTER ; + } + *result = NULL ; + if (value == NULL) + { + return E_INVALIDARG ; + } + size_t size = strlen(value) + 1 ; + *result = static_cast(CoTaskMemAlloc(size)) ; + if (*result == NULL) + { + return E_OUTOFMEMORY ; + } + memcpy(*result, value, size) ; + return S_OK ; +} + +HRESULT __stdcall CByteCompatibilityTest::ReadCharPointer(unsigned char* value, byte* result) +{ + if (value == NULL || result == NULL) + { + return E_POINTER ; + } + // A raw char* marshals one character, not a NUL-terminated string. + *result = static_cast(*value) ; + return S_OK ; +} + +HRESULT __stdcall CByteCompatibilityTest::GetCharacterBytes(byte value, unsigned char* result) +{ + if (result == NULL) + { + return E_POINTER ; + } + *result = value ; + return S_OK ; +} diff --git a/source/CppTestSrv/CoByteCompatibilityTest.h b/source/CppTestSrv/CoByteCompatibilityTest.h new file mode 100644 index 000000000..412dbad96 --- /dev/null +++ b/source/CppTestSrv/CoByteCompatibilityTest.h @@ -0,0 +1,20 @@ +// A separate COM component for byte and narrow-string compatibility. +#include "CUnknown.h" +#include "Iface.h" + +class CByteCompatibilityTest : public CUnknown, public IByteCompatibilityTest +{ +public: + static HRESULT CreateInstance(IUnknown* pUnknownOuter, CUnknown** ppNewComponent) ; + +private: + DECLARE_IUNKNOWN + virtual HRESULT __stdcall NondelegatingQueryInterface(const IID& iid, void** ppv) ; + virtual HRESULT __stdcall EchoInt8(char value, char* result) ; + virtual HRESULT __stdcall EchoUint8(byte value, byte* result) ; + virtual HRESULT __stdcall EchoLpStr(LPSTR value, LPSTR* result) ; + virtual HRESULT __stdcall ReadCharPointer(unsigned char* value, byte* result) ; + virtual HRESULT __stdcall GetCharacterBytes(byte value, unsigned char* result) ; + + CByteCompatibilityTest(IUnknown* pUnknownOuter) : CUnknown(pUnknownOuter) {} ; +} ; diff --git a/source/CppTestSrv/CoNumericSafearrayTest.h b/source/CppTestSrv/CoNumericSafearrayTest.h index 821a811bf..7224e01e3 100644 --- a/source/CppTestSrv/CoNumericSafearrayTest.h +++ b/source/CppTestSrv/CoNumericSafearrayTest.h @@ -12,8 +12,6 @@ class CNumericSafearrayTest : public CUnknown, public INumericSafearrayTest virtual HRESULT __stdcall NondelegatingQueryInterface(const IID& iid, void** ppv) ; // Interface INumericSafearrayTest - // VT_I1 has COM/code-generator smoke coverage; NumPy dtype coverage awaits - // the separate c_char/c_byte issue (#935). virtual HRESULT __stdcall GetInt8Array(short dimensions, long count, SAFEARRAY** result) ; virtual HRESULT __stdcall GetUint8Array(short dimensions, long count, SAFEARRAY** result) ; virtual HRESULT __stdcall GetInt16Array(short dimensions, long count, SAFEARRAY** result) ; diff --git a/source/CppTestSrv/MAKEFILE b/source/CppTestSrv/MAKEFILE index 13d66b1c3..d42893b46 100644 --- a/source/CppTestSrv/MAKEFILE +++ b/source/CppTestSrv/MAKEFILE @@ -41,7 +41,7 @@ guids.obj : guids.c # Component/server source files # -server.obj : server.cpp cunknown.h cfactory.h iface.h CoNumericSafearrayTest.h CoVariantErrorTest.h +server.obj : server.cpp cunknown.h cfactory.h iface.h CoNumericSafearrayTest.h CoByteCompatibilityTest.h CoVariantErrorTest.h cl $(CPP_FLAGS) server.cpp CoComtypesDispRecordParamTest.obj : CoComtypesDispRecordParamTest.cpp \ @@ -56,6 +56,9 @@ CoNumericSafearrayTest.obj : CoNumericSafearrayTest.cpp \ CoNumericSafearrayTest.h iface.h CUnknown.h cl $(CPP_FLAGS) CoNumericSafearrayTest.cpp +CoByteCompatibilityTest.obj : CoByteCompatibilityTest.cpp \ + CoByteCompatibilityTest.h iface.h CUnknown.h + cl $(CPP_FLAGS) CoByteCompatibilityTest.cpp CoStringSafearrayInsideVariantTest.obj : CoStringSafearrayInsideVariantTest.cpp \ CoStringSafearrayInsideVariantTest.h iface.h registry.h CUnknown.h cl $(CPP_FLAGS) CoStringSafearrayInsideVariantTest.cpp @@ -94,6 +97,7 @@ SERVER_OBJS = Server.obj \ CoComtypesDispRecordParamTest.obj \ CoComtypesDispSafearrayParamTest.obj \ CoNumericSafearrayTest.obj \ + CoByteCompatibilityTest.obj \ CoStringSafearrayInsideVariantTest.obj \ CoVariantErrorTest.obj \ Registry.obj \ diff --git a/source/CppTestSrv/SERVER.CPP b/source/CppTestSrv/SERVER.CPP index 90b307958..433b27231 100644 --- a/source/CppTestSrv/SERVER.CPP +++ b/source/CppTestSrv/SERVER.CPP @@ -11,6 +11,7 @@ #include "CoComtypesDispRecordParamTest.h" #include "CoComtypesDispSafearrayParamTest.h" #include "CoNumericSafearrayTest.h" +#include "CoByteCompatibilityTest.h" #include "CoStringSafearrayInsideVariantTest.h" #include "CoVariantErrorTest.h" @@ -57,6 +58,12 @@ CFactoryData g_FactoryDataArray[] = L"Comtypes.NumericSafearrayTest", &LIBID_ComtypesCppTestSrvLib, NULL, 0}, + {&CLSID_CoByteCompatibilityTest, CByteCompatibilityTest::CreateInstance, + L"Comtypes component for byte and narrow-string compatibility tests", + L"Comtypes.ByteCompatibilityTest.1", + L"Comtypes.ByteCompatibilityTest", + &LIBID_ComtypesCppTestSrvLib, + NULL, 0}, {&CLSID_CoStringSafearrayInsideVariantTest, CStringSafearrayInsideVariantTest::CreateInstance, L"String SAFEARRAY inside VARIANT Tester", L"Comtypes.CoStringSafearrayInsideVariantTest.1", diff --git a/source/CppTestSrv/SERVER.IDL b/source/CppTestSrv/SERVER.IDL index 30f26ee54..c261ed3c7 100644 --- a/source/CppTestSrv/SERVER.IDL +++ b/source/CppTestSrv/SERVER.IDL @@ -88,6 +88,22 @@ interface INumericSafearrayTest : IUnknown HRESULT GetFloat64Array([in] short dimensions, [in] long count, [out, retval] SAFEARRAY(double)* result); } ; +// A separate interface for scalar input/output and string compatibility. +[ + object, + uuid(047E34C1-33A8-4DAA-A1E5-B941756A8319), + oleautomation +] +interface IByteCompatibilityTest : IUnknown +{ + HRESULT EchoInt8([in] small value, [out, retval] small* result); + HRESULT EchoUint8([in] byte value, [out, retval] byte* result); + HRESULT EchoLpStr([in] LPSTR value, [out, retval] LPSTR* result); + HRESULT ReadCharPointer([in] char* value, [out, retval] byte* result); + // Return one character through a plain char pointer. + HRESULT GetCharacterBytes([in] byte value, [out, retval] char* result); +} ; + // Interface IDispSafearrayParamTest [ uuid(4097A6D0-A111-40E2-BD0B-177B775A9496) @@ -165,6 +181,15 @@ library ComtypesCppTestSrvLib [default] interface INumericSafearrayTest ; } ; + [ + uuid(E07D71E8-C8A5-4B76-9A88-688DB10CF712), + helpstring("Comtypes component for byte and narrow-string compatibility tests.") + ] + coclass CoByteCompatibilityTest + { + [default] interface IByteCompatibilityTest ; + } ; + // Interface IStringSafearrayInsideVariantTest [ odl,