Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement the creation of SAFEARRAY(VT_RECORD) from a sequence of COM Records #2317

Merged
merged 4 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ Coming in build 309, as yet unreleased
--------------------------------------

* Dropped support for Python 3.7 (#2207, @Avasam)
* Implement record pointers as [in, out] method parameters of a Dispatch Interface (#2310)
* Implement the creation of SAFEARRAY(VT_RECORD) from a sequence of COM Records (#2317, @geppi)
* Implement record pointers as [in, out] method parameters of a Dispatch Interface (#2304, #2310, @geppi)
* Fix memory leak converting to PyObject from some SAFEARRAY elements (#2316)
* Fix bug where makepy support was unnecessarily generated (#2354, #2353, @geppi)
* Fail sooner on invalid `win32timezone.TimeZoneInfo` creation (#2338, @Avasam)
Expand Down
21 changes: 21 additions & 0 deletions com/TestSources/PyCOMTest/PyCOMImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,27 @@ HRESULT CPyCOMTest::ModifyStruct(TestStruct1 *prec)
return S_OK;
}

HRESULT CPyCOMTest::VerifyArrayOfStructs(TestStruct2 *prec, VARIANT_BOOL *is_ok)
{
long i;
TestStruct1 *pdata = NULL;
HRESULT hr;

hr = SafeArrayAccessData(prec->array_of_records, reinterpret_cast<void **>(&pdata));
if (FAILED(hr)) {
return E_FAIL;
}
*is_ok = VARIANT_TRUE;
for (i = 0; i < prec->rec_count; i++)
{
if (_wcsicmp(pdata[i].str_value, L"This is record number") != 0 || pdata[i].int_value != i + 1) {
*is_ok = VARIANT_FALSE;
break;
}
}
return S_OK;
}

HRESULT CPyCOMTest::DoubleString(BSTR in, BSTR *out)
{
*out = SysAllocStringLen(NULL, SysStringLen(in) * 2);
Expand Down
1 change: 1 addition & 0 deletions com/TestSources/PyCOMTest/PyCOMImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ class CPyCOMTest : public IDispatchImpl<IPyCOMTest, &IID_IPyCOMTest, &LIBID_PyCO
STDMETHOD(def)();

STDMETHOD(ModifyStruct)(TestStruct1 *prec);
STDMETHOD(VerifyArrayOfStructs)(TestStruct2 *prec, VARIANT_BOOL *is_ok);

// info associated to each session
struct PyCOMTestSessionData {
Expand Down
6 changes: 6 additions & 0 deletions com/TestSources/PyCOMTest/PyCOMTest.idl
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,11 @@ library PyCOMTestLib
int int_value;
BSTR str_value;
} StructWithoutUUID;
typedef [uuid(78F0EA07-B7CF-42EA-A251-A4C6269F76AF), version(1.0)]
struct TestStruct2 {
SAFEARRAY(TestStruct1) array_of_records;
int rec_count;
} TestStruct2;

// Test enumerators.
[
Expand Down Expand Up @@ -295,6 +300,7 @@ library PyCOMTestLib
HRESULT def();
// Test struct byref as [ in, out ] parameter.
HRESULT ModifyStruct([ in, out ] TestStruct1 * prec);
HRESULT VerifyArrayOfStructs([in] TestStruct2 * prec, [ out, retval ] VARIANT_BOOL * is_ok);
};

// Define a new class to test how Python handles derived interfaces!
Expand Down
11 changes: 7 additions & 4 deletions com/win32com/src/PyRecord.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -499,9 +499,6 @@ PyObject *PyRecord::getattro(PyObject *self, PyObject *obname)
return new PyRecord(V_RECORDINFO(&vret), V_RECORD(&vret), pyrec->owner);
else if (V_VT(&vret) == (VT_BYREF | VT_ARRAY | VT_RECORD)) {
SAFEARRAY *psa = *V_ARRAYREF(&vret);
int d = SafeArrayGetDim(psa);
if (sub_data == NULL)
return PyErr_Format(PyExc_RuntimeError, "Did not get a buffer for the array!");
if (SafeArrayGetDim(psa) != 1)
return PyErr_Format(PyExc_TypeError, "Only support single dimensional arrays of records");
IRecordInfo *sub = NULL;
Expand All @@ -526,7 +523,13 @@ PyObject *PyRecord::getattro(PyObject *self, PyObject *obname)
ret_tuple = PyTuple_New(nelems);
if (ret_tuple == NULL)
goto array_end;
this_data = (BYTE *)sub_data;
// 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++) {
PyTuple_SET_ITEM(ret_tuple, i, new PyRecord(sub, this_data, pyrec->owner));
this_data += element_size;
Expand Down
33 changes: 29 additions & 4 deletions com/win32com/src/oleargs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "stdafx.h"
#include "PythonCOM.h"
#include "PyRecord.h"

extern PyObject *PyObject_FromRecordInfo(IRecordInfo *, void *, ULONG);
extern PyObject *PyObject_FromSAFEARRAYRecordInfo(SAFEARRAY *psa);
Expand Down Expand Up @@ -278,9 +279,23 @@ 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 (!PyCom_SAFEARRAYFromPyObject(obj, &V_ARRAY(var)))
return FALSE;
V_VT(var) = VT_ARRAY | VT_VARIANT;
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.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm mildly surprised this check doesn't need to be deeper in PyCom_SAFEARRAYFromPyObject - are nested arrays of these a thing? But this still looks fine for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid point. To be honest I didn't open this Pandora's box because my use case doesn't require nested arrays.

Not sure how common they are.
I'm dealing with an engineering application that publishes round about 2000 COM methods and while the majority requires [in, out] Record pointers only 2 of them (unfortunately quite important ones) require plain Safearrays of Recods as parameters.

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;
}
}
else if (PyRecord_Check(obj)) {
if (!PyObject_AsVARIANTRecordInfo(obj, var))
Expand Down Expand Up @@ -554,6 +569,9 @@ static BOOL PyCom_SAFEARRAYFromPyObjectBuildDimension(PyObject *obj, SAFEARRAY *
helper.m_reqdType = vt;
ok = helper.MakeObjToVariant(item, &element);
switch (vt) {
case VT_RECORD:
pvData = V_RECORD(&element);
break;
case VT_DISPATCH:
pvData = V_DISPATCH(&element);
break;
Expand Down Expand Up @@ -759,7 +777,14 @@ static BOOL PyCom_SAFEARRAYFromPyObjectEx(PyObject *obj, SAFEARRAY **ppSA, bool

if (bAllocNewArray) {
// OK - Finally can create the array...
*ppSA = SafeArrayCreate(vt, cDims, pBounds);
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);
}
else
*ppSA = SafeArrayCreate(vt, cDims, pBounds);
if (*ppSA == NULL) {
delete[] pBounds;
PyErr_SetString(PyExc_MemoryError, "CreatingSafeArray");
Expand Down
14 changes: 14 additions & 0 deletions com/win32com/test/testPyComTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,7 @@ def TestDynamic():

def TestGenerated():
# Create an instance of the server.
from win32com.client import Record
from win32com.client.gencache import EnsureDispatch

o = EnsureDispatch("PyCOMTest.PyCOMTest")
Expand All @@ -433,6 +434,19 @@ def TestGenerated():
coclass = GetClass("{B88DD310-BAE8-11D0-AE86-76F2C1000000}")()
TestCounter(coclass, True)

# Test records with SAFEARRAY(VT_RECORD) fields.
progress("Testing records with SAFEARRAY(VT_RECORD) fields.")
l = []
for i in range(3):
rec = Record("TestStruct1", o)
rec.str_value = "This is record number"
rec.int_value = i + 1
l.append(rec)
test_rec = Record("TestStruct2", o)
test_rec.array_of_records = l
test_rec.rec_count = i + 1
assert o.VerifyArrayOfStructs(test_rec)

# XXX - this is failing in dynamic tests, but should work fine.
i1, i2 = o.GetMultipleInterfaces()
# Yay - is now an instance returned!
Expand Down
Loading