From 246d028e66744a7866318a2ee5899bbe92157564 Mon Sep 17 00:00:00 2001 From: Thomas Geppert Date: Mon, 22 Sep 2025 09:56:56 +0200 Subject: [PATCH 1/5] Implementation of multidimensional SAFEARRAY(COM Record) and SAFEARRAY(double) Create SAFEARRAY(double) from a sequence of PyFloat objects instead of SAFEARRAY(VARIANT(RT_8)). The implementation builds multidimensional SAFEARRAYs from nested sequences. The creation of SAFEARRAY(VT_RECORD) was improved to also allow nested sequences. --- CHANGES.md | 4 ++ com/TestSources/PyCOMTest/PyCOMImpl.cpp | 61 +++++++++++++++++ com/TestSources/PyCOMTest/PyCOMImpl.h | 1 + com/TestSources/PyCOMTest/PyCOMTest.idl | 6 ++ com/win32com/src/PyRecord.cpp | 91 +++++++------------------ com/win32com/src/oleargs.cpp | 42 ++++++------ com/win32com/test/testPyComTest.py | 80 ++++++++++++++++++++++ 7 files changed, 199 insertions(+), 86 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 29eb008c76..1615a08a47 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -18,6 +18,10 @@ As of build 305, installation .exe files have been deprecated; see Coming in build 312, as yet unreleased -------------------------------------- +* Implement multidimensional SAFEARRAY(COM Record) and SAFEARRAY(double) (mhammond#, [@geppi][geppi]) +* Bugfix for COM Record instance creation (mhammond#2641, [@geppi][geppi]) +* Fix regression introduced by mhammond#2506 (mhammond#2640, [@geppi][geppi]) + Build 311, released 2025/07/14 ------------------------------ diff --git a/com/TestSources/PyCOMTest/PyCOMImpl.cpp b/com/TestSources/PyCOMTest/PyCOMImpl.cpp index 332ebd5688..e3a738c78e 100644 --- a/com/TestSources/PyCOMTest/PyCOMImpl.cpp +++ b/com/TestSources/PyCOMTest/PyCOMImpl.cpp @@ -650,9 +650,70 @@ HRESULT CPyCOMTest::VerifyArrayOfStructs(TestStruct2 *prec, VARIANT_BOOL *is_ok) break; } } + hr = SafeArrayUnaccessData(prec->array_of_records); + if (FAILED(hr)) { + return E_FAIL; + } return S_OK; } +HRESULT CPyCOMTest::ModifyArrayOfStructs(SAFEARRAY **array_of_structs) +{ + HRESULT hr; + double *d; + LONG index[3] = {0, 0, 0}; + LONG d_index[3] = {0, 0, 0}; + TestStruct3 *pstruct; + + // This method loops over all Records in a 3 dimensional SAFEARRAY and + // multiplies each Records 'array_of_double' member, which is a 3 + // dimensional SAFEARRAY of doubles, element wise with a number calculated + // from the loop indices. + hr = SafeArrayLock(*array_of_structs); + if (FAILED(hr)) { + return E_FAIL; + } + for (int k = 0; k < 4; k++) { + index[0] = k; + for (int j = 0; j < 5; j++) { + index[1] = j; + for (int i = 0; i < 3; i++) { + index[2] = i; + hr = SafeArrayPtrOfIndex(*array_of_structs, index, (void **)&pstruct); + if (FAILED(hr)) { + return E_FAIL; + } + hr = SafeArrayLock(pstruct->array_of_double); + if (FAILED(hr)) { + return E_FAIL; + } + for (int n = 0; n < 4; n++) { + d_index[0] = n; + for (int m = 0; m < 5; m++) { + d_index[1] = m; + for (int l = 0; l < 3; l++) { + d_index[2] = l; + hr = SafeArrayPtrOfIndex(pstruct->array_of_double, d_index, (void **)&d); + if (FAILED(hr)) { + return E_FAIL; + } + *d = *d * (k * 15 + j * 3 + i); + } + } + } + hr = SafeArrayUnlock(pstruct->array_of_double); + if (FAILED(hr)) { + return E_FAIL; + } + } + } + } + hr = SafeArrayUnlock(*array_of_structs); + if (FAILED(hr)) { + return E_FAIL; + } +} + HRESULT CPyCOMTest::DoubleString(BSTR in, BSTR *out) { *out = SysAllocStringLen(NULL, SysStringLen(in) * 2); diff --git a/com/TestSources/PyCOMTest/PyCOMImpl.h b/com/TestSources/PyCOMTest/PyCOMImpl.h index 416a1ccaa2..0968c4be3c 100644 --- a/com/TestSources/PyCOMTest/PyCOMImpl.h +++ b/com/TestSources/PyCOMTest/PyCOMImpl.h @@ -131,6 +131,7 @@ class CPyCOMTest : public IDispatchImpldata; - for (i = 0; i < nelems; i++) { + for (i = 0; i < nelems; (*pMyArrayIndex)++, i++) { hr = info->RecordInit(this_dest_data); if (FAILED(hr)) goto exit; - + hr = SafeArrayPtrOfIndex(psa, arrayIndices, (void **)&source_data); + if (FAILED(hr)) + goto exit; hr = info->RecordCopy(source_data, this_dest_data); if (FAILED(hr)) goto exit; @@ -99,7 +107,6 @@ PyObject *PyObject_FromSAFEARRAYRecordInfo(SAFEARRAY *psa) goto exit; PyTuple_SET_ITEM(ret_tuple, i, rec); this_dest_data += cb_elem; - source_data += cb_elem; } ret = ret_tuple; Py_INCREF(ret); // for decref on cleanup. @@ -118,7 +125,7 @@ PyObject *PyObject_FromSAFEARRAYRecordInfo(SAFEARRAY *psa) if (info) info->Release(); if (source_data != NULL) - SafeArrayUnaccessData(psa); + SafeArrayUnlock(psa); return ret; } // Creates a new Record by TAKING A COPY of the passed record. @@ -675,66 +682,20 @@ PyObject *PyRecord::getattro(PyObject *self, PyObject *obname) return PyCom_BuildPyException(hr, pyrec->pri, IID_IRecordInfo); } - // Short-circuit sub-structs and arrays here, so we don't allocate a new chunk - // of memory and copy it - we need sub-structs to persist. + // Short-circuit sub-structs here. if (V_VT(&vret) == (VT_BYREF | VT_RECORD)) return PyRecord::new_record(V_RECORDINFO(&vret), V_RECORD(&vret), pyrec->owner); - else if (V_VT(&vret) == (VT_BYREF | VT_ARRAY | VT_RECORD)) { - SAFEARRAY *psa = *V_ARRAYREF(&vret); - if (SafeArrayGetDim(psa) != 1) - return PyErr_Format(PyExc_TypeError, "Only support single dimensional arrays of records"); - IRecordInfo *sub = NULL; - long ubound, lbound, nelems; - int i; - BYTE *this_data; - PyObject *ret_tuple = NULL; - ULONG element_size = 0; - hr = SafeArrayGetUBound(psa, 1, &ubound); - if (FAILED(hr)) - goto array_end; - hr = SafeArrayGetLBound(psa, 1, &lbound); - if (FAILED(hr)) - goto array_end; - hr = SafeArrayGetRecordInfo(psa, &sub); - if (FAILED(hr)) - goto array_end; - hr = sub->GetSize(&element_size); - if (FAILED(hr)) - goto array_end; - nelems = ubound - lbound + 1; - ret_tuple = PyTuple_New(nelems); - if (ret_tuple == NULL) - goto array_end; - // We're dealing here with a Record field that is a SAFEARRAY of Records. - // Therefore the VARIANT that was returned by the call to 'pyrec->pri->GetFieldNoCopy' - // does contain a reference to the SAFEARRAY of Records, i.e. the actual data of the - // Record elements of this SAFEARRAY is referenced by the 'pvData' field of the SAFEARRAY. - // In this particular case the implementation of 'GetFieldNoCopy' returns a NULL pointer - // in the last parameter, i.e. 'sub_data == NULL'. - this_data = (BYTE *)psa->pvData; - for (i = 0; i < nelems; i++) { - PyRecord *rec = PyRecord::new_record(sub, this_data, pyrec->owner); - if (rec == NULL) { - Py_DECREF(ret_tuple); - ret_tuple = NULL; - goto array_end; - } - PyTuple_SET_ITEM(ret_tuple, i, rec); - this_data += element_size; - } - array_end: - if (sub) - sub->Release(); - if (FAILED(hr)) - return PyCom_BuildPyException(hr, pyrec->pri, IID_IRecordInfo); - return ret_tuple; - } - // This default conversion we use is a little slow (but it will do!) - // For arrays, the pparray->pvData member is *not* set, since the actual data - // pointer from the record is returned in sub_data, so set it here. - if (V_ISARRAY(&vret) && V_ISBYREF(&vret)) - (*V_ARRAYREF(&vret))->pvData = sub_data; + // The following old comment seems to be invalid because the 'pparray->pvData' member + // does indeed contain a reference to the actual data of the SAFEARRAY and the + // last parameter 'sub_data' contains a NULL pointer afer the call to 'GetFieldNoCopy'. + // Leaving the old comment here for reference in any case: + /* + * Invalid * For arrays, the pparray->pvData member is *not* set, since the actual data + * Invalid * pointer from the record is returned in sub_data, so set it here. + * Invalid * if (V_ISARRAY(&vret) && V_ISBYREF(&vret)) + * Invalid * (*V_ARRAYREF(&vret))->pvData = sub_data; + */ PyObject *ret = PyCom_PyObjectFromVariant(&vret); // VariantClear(&vret); diff --git a/com/win32com/src/oleargs.cpp b/com/win32com/src/oleargs.cpp index 40e90c3f2b..2c78a3dbf3 100644 --- a/com/win32com/src/oleargs.cpp +++ b/com/win32com/src/oleargs.cpp @@ -7,7 +7,7 @@ #include "PyRecord.h" extern PyObject *PyObject_FromRecordInfo(IRecordInfo *, void *, ULONG, PyTypeObject *type = NULL); -extern PyObject *PyObject_FromSAFEARRAYRecordInfo(SAFEARRAY *psa); +extern PyObject *PyObject_FromSAFEARRAYRecordInfo(SAFEARRAY *psa, long *arrayIndices); extern BOOL PyObject_AsVARIANTRecordInfo(PyObject *ob, VARIANT *pv); extern BOOL PyRecord_Check(PyObject *ob); @@ -279,18 +279,22 @@ BOOL PyCom_VariantFromPyObject(PyObject *obj, VARIANT *var) // So make sure this check is after anything else which qualifies. else if (PySequence_Check(obj)) { V_ARRAY(var) = NULL; // not a valid, existing array. - BOOL is_record_item = false; - if (PyObject_Length(obj) > 0) { - PyObject *obItemCheck = PySequence_GetItem(obj, 0); - is_record_item = PyRecord_Check(obItemCheck); - } - // If the sequence elements are PyRecord objects we do NOT package - // them as VARIANT elements but put them directly into the SAFEARRAY. - if (is_record_item) { + // If the sequence elements are PyRecord or PyFloat objects we do NOT package + // them as VARIANT elements but put them unpackaged into the SAFEARRAY. If we + // deal with nested sequences, we need to go down until we find a non sequence + // element to figure out the VARTYPE. + PyObject *obItemCheck = PySequence_GetItem(obj, 0); + while (obItemCheck && PySequence_Check(obItemCheck)) obItemCheck = PySequence_GetItem(obItemCheck, 0); + if (obItemCheck != NULL && PyRecord_Check(obItemCheck)) { if (!PyCom_SAFEARRAYFromPyObject(obj, &V_ARRAY(var), VT_RECORD)) return FALSE; V_VT(var) = VT_ARRAY | VT_RECORD; } + else if (obItemCheck != NULL && PyFloat_Check(obItemCheck)) { + if (!PyCom_SAFEARRAYFromPyObject(obj, &V_ARRAY(var), VT_R8)) + return FALSE; + V_VT(var) = VT_ARRAY | VT_R8; + } else { if (!PyCom_SAFEARRAYFromPyObject(obj, &V_ARRAY(var))) return FALSE; @@ -784,7 +788,6 @@ static BOOL PyCom_SAFEARRAYFromPyObjectEx(PyObject *obj, SAFEARRAY **ppSA, bool // OK - Finally can create the array... if (vt == VT_RECORD) { // SAFEARRAYS of UDTs need a special treatment. - obItemCheck = PySequence_GetItem(obj, 0); PyRecord *pyrec = (PyRecord *)obItemCheck; *ppSA = SafeArrayCreateEx(vt, cDims, pBounds, pyrec->pri); } @@ -936,8 +939,6 @@ static PyObject *PyCom_PyObjectFromSAFEARRAYDimensionItem(SAFEARRAY *psa, VARENU subitem = PyCom_PyObjectFromIUnknown(pUnk, IID_IUnknown, FALSE); break; } - // case VT_RECORD - case VT_I1: case VT_UI1: { unsigned char i1; @@ -1044,20 +1045,19 @@ PyObject *PyCom_PyObjectFromSAFEARRAYBuildDimension(SAFEARRAY *psa, VARENUM vt, SafeArrayUnaccessData(psa); return ret; } - // Another shortcut for VT_RECORD types. - if (vt == VT_RECORD) { - return PyObject_FromSAFEARRAYRecordInfo(psa); - } - // Normal SAFEARRAY case returning a tuple. + BOOL bBuildItems = (nDims == dimNo); + // Get a pointer for the dimension to iterate. + long *pMyArrayIndex = arrayIndices + (dimNo - 1); + *pMyArrayIndex = lb; + // For Records wie arrange all BuildItems in one single RecordBuffer. + if (bBuildItems && vt == VT_RECORD) { + return PyObject_FromSAFEARRAYRecordInfo(psa, arrayIndices); + } PyObject *retTuple = PyTuple_New(ub - lb + 1); if (retTuple == NULL) return FALSE; int tupleIndex = 0; - // Get a pointer for the dimension to iterate (the last one) - long *pMyArrayIndex = arrayIndices + (dimNo - 1); - *pMyArrayIndex = lb; - BOOL bBuildItems = (nDims == dimNo); for (; *pMyArrayIndex <= ub; (*pMyArrayIndex)++, tupleIndex++) { PyObject *subItem = NULL; if (bBuildItems) { diff --git a/com/win32com/test/testPyComTest.py b/com/win32com/test/testPyComTest.py index 921a45630a..3b19c6a86c 100644 --- a/com/win32com/test/testPyComTest.py +++ b/com/win32com/test/testPyComTest.py @@ -73,6 +73,15 @@ class TestStruct2(pythoncom.com_record): GUID = "{78F0EA07-B7CF-42EA-A251-A4C6269F76AF}" +class TestStruct3(pythoncom.com_record): + __slots__ = () + TLBID = "{6BCDCB60-5605-11D0-AE5F-CADD4C000000}" + MJVER = 1 + MNVER = 1 + LCID = 0 + GUID = "{865045EB-A7AE-4E88-B102-E2C5B97A64B6}" + + # We don't need to stick with the struct name in the TypeLibrary for the subclass name. # The following class has the same GUID as TestStruct2 from the TypeLibrary. class ArrayOfStructsTestStruct(pythoncom.com_record): @@ -519,6 +528,74 @@ def TestArrayOfStructs(o, test_rec): assert o.VerifyArrayOfStructs(test_rec) +def TestNestedArrays(o): + # First we test the assignment of a nested sequence of Records + # to a Record member attribute, followed by the retrieval of the + # multidimensional SAFEARRAY from the Record member attribute. + # Create a nested 3 dimensional tuple of COM Records. + record_tuple = tuple( + [ + tuple([tuple([TestStruct1() for i in range(3)]) for j in range(5)]) + for k in range(4) + ] + ) + # Assign a different integer identifier to each Record in the nested tuple. + for k in range(4): + for j in range(5): + for i in range(3): + record_tuple[k][j][i].int_value = k * 15 + j * 3 + i + # Create an instance of a COM Record that has a member of type + # SAFEARRAY(TestStruct1) and assign the nested tuple to this member. + rec_with_multidim_sa_of_rec = ArrayOfStructsTestStruct() + rec_with_multidim_sa_of_rec.array_of_records = record_tuple + # Now retrieve a tuple from the Record member and check that + # it is equal to our input nested tuple but not the same object. + tuple_retrieved_from_rec = rec_with_multidim_sa_of_rec.array_of_records + assert tuple_retrieved_from_rec == record_tuple + assert tuple_retrieved_from_rec is not record_tuple + # Next we test passing a multidimensional SAFEARRAY of Records + # to a COM method that modifies the Records in the SAFEARRAY. + record_tuple = tuple( + [ + tuple([tuple([TestStruct3() for i in range(3)]) for j in range(5)]) + for k in range(4) + ] + ) + # We also test a nested sequence of doubles assigned + # to a member attribute of the Records. + float_tuple = tuple( + [ + tuple( + [tuple([float(i + 3 * j + 15 * k) for i in range(3)]) for j in range(5)] + ) + for k in range(4) + ] + ) + # Assign the nested sequence of doubles to the SAFEARRAY member attribute + # of the Records in the nested Record sequence and also assign a unique + # identifier to each of the Records. + for k in range(4): + for j in range(5): + for i in range(3): + record_tuple[k][j][i].id = float(k * 15 + j * 3 + i) + record_tuple[k][j][i].array_of_double = float_tuple + # Use the created nested sequence in the call to a COM method + # that modifies the Records. Note that the array dimension sizes + # are hard wired in the COM method. + array_of_structs = o.ModifyArrayOfStructs(record_tuple) + # The method should have multiplied each element of each of the + # SAFEARRAY(double) Record members by the id of the respective Record. + for k in range(4): + for j in range(5): + for i in range(3): + for n in range(4): + for m in range(5): + for l in range(3): + rec = array_of_structs[k][j][i] + f = float_tuple[n][m][l] + assert rec.array_of_double[n][m][l] == f * rec.id + + def TestGenerated(): # Create an instance of the server. from win32com.client.gencache import EnsureDispatch @@ -566,6 +643,7 @@ def TestGenerated(): # Register the subclasses in pythoncom. register_record_class(TestStruct1) register_record_class(ArrayOfStructsTestStruct) + register_record_class(TestStruct3) # Now the type of the instance is the registered subclass. r_sub = TestStruct1() assert type(r_sub) is TestStruct1 @@ -588,6 +666,8 @@ def TestGenerated(): test_rec = ArrayOfStructsTestStruct() assert type(test_rec) is ArrayOfStructsTestStruct TestArrayOfStructs(o, test_rec) + progress("Testing multidimensional SAFEARRAYS of Records and double.") + TestNestedArrays(o) # Test initialization of registered pythoncom.com_record subclasses. progress("Testing initialization of pythoncom.com_record subclasses.") From 96575e33d5783d9dafe746009deb74dc00978afb Mon Sep 17 00:00:00 2001 From: Thomas Geppert Date: Thu, 25 Sep 2025 17:41:02 +0200 Subject: [PATCH 2/5] Retargeting the Visual Studio project of PyCOMTest. --- CHANGES.md | 2 +- com/TestSources/PyCOMTest/PyCOMTest.vcxproj | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 1615a08a47..5f8b559fc7 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -18,7 +18,7 @@ As of build 305, installation .exe files have been deprecated; see Coming in build 312, as yet unreleased -------------------------------------- -* Implement multidimensional SAFEARRAY(COM Record) and SAFEARRAY(double) (mhammond#, [@geppi][geppi]) +* Implement multidimensional SAFEARRAY(COM Record) and SAFEARRAY(double) (mhammond#2655, [@geppi][geppi]) * Bugfix for COM Record instance creation (mhammond#2641, [@geppi][geppi]) * Fix regression introduced by mhammond#2506 (mhammond#2640, [@geppi][geppi]) diff --git a/com/TestSources/PyCOMTest/PyCOMTest.vcxproj b/com/TestSources/PyCOMTest/PyCOMTest.vcxproj index 050d0f6b60..4232e05a84 100644 --- a/com/TestSources/PyCOMTest/PyCOMTest.vcxproj +++ b/com/TestSources/PyCOMTest/PyCOMTest.vcxproj @@ -24,7 +24,7 @@ Win32Proj - 10.0.20348.0 + 10.0 @@ -42,13 +42,13 @@ DynamicLibrary true - v142 + v143 NotSet DynamicLibrary false - v142 + v143 NotSet @@ -163,4 +163,4 @@ - + \ No newline at end of file From b3ff56130f26a678e02dacca294a3b8724192f85 Mon Sep 17 00:00:00 2001 From: Thomas Geppert Date: Sun, 28 Sep 2025 12:38:27 +0200 Subject: [PATCH 3/5] Update testPyComTest.py Resolve an inefficiency --- com/win32com/test/testPyComTest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com/win32com/test/testPyComTest.py b/com/win32com/test/testPyComTest.py index 3b19c6a86c..96a3c7c8b9 100644 --- a/com/win32com/test/testPyComTest.py +++ b/com/win32com/test/testPyComTest.py @@ -588,10 +588,10 @@ def TestNestedArrays(o): for k in range(4): for j in range(5): for i in range(3): + rec = array_of_structs[k][j][i] for n in range(4): for m in range(5): for l in range(3): - rec = array_of_structs[k][j][i] f = float_tuple[n][m][l] assert rec.array_of_double[n][m][l] == f * rec.id From d409a28c8d5c4e86005e3945c78eea4abd6db93a Mon Sep 17 00:00:00 2001 From: Thomas Geppert Date: Mon, 29 Sep 2025 08:26:10 +0200 Subject: [PATCH 4/5] Addressing first code review. --- com/TestSources/PyCOMTest/PyCOMImpl.cpp | 17 +++++++++-------- com/TestSources/PyCOMTest/PyCOMTest.vcxproj | 2 +- com/win32com/src/PyRecord.cpp | 4 +++- com/win32com/src/oleargs.cpp | 2 +- 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/com/TestSources/PyCOMTest/PyCOMImpl.cpp b/com/TestSources/PyCOMTest/PyCOMImpl.cpp index e3a738c78e..27947c7bc6 100644 --- a/com/TestSources/PyCOMTest/PyCOMImpl.cpp +++ b/com/TestSources/PyCOMTest/PyCOMImpl.cpp @@ -641,7 +641,7 @@ HRESULT CPyCOMTest::VerifyArrayOfStructs(TestStruct2 *prec, VARIANT_BOOL *is_ok) hr = SafeArrayAccessData(prec->array_of_records, reinterpret_cast(&pdata)); if (FAILED(hr)) { - return E_FAIL; + return hr; } *is_ok = VARIANT_TRUE; for (i = 0; i < prec->rec_count; i++) { @@ -652,7 +652,7 @@ HRESULT CPyCOMTest::VerifyArrayOfStructs(TestStruct2 *prec, VARIANT_BOOL *is_ok) } hr = SafeArrayUnaccessData(prec->array_of_records); if (FAILED(hr)) { - return E_FAIL; + return hr; } return S_OK; } @@ -671,7 +671,7 @@ HRESULT CPyCOMTest::ModifyArrayOfStructs(SAFEARRAY **array_of_structs) // from the loop indices. hr = SafeArrayLock(*array_of_structs); if (FAILED(hr)) { - return E_FAIL; + return hr; } for (int k = 0; k < 4; k++) { index[0] = k; @@ -681,11 +681,11 @@ HRESULT CPyCOMTest::ModifyArrayOfStructs(SAFEARRAY **array_of_structs) index[2] = i; hr = SafeArrayPtrOfIndex(*array_of_structs, index, (void **)&pstruct); if (FAILED(hr)) { - return E_FAIL; + return hr; } hr = SafeArrayLock(pstruct->array_of_double); if (FAILED(hr)) { - return E_FAIL; + return hr; } for (int n = 0; n < 4; n++) { d_index[0] = n; @@ -695,7 +695,7 @@ HRESULT CPyCOMTest::ModifyArrayOfStructs(SAFEARRAY **array_of_structs) d_index[2] = l; hr = SafeArrayPtrOfIndex(pstruct->array_of_double, d_index, (void **)&d); if (FAILED(hr)) { - return E_FAIL; + return hr; } *d = *d * (k * 15 + j * 3 + i); } @@ -703,15 +703,16 @@ HRESULT CPyCOMTest::ModifyArrayOfStructs(SAFEARRAY **array_of_structs) } hr = SafeArrayUnlock(pstruct->array_of_double); if (FAILED(hr)) { - return E_FAIL; + return hr; } } } } hr = SafeArrayUnlock(*array_of_structs); if (FAILED(hr)) { - return E_FAIL; + return hr; } + return S_OK; } HRESULT CPyCOMTest::DoubleString(BSTR in, BSTR *out) diff --git a/com/TestSources/PyCOMTest/PyCOMTest.vcxproj b/com/TestSources/PyCOMTest/PyCOMTest.vcxproj index 4232e05a84..1bbcc2fea5 100644 --- a/com/TestSources/PyCOMTest/PyCOMTest.vcxproj +++ b/com/TestSources/PyCOMTest/PyCOMTest.vcxproj @@ -163,4 +163,4 @@ - \ No newline at end of file + diff --git a/com/win32com/src/PyRecord.cpp b/com/win32com/src/PyRecord.cpp index 8a342480da..d07f86fa9f 100644 --- a/com/win32com/src/PyRecord.cpp +++ b/com/win32com/src/PyRecord.cpp @@ -62,12 +62,14 @@ PyObject *PyObject_FromSAFEARRAYRecordInfo(SAFEARRAY *psa, long *arrayIndices) long lbound, ubound, nelems, i; ULONG cb_elem; PyRecordBuffer *owner = NULL; + HRESULT hr; dimNo = SafeArrayGetDim(psa); if (dimNo == 0) { + hr = E_UNEXPECTED; goto exit; } long *pMyArrayIndex = arrayIndices + (dimNo - 1); - HRESULT hr = SafeArrayGetRecordInfo(psa, &info); + hr = SafeArrayGetRecordInfo(psa, &info); if (FAILED(hr)) goto exit; hr = SafeArrayLock(psa); diff --git a/com/win32com/src/oleargs.cpp b/com/win32com/src/oleargs.cpp index 2c78a3dbf3..523d6b3bb8 100644 --- a/com/win32com/src/oleargs.cpp +++ b/com/win32com/src/oleargs.cpp @@ -1050,7 +1050,7 @@ PyObject *PyCom_PyObjectFromSAFEARRAYBuildDimension(SAFEARRAY *psa, VARENUM vt, // Get a pointer for the dimension to iterate. long *pMyArrayIndex = arrayIndices + (dimNo - 1); *pMyArrayIndex = lb; - // For Records wie arrange all BuildItems in one single RecordBuffer. + // For Records we arrange all BuildItems in one single RecordBuffer. if (bBuildItems && vt == VT_RECORD) { return PyObject_FromSAFEARRAYRecordInfo(psa, arrayIndices); } From 6617885dd411c8ab580b1f16c83d2c13b80ad161 Mon Sep 17 00:00:00 2001 From: Thomas Geppert Date: Sat, 4 Oct 2025 15:29:51 +0200 Subject: [PATCH 5/5] Changed the 'setattro' method of PyRecord to use ITypeInfo. --- com/TestSources/PyCOMTest/PyCOMTest.idl | 1 + com/win32com/src/PyRecord.cpp | 101 ++++++++++++++++++++++-- com/win32com/src/oleargs.cpp | 26 ++---- com/win32com/test/testPyComTest.py | 32 +++++--- 4 files changed, 122 insertions(+), 38 deletions(-) diff --git a/com/TestSources/PyCOMTest/PyCOMTest.idl b/com/TestSources/PyCOMTest/PyCOMTest.idl index 0ae21c8673..d3aa54ecad 100644 --- a/com/TestSources/PyCOMTest/PyCOMTest.idl +++ b/com/TestSources/PyCOMTest/PyCOMTest.idl @@ -96,6 +96,7 @@ library PyCOMTestLib } TestStruct2; typedef [uuid(865045EB-A7AE-4E88-B102-E2C5B97A64B6), version(1.0)] struct TestStruct3 { + TestStruct1 a_struct_field; SAFEARRAY(double) array_of_double; float id; } TestStruct3; diff --git a/com/win32com/src/PyRecord.cpp b/com/win32com/src/PyRecord.cpp index d07f86fa9f..ecfa3b8f4c 100644 --- a/com/win32com/src/PyRecord.cpp +++ b/com/win32com/src/PyRecord.cpp @@ -706,27 +706,112 @@ PyObject *PyRecord::getattro(PyObject *self, PyObject *obname) int PyRecord::setattro(PyObject *self, PyObject *obname, PyObject *v) { + int ret = 0; VARIANT val; VariantInit(&val); PyRecord *pyrec = (PyRecord *)self; - - if (!PyCom_VariantFromPyObject(v, &val)) - return -1; - + ITypeInfo *pti = NULL; + TYPEATTR *pta = NULL; + MEMBERID mem_id; + VARDESC *pVarDesc = NULL; + HRESULT hr; WCHAR *wname; + VARTYPE vt; + if (!PyWinObject_AsWCHAR(obname, &wname, FALSE)) return -1; + // We need to create a VARIANT from the PyObject to set + // the new Record field value. Before we can do this, we + // need to retrieve the information about the required + // VARIANT type from ITypeInfo. + hr = pyrec->pri->GetTypeInfo(&pti); + if (FAILED(hr) || pti == NULL) { + PyCom_BuildPyException(hr, pyrec->pri, IID_IRecordInfo); + ret = -1; + goto done; + } + // Unfortunately there is no method in ITypeInfo to dirctly + // map a field name to the variable description of the field. + // Field names are mapped to a member ID but 'GetVarDesc' + // uses an 'index' to identify the field and the 'index' is + // not the member ID. :-( + // We need to take a small detour and loop over all variable + // descriptions for this Record type to find the one with a + // matching member ID. + hr = pti->GetTypeAttr(&pta); + if (FAILED(hr) || pta == NULL) { + PyCom_BuildPyException(hr); + ret = -1; + goto done; + } + hr = pti->GetIDsOfNames(&wname, 1, &mem_id); + if (FAILED(hr)) { + PyCom_BuildPyException(hr); + ret = -1; + goto done; + } + int idx; + for (idx = 0; idx < pta->cVars; idx++) { + hr = pti->GetVarDesc(idx, &pVarDesc); + if (FAILED(hr)) { + PyCom_BuildPyException(hr); + ret = -1; + goto done; + } + if ((pVarDesc)->memid == mem_id) + break; + pti->ReleaseVarDesc(pVarDesc); + } + vt = (pVarDesc)->elemdescVar.tdesc.vt; + // The documentation for the TYPEDESC structure (oaidl.h) states: + // "If the variable is VT_SAFEARRAY or VT_PTR, the union portion + // of the TYPEDESC contains a pointer to a TYPEDESC that specifies + // the element type." + if (vt == VT_SAFEARRAY) { + vt = (pVarDesc)->elemdescVar.tdesc.lpadesc->tdescElem.vt; + // The struct definitions of COM Records in an IDL file are + // translated to an element type of 'VT_USERDEFINED'. + if (vt == VT_USERDEFINED) { + vt = VT_RECORD; + } + if (!PyCom_SAFEARRAYFromPyObject(v, &V_ARRAY(&val), (VARENUM)vt)) { + ret = -1; + goto done; + } + V_VT(&val) = VT_ARRAY | vt; + } + else { + PythonOleArgHelper helper; + // The struct definitions of COM Records in an IDL file are + // translated to an element type of 'VT_USERDEFINED'. + if (vt == VT_USERDEFINED) { + vt = VT_RECORD; + } + helper.m_reqdType = vt; + if (!helper.MakeObjToVariant(v, &val)) { + ret = -1; + goto done; + } + } PY_INTERFACE_PRECALL; - HRESULT hr = pyrec->pri->PutField(INVOKE_PROPERTYPUT, pyrec->pdata, wname, &val); + hr = pyrec->pri->PutField(INVOKE_PROPERTYPUT, pyrec->pdata, wname, &val); PY_INTERFACE_POSTCALL; - PyWinObject_FreeWCHAR(wname); + VariantClear(&val); if (FAILED(hr)) { PyCom_BuildPyException(hr, pyrec->pri, IID_IRecordInfo); - return -1; + ret = -1; } - return 0; +done: + PyWinObject_FreeWCHAR(wname); + if (pta && pti) + pti->ReleaseTypeAttr(pta); + if (pVarDesc && pti) + pti->ReleaseVarDesc(pVarDesc); + if (pti) + pti->Release(); + return ret; } PyObject *PyRecord::tp_richcompare(PyObject *self, PyObject *other, int op) diff --git a/com/win32com/src/oleargs.cpp b/com/win32com/src/oleargs.cpp index 523d6b3bb8..8a5039d478 100644 --- a/com/win32com/src/oleargs.cpp +++ b/com/win32com/src/oleargs.cpp @@ -279,27 +279,9 @@ BOOL PyCom_VariantFromPyObject(PyObject *obj, VARIANT *var) // So make sure this check is after anything else which qualifies. else if (PySequence_Check(obj)) { V_ARRAY(var) = NULL; // not a valid, existing array. - // If the sequence elements are PyRecord or PyFloat objects we do NOT package - // them as VARIANT elements but put them unpackaged into the SAFEARRAY. If we - // deal with nested sequences, we need to go down until we find a non sequence - // element to figure out the VARTYPE. - PyObject *obItemCheck = PySequence_GetItem(obj, 0); - while (obItemCheck && PySequence_Check(obItemCheck)) obItemCheck = PySequence_GetItem(obItemCheck, 0); - if (obItemCheck != NULL && PyRecord_Check(obItemCheck)) { - if (!PyCom_SAFEARRAYFromPyObject(obj, &V_ARRAY(var), VT_RECORD)) - return FALSE; - V_VT(var) = VT_ARRAY | VT_RECORD; - } - else if (obItemCheck != NULL && PyFloat_Check(obItemCheck)) { - if (!PyCom_SAFEARRAYFromPyObject(obj, &V_ARRAY(var), VT_R8)) - return FALSE; - V_VT(var) = VT_ARRAY | VT_R8; - } - else { - if (!PyCom_SAFEARRAYFromPyObject(obj, &V_ARRAY(var))) - return FALSE; - V_VT(var) = VT_ARRAY | VT_VARIANT; - } + if (!PyCom_SAFEARRAYFromPyObject(obj, &V_ARRAY(var))) + return FALSE; + V_VT(var) = VT_ARRAY | VT_VARIANT; } else if (PyRecord_Check(obj)) { if (!PyObject_AsVARIANTRecordInfo(obj, var)) @@ -1358,6 +1340,7 @@ BOOL PythonOleArgHelper::MakeObjToVariant(PyObject *obj, VARIANT *var, PyObject *V_UI8REF(var) = 0; break; case VT_I4: + case VT_INT: if ((obUse = PyNumber_Long(obj)) == NULL) BREAK_FALSE V_I4(var) = PyLong_AsLong(obUse); @@ -1365,6 +1348,7 @@ BOOL PythonOleArgHelper::MakeObjToVariant(PyObject *obj, VARIANT *var, PyObject BREAK_FALSE; break; case VT_I4 | VT_BYREF: + case VT_INT | VT_BYREF: if (bCreateBuffers) V_I4REF(var) = &m_lBuf; diff --git a/com/win32com/test/testPyComTest.py b/com/win32com/test/testPyComTest.py index 96a3c7c8b9..64d7b21432 100644 --- a/com/win32com/test/testPyComTest.py +++ b/com/win32com/test/testPyComTest.py @@ -555,14 +555,18 @@ def TestNestedArrays(o): assert tuple_retrieved_from_rec is not record_tuple # Next we test passing a multidimensional SAFEARRAY of Records # to a COM method that modifies the Records in the SAFEARRAY. + # The test seems a little convoluted. However, it should show + # that we got the multidimensional indexing right. + # First step: + # We create a nested sequence of COM Records. record_tuple = tuple( [ tuple([tuple([TestStruct3() for i in range(3)]) for j in range(5)]) for k in range(4) ] ) - # We also test a nested sequence of doubles assigned - # to a member attribute of the Records. + # Second step: + # We create a nested sequence of doubles. float_tuple = tuple( [ tuple( @@ -571,17 +575,18 @@ def TestNestedArrays(o): for k in range(4) ] ) - # Assign the nested sequence of doubles to the SAFEARRAY member attribute - # of the Records in the nested Record sequence and also assign a unique - # identifier to each of the Records. + # Third step: + # We assign the nested sequence of doubles to the SAFEARRAY member attribute + # in each of the Records in the nested Record sequence. + # In addition we also assign a unique identifier to each of the Records. for k in range(4): for j in range(5): for i in range(3): record_tuple[k][j][i].id = float(k * 15 + j * 3 + i) record_tuple[k][j][i].array_of_double = float_tuple - # Use the created nested sequence in the call to a COM method - # that modifies the Records. Note that the array dimension sizes - # are hard wired in the COM method. + # Now we use the nested sequence of Records in the call to a COM method + # that modifies the Records. Note that the array dimension sizes are + # hard wired in the COM method. array_of_structs = o.ModifyArrayOfStructs(record_tuple) # The method should have multiplied each element of each of the # SAFEARRAY(double) Record members by the id of the respective Record. @@ -656,8 +661,17 @@ def TestGenerated(): # Also registering a class with a GUID that is not in the TypeLibrary should fail. check_get_set_raises(TypeError, register_record_class, NotInTypeLibraryTestStruct) - # Perform the 'Byref' and 'ArrayOfStruct tests using the registered subclasses. progress("Testing subclasses of pythoncom.com_record.") + # Test assignment and retrieval of a Record field. + member_struct = TestStruct1() + member_struct.int_value = 42 + member_struct.str_value = "The meaning of life, the universe and everything." + parent_struct = TestStruct3() + parent_struct.a_struct_field = member_struct + retrieved_struct = parent_struct.a_struct_field + assert retrieved_struct == member_struct + + # Perform the 'Byref' and 'ArrayOfStruct tests using the registered subclasses. r = o.GetStruct() # After 'TestStruct1' was registered as an instantiable subclass # of pythoncom.com_record, the return value should have this type.