[pywin32-bugs] [ pywin32-Bugs-1709340 ] ByRef 2D arrays of 1 Byte improperly handled

[SourceForge.net <no...@so...>]

Bugs item #1709340, was opened at 2007-04-29 04:26
Message generated for change (Comment added) made by mhammond
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=551954&aid=1709340&group_id=78018

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: win32
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: wyrock (wyrock)
Assigned to: Mark Hammond (mhammond)
Summary: ByRef 2D arrays of 1 Byte improperly handled

Initial Comment:
If a COM module passes a 2D array where one each dimension is only one byte, then win32com only returns a buffer. The buffer prevents access from the other dimension of the array. It is expected to get an array of buffers.

The bug lies in oleargs.cpp. I've attached an updated version of oleargs.cpp and also some modified versions of the com_testing code to show the problem.

Could someone please review the proposed fix and commit the change?

Thanks,


----------------------------------------------------------------------

>Comment By: Mark Hammond (mhammond)
Date: 2007-07-26 18:31

Message:
Logged In: YES 
user_id=14198
Originator: NO

I'm adding a new patch that doesn't attempt to return multiple buffers - I
couldn't make the old patch return the correct data, and
http://mail.python.org/pipermail/python-win32/2007-July/006096.html
convinces me that the right thing to do may well be to return a single
buffer in this case, so long as the caller can get the dimensions - that
way, if the data is destined for PIL etc, its very efficient, but still
simple for someone to slice up if they really do need it as a multi-dim
array.

The patch works, but I'm reluctant to check it in until we do return the
dimensions.  This might mean creating a new buffer-like object.
File Added: 2d_ui1.patch

----------------------------------------------------------------------

Comment By: wyrock (wyrock)
Date: 2007-07-23 21:59

Message:
Logged In: YES 
user_id=1681725
Originator: YES

Mark,

I was just curious if you had been able to review this change and add it
to the current source tree?

Thanks,
Kevin

----------------------------------------------------------------------

Comment By: Mark Hammond (mhammond)
Date: 2007-05-16 08:31

Message:
Logged In: YES 
user_id=14198
Originator: NO

Thanks - I'll look at this when I get time.  You can build an executable
by running:

  setup.py bdist_wininst

----------------------------------------------------------------------

Comment By: wyrock (wyrock)
Date: 2007-05-15 23:00

Message:
Logged In: YES 
user_id=1681725
Originator: YES

Now that I've built the source with the fix. I know i can do setup.py -q
install, but is there an existing script for making the win32 .exe that
gets distributed?

Also, the diff is fairly short, so I'll just post directly here:

? diff.out
? com/TestSources/PyCOMTest/PyCOMTest.plg
? com/win32com/src/oleargs.orig.cpp
? win32/src/_winxpthememodule.backup.cpp
Index: com/TestSources/PyCOMTest/PyCOMImpl.cpp
===================================================================
RCS file:
/cvsroot/pywin32/pywin32/com/TestSources/PyCOMTest/PyCOMImpl.cpp,v
retrieving revision 1.15
diff -r1.15 PyCOMImpl.cpp
312a313,347
> STDMETHODIMP CPyCOMTest::GetComplexVariantByRef(VARIANT *out)
> {
> 	VariantClear(out); // necessary?
> 	// Creating a safe array with two dimensions to make sure
> 	// an array of buffers can be returned
> 	SAFEARRAYBOUND sabBound[2]={ {5,0}, {8,0}};
> 	SAFEARRAY *psaDataBuffer = SafeArrayCreate(VT_UI1,2,sabBound);
> 	VariantInit(out);
> 	out->vt = VT_ARRAY | VT_UI1;
> 	out->parray = psaDataBuffer;
> 	long indicies[2] = {0,0};
> 	for(int i=0;i<5;i++){
> 		indicies[0]=i;
> 		for(int j=0;j<8;j++){
> 			indicies[1] = j;
> 			SafeArrayPutElement(psaDataBuffer,indicies,&j);
> 
> 		}
> 	}
> 	return S_OK;
> }
> 
> 
> STDMETHODIMP CPyCOMTest::SetComplexVariantByRef(VARIANT *v)
> {
> 
> 	if( v->parray->cDims == 2 && v->parray->rgsabound[1].cElements==5 &&
> 		v->parray->rgsabound[0].cElements==8){
> 		return S_OK;
> 	}
> 	return -1;
> }
> 
> 
> 
Index: com/TestSources/PyCOMTest/PyCOMImpl.h
===================================================================
RCS file:
/cvsroot/pywin32/pywin32/com/TestSources/PyCOMTest/PyCOMImpl.h,v
retrieving revision 1.14
diff -r1.14 PyCOMImpl.h
81a82,84
> 	STDMETHOD(GetComplexVariantByRef)(VARIANT *out);
> 	STDMETHOD(SetComplexVariantByRef)(VARIANT *v);
> 
Index: com/TestSources/PyCOMTest/PyCOMTest.dsp
===================================================================
RCS file:
/cvsroot/pywin32/pywin32/com/TestSources/PyCOMTest/PyCOMTest.idl,v
retrieving revision 1.15
diff -r1.15 PyCOMTest.idl
198a199,200
> 		HRESULT GetComplexVariantByRef([out, retval] VARIANT *vout);
> 		HRESULT SetComplexVariantByRef([out] VARIANT *v);
Index: com/win32com/src/oleargs.cpp
===================================================================
RCS file: /cvsroot/pywin32/pywin32/com/win32com/src/oleargs.cpp,v
retrieving revision 1.37
diff -r1.37 oleargs.cpp
398c398
< 	if (dimNo==nDims && vt==VT_UI1 && obj->ob_type->tp_as_buffer) {
---
> 	if (dimNo==nDims && obj->ob_type->tp_as_buffer) {
831c831
< 	if (vt==VT_UI1) {
---
> 	if (vt==VT_UI1 && dimNo==nDims) {

----------------------------------------------------------------------

Comment By: wyrock (wyrock)
Date: 2007-05-12 22:14

Message:
Logged In: YES 
user_id=1681725
Originator: YES

File Added: test2dByRef.py

----------------------------------------------------------------------

Comment By: wyrock (wyrock)
Date: 2007-05-12 22:13

Message:
Logged In: YES 
user_id=1681725
Originator: YES

Attaching the cvs diff with the fix (in oleargs.cpp) and some
modifications to the test bench that test for the issue.
File Added: diff.out

----------------------------------------------------------------------

Comment By: Mark Hammond (mhammond)
Date: 2007-05-06 21:00

Message:
Logged In: YES 
user_id=14198
Originator: NO

Thanks!  I haven't looked yet, but I can believe what you say is true. 
I'll (try to) get to this before the next release (but a .diff against the
test suite instead of the .zip with a complete copy would help even more!) 
If you have the c compiler handy, then 'setup.py install' from CVS is quite
an easy way to get a build from sources (but I digress... :)

----------------------------------------------------------------------

Comment By: wyrock (wyrock)
Date: 2007-05-06 08:10

Message:
Logged In: YES 
user_id=1681725
Originator: YES

File Added: oleargs.new.cpp

----------------------------------------------------------------------

Comment By: wyrock (wyrock)
Date: 2007-04-29 04:35

Message:
Logged In: YES 
user_id=1681725
Originator: YES

File Added: testSmaller.py

----------------------------------------------------------------------

Comment By: wyrock (wyrock)
Date: 2007-04-29 04:34

Message:
Logged In: YES 
user_id=1681725
Originator: YES

File Added: com_testing2.zip

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=551954&aid=1709340&group_id=78018