diff --git a/CHANGES.md b/CHANGES.md index 924a33196..17b41bd4f 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -18,11 +18,14 @@ 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#2655, [@geppi][geppi]) * Fixed missing version stamp on built `.dll` and `.exe` files (mhammond#2647, [@Avasam][Avasam]) * Removed considerations for Windows 95/98/ME (mhammond#2400, [@Avasam][Avasam]) This removes the following constants: * `win32con.FILE_ATTRIBUTE_ATOMIC_WRITE` * `win32con.FILE_ATTRIBUTE_XACTION_WRITE` +* 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 332ebd568..27947c7bc 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++) { @@ -650,6 +650,68 @@ HRESULT CPyCOMTest::VerifyArrayOfStructs(TestStruct2 *prec, VARIANT_BOOL *is_ok) break; } } + hr = SafeArrayUnaccessData(prec->array_of_records); + if (FAILED(hr)) { + return hr; + } + 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 hr; + } + 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 hr; + } + hr = SafeArrayLock(pstruct->array_of_double); + if (FAILED(hr)) { + return hr; + } + 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 hr; + } + *d = *d * (k * 15 + j * 3 + i); + } + } + } + hr = SafeArrayUnlock(pstruct->array_of_double); + if (FAILED(hr)) { + return hr; + } + } + } + } + hr = SafeArrayUnlock(*array_of_structs); + if (FAILED(hr)) { + return hr; + } return S_OK; } diff --git a/com/TestSources/PyCOMTest/PyCOMImpl.h b/com/TestSources/PyCOMTest/PyCOMImpl.h index 416a1ccaa..0968c4be3 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 +109,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 +127,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 +684,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); @@ -743,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 40e90c3f2..8a5039d47 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,23 +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. - 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 (!PyCom_SAFEARRAYFromPyObject(obj, &V_ARRAY(var), VT_RECORD)) - return FALSE; - V_VT(var) = VT_ARRAY | VT_RECORD; - } - 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)) @@ -784,7 +770,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 +921,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 +1027,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 we 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) { @@ -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 921a45630..64d7b2143 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,79 @@ 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. + # 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) + ] + ) + # Second step: + # We create a nested sequence of doubles. + 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) + ] + ) + # 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 + # 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. + 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): + 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 +648,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 @@ -578,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. @@ -588,6 +680,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.")