COM Record field assignment using type information and multidim SAFEARRAYs as field values

CHANGES.md

@@ -18,10 +18,13 @@ 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])
 * 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
 ------------------------------

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);

com/TestSources/PyCOMTest/PyCOMImpl.h

@@ -131,6 +131,7 @@ class CPyCOMTest : public IDispatchImpl<IPyCOMTest, &IID_IPyCOMTest, &LIBID_PyCO
 
     STDMETHOD(ModifyStruct)(TestStruct1 *prec);
     STDMETHOD(VerifyArrayOfStructs)(TestStruct2 *prec, VARIANT_BOOL *is_ok);
+    STDMETHOD(ModifyArrayOfStructs)(SAFEARRAY **array_of_structs);
 
     // info associated to each session
     struct PyCOMTestSessionData {

com/TestSources/PyCOMTest/PyCOMTest.idl

@@ -94,6 +94,11 @@ library PyCOMTestLib
         SAFEARRAY(TestStruct1) array_of_records;
         int rec_count;
     } TestStruct2;
+    typedef [uuid(865045EB-A7AE-4E88-B102-E2C5B97A64B6), version(1.0)]
+    struct TestStruct3 {
+        SAFEARRAY(double) array_of_double;
+        float id;
+    } TestStruct3;
 
 	// Test enumerators.
    [
@@ -311,6 +316,7 @@ library PyCOMTestLib
         // Test struct byref as [ in, out ] parameter.
         HRESULT ModifyStruct([ in, out ] TestStruct1 * prec);
         HRESULT VerifyArrayOfStructs([in] TestStruct2 * prec, [ out, retval ] VARIANT_BOOL * is_ok);
+        HRESULT ModifyArrayOfStructs([in, out] SAFEARRAY(TestStruct3) * array_of_structs);
 	};
 
 	// Define a new class to test how Python handles derived interfaces!

com/TestSources/PyCOMTest/PyCOMTest.vcxproj

@@ -163,4 +163,4 @@
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
   <ImportGroup Label="ExtensionTargets">
   </ImportGroup>
-</Project>
+</Project>

com/win32com/src/PyRecord.cpp

@@ -53,25 +53,31 @@ BOOL PyObject_AsVARIANTRecordInfo(PyObject *ob, VARIANT *pv)
     return TRUE;
 }
 
-PyObject *PyObject_FromSAFEARRAYRecordInfo(SAFEARRAY *psa)
+PyObject *PyObject_FromSAFEARRAYRecordInfo(SAFEARRAY *psa, long *arrayIndices)
 {
     PyObject *ret = NULL, *ret_tuple = NULL;
     IRecordInfo *info = NULL;
     BYTE *source_data = NULL, *this_dest_data = NULL;
+    UINT dimNo = 0;
     long lbound, ubound, nelems, i;
     ULONG cb_elem;
     PyRecordBuffer *owner = NULL;
+    dimNo = SafeArrayGetDim(psa);
+    if (dimNo == 0) {
+        goto exit;
+    }
+    long *pMyArrayIndex = arrayIndices + (dimNo - 1);
     HRESULT hr = SafeArrayGetRecordInfo(psa, &info);
     if (FAILED(hr))
         goto exit;
-    hr = SafeArrayAccessData(psa, (void **)&source_data);
+    hr = SafeArrayLock(psa);
     if (FAILED(hr))
         goto exit;
     // Allocate a new chunk of memory
-    hr = SafeArrayGetUBound(psa, 1, &ubound);
+    hr = SafeArrayGetUBound(psa, dimNo, &ubound);
     if (FAILED(hr))
         goto exit;
-    hr = SafeArrayGetLBound(psa, 1, &lbound);
+    hr = SafeArrayGetLBound(psa, dimNo, &lbound);
     if (FAILED(hr))
         goto exit;
     nelems = ubound - lbound + 1;
@@ -86,11 +92,13 @@ PyObject *PyObject_FromSAFEARRAYRecordInfo(SAFEARRAY *psa)
     if (ret_tuple == NULL)
         goto exit;
     this_dest_data = (BYTE *)owner->data;
-    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);

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) {