modifying a string array via MFC COM Interop

A

Arnshea

(apologies for the crosspost)

I'm working with an MFC based COM object. From C# I'd like to be able
to call a method on the COM object that takes a string array and
modifies the contents.

Is there any way to do this with a variable length array?

I've only been able to get it to work with a fixed size array. The
relevant code snippets are below. Suggestions are greatly appreciated
(e.g., there's a better way to do it, the code will cause memory
leaks, etc...).

MFC DISPATCH MAP ENTRY:

DISP_FUNCTION_ID(CMyClass, "StrArrayFunc", dispidStrArrayFunc,
StrArrayFunc, VT_I4, VTS_VARIANT)


..H PROTOTYPE:

HRESULT StrArrayFunc(VARIANT &vArray);


..CPP:

HRESULT CMyClass::StrArrayFunc(VARIANT &vArray)
{
AFX_MANAGE_STATE(AfxGetStaticModuleState());

SAFEARRAY **ppsa = V_ARRAYREF(&vArray);

// ...
}


MANAGED/C# CLIENT:

static int Main(string[] args)
{
String[] ar = new string[30];

for (int i=0; i < ar.Length; i++)
ar = "str";

Array a = (Array) ar;

MyCOMServer.MyClassClass mc = new MyClassClass();

mc.StrArrayFunc(ref a); // INTEROP call

string[] nAr = (string[]) a;

// ... remaining code accesses modified array through nAr
}
 
G

Giovanni Dicanio

I'm working with an MFC based COM object. From C# I'd like to be able
to call a method on the COM object that takes a string array and
modifies the contents.

Is there any way to do this with a variable length array?

I've only been able to get it to work with a fixed size array.
Click to expand...

I don't know about C# side.

But I believe that - at least for COM and MFC side - if you use a SAFEARRAY
containing BSTRs, you should have no problem at all.

Have you tried to use your MFC COM object e.g. with Visual Basic 6? VB6
understands COM very well.
You could also try to test it from a MFC client, as well.

My suggestion is to first make it work correctly with native COM (so you may
test your COM object from a VB6 or MFC client), only after then, you may try
to fix the C# Interop side.

BTW: COM programming is *full* of lots of *details* (see other posts in this
MFC newsgroup, for some example... e.g. there was a bug with a VT_UI8 flag
used instead of correct VT_UI1, immersed in other well-working code), so
posting the C++/MFC code snippet you gave us is useless.

Just my 2 cents.

Giovanni
 
A

Arnshea

I don't know about C# side.

But I believe that - at least for COM and MFC side - if you use a SAFEARRAY
containing BSTRs, you should have no problem at all.

Have you tried to use your MFC COM object e.g. with Visual Basic 6? VB6
understands COM very well.
You could also try to test it from a MFC client, as well.

My suggestion is to first make it work correctly with native COM (so you may
test your COM object from a VB6 or MFC client), only after then, you may try
to fix the C# Interop side.

BTW: COM programming is *full* of lots of *details* (see other posts in this
MFC newsgroup, for some example... e.g. there was a bug with a VT_UI8 flag
used instead of correct VT_UI1, immersed in other well-working code), so
posting the C++/MFC code snippet you gave us is useless.

Just my 2 cents.

Giovanni
Click to expand...

Thanks, I appreciate it.

The full C++/MFC method is below. The method works (the input array
is modified and the modifications are visible from C#). I'm mainly
looking for ideas for improving this, e.g., alternative
implementation, style, potential pitfalls and the like.

HRESULT CMyClass::StrArrayFunc(VARIANT &vArray)
{
AFX_MANAGE_STATE(AfxGetStaticModuleState());

// make sure it's a string array
if (V_VT(&vArray) != (VT_BYREF | VT_ARRAY | VT_BSTR))
AfxThrowOleDispatchException(1001, "Type Mismatch in Parameter.
Pass a string array by reference");

// get the safearray from the variant
SAFEARRAY **ppsa = V_ARRAYREF(&vArray);

// get lower and upper bounds
long lLBound, lUBound;
SafeArrayGetLBound(*ppsa, 1, &lLBound);
SafeArrayGetUBound(*ppsa, 1, &lUBound);

// access each element
BSTR bstrCurrent;
BSTR bstrNew;
CString curStr;
for (long i=lLBound; i <= lUBound; i++)
{
SafeArrayGetElement(*ppsa, &i, &bstrCurrent);

SysFreeString(bstrCurrent);
bstrNew = SysAllocString(L"replaced");

HRESULT hr = SafeArrayPutElement(*ppsa, &i, bstrNew);
if (FAILED(hr))
goto error;
}

return S_OK;

error:
AfxThrowOleDispatchException(1003, "Unexpected Failure in
StrArrayFunc method");
return 0;
}
 
G

Giovanni Dicanio

The full C++/MFC method is below. The method works (the input array
is modified and the modifications are visible from C#).
Click to expand...

OK. So my understanding was wrong (I thought you had problems and bugs, e.g.
about array resizing...).
I apologize.
I'm mainly
looking for ideas for improving this, e.g., alternative
implementation, style, potential pitfalls and the like.
Click to expand...

OK. I put some comments in code below:
HRESULT CMyClass::StrArrayFunc(VARIANT &vArray)
{
AFX_MANAGE_STATE(AfxGetStaticModuleState());
Click to expand...

I would put an HRESULT variable, local to the method, e.g.

HRESULT hr;

So, you can assign it as the result of every function call you do in the
method, and if you get an error (FAILED(hr)), then you can return this 'hr'
value as function return value. So, the caller (client) can have better
information about the cause of the error condition.
e.g.

hr = SomeCOMCall(...);
if ( FAILED(hr) )
{
return hr;
}
// make sure it's a string array
if (V_VT(&vArray) != (VT_BYREF | VT_ARRAY | VT_BSTR))
AfxThrowOleDispatchException(1001, "Type Mismatch in Parameter.
Pass a string array by reference");
Click to expand...

When you use AfxThrowOleDispatchException, I would prefer not using a string
literal, but instead an integer reference into a string-table.

AfxThrowOleDispatchException (MFC):
http://msdn2.microsoft.com/en-us/library/hf2kbh8z(VS.80).aspx

You could use the following overload, which takes an integer instead of a
string:

void AFXAPI AfxThrowOleDispatchException(
WORD wCode,
UINT nDescriptionID,
UINT nHelpID = -1
);

Using an integer into a string table has several advantages, like making
localization easier (you can just change the string error description in the
string table, without modifying string literals in source code).
In general, I prefer storing strings in string table resources, and
reference the string via an integer resource ID, and not embedd string
literals in code.
// get the safearray from the variant
SAFEARRAY **ppsa = V_ARRAYREF(&vArray);
Click to expand...

You may want to check the pointer value here.

if ( ppsa == NULL )
... error: pointer not valid

(you may return an E_POINTER error as HRESULT).

Moreover, considering that you use "SAFEARRAY *" parameters, I would get rid
of double level of indirection (SAFEARRAY **), and just use one level of
indirection, i.e.:

SAFEARRAY * psa = *ppsa;
ASSERT( psa != NULL ); // ASSERT in debug builds only
if ( psa == NULL ) // Check for release builds
{
// signal errors in release builds
return E_POINTER; // or something similar
}

So, you can use 'psa' instead of '*ppsa' in following lines, e.g.:
// get lower and upper bounds
long lLBound, lUBound;
SafeArrayGetLBound(*ppsa, 1, &lLBound);
SafeArrayGetUBound(*ppsa, 1, &lUBound);
Click to expand...

becomes:

SafeArrayGetLBound( psa, 1, ... );
SafeArrayGetUBound( psa, ... );

// access each element
BSTR bstrCurrent;
BSTR bstrNew;
CString curStr;
for (long i=lLBound; i <= lUBound; i++)
{
SafeArrayGetElement(*ppsa, &i, &bstrCurrent);

SysFreeString(bstrCurrent);
bstrNew = SysAllocString(L"replaced");
Click to expand...

In general, I would use a BSTR class wrapper (like CComBSTR or _bstr_t)
instead of "raw" BSTR.
HRESULT hr = SafeArrayPutElement(*ppsa, &i, bstrNew);
Click to expand...

'hr' should be defined at the top of the method, so you can assign it from
whatever function call you make.

For example, also SafeArrayGetLBound and SafeArrayGetUBound return an
HRESULT. For robust code, I think you should check hr also in these cases.
So 'HRESULT hr' should be local to the whole method.

HRESULT hr; // <-- in the top of method

...

hr = SafeArrayGetLBound( psa, ... );
if ( FAILED(hr) )
{
...
return hr;
}

...

hr = ...
if ( FAILED(hr) )
...
error: ....
return 0;
Click to expand...

If you have an error, you should not return 0. You should return at least an
error code (generic) like E_FAIL. But you should return a more specific
error code, if possible.

Please consider all above as IMHO. I like freely expressing opinions (and
elaborate them), but I don't like when personal opinions become "religious"
wars or flames :)

Giovanni
 
W

Willy Denoyette [MVP]

Arnshea said:
(apologies for the crosspost)

I'm working with an MFC based COM object. From C# I'd like to be able
to call a method on the COM object that takes a string array and
modifies the contents.

Is there any way to do this with a variable length array?

I've only been able to get it to work with a fixed size array. The
relevant code snippets are below. Suggestions are greatly appreciated
(e.g., there's a better way to do it, the code will cause memory
leaks, etc...).

MFC DISPATCH MAP ENTRY:

DISP_FUNCTION_ID(CMyClass, "StrArrayFunc", dispidStrArrayFunc,
StrArrayFunc, VT_I4, VTS_VARIANT)


.H PROTOTYPE:

HRESULT StrArrayFunc(VARIANT &vArray);


.CPP:

HRESULT CMyClass::StrArrayFunc(VARIANT &vArray)
{
AFX_MANAGE_STATE(AfxGetStaticModuleState());

SAFEARRAY **ppsa = V_ARRAYREF(&vArray);

// ...
}


MANAGED/C# CLIENT:

static int Main(string[] args)
{
String[] ar = new string[30];

for (int i=0; i < ar.Length; i++)
ar = "str";

Array a = (Array) ar;

MyCOMServer.MyClassClass mc = new MyClassClass();

mc.StrArrayFunc(ref a); // INTEROP call

string[] nAr = (string[]) a;

// ... remaining code accesses modified array through nAr
}
Click to expand...



Not sure what you mean exactly with *variable* length array, but following
fills a List with a number of strings and passes it by ref to the COM
method. Of course what gets passed is a fixed length array (precise bounds).


List<string> stringList = new List<string>();
for(int i = 0; i < 5; i++)
stringList .Add("str" );
Array sar = stringList .ToArray();
MyCOMServer.MyClassClass mc = new MyClassClass();
mc.StrArrayFunc(sar);
....


IDL
..... HRESULT StrArrayFunc [in,out] SAFEARRAY(BSTR) vArray);


C++ implementation

..... StrArrayFunc(SAFEARRAY* vArray)
{
...

}

Willy.
 
A

Arnshea

OK. So my understanding was wrong (I thought you had problems and bugs, e.g.
about array resizing...).
I apologize.


OK. I put some comments in code below:


I would put an HRESULT variable, local to the method, e.g.

HRESULT hr;

So, you can assign it as the result of every function call you do in the
method, and if you get an error (FAILED(hr)), then you can return this 'hr'
value as function return value. So, the caller (client) can have better
information about the cause of the error condition.
e.g.

hr = SomeCOMCall(...);
if ( FAILED(hr) )
{
return hr;
}


When you use AfxThrowOleDispatchException, I would prefer not using a string
literal, but instead an integer reference into a string-table.

AfxThrowOleDispatchException (MFC):http://msdn2.microsoft.com/en-us/library/hf2kbh8z(VS.80).aspx

You could use the following overload, which takes an integer instead of a
string:

void AFXAPI AfxThrowOleDispatchException(
WORD wCode,
UINT nDescriptionID,
UINT nHelpID = -1
);

Using an integer into a string table has several advantages, like making
localization easier (you can just change the string error description in the
string table, without modifying string literals in source code).
In general, I prefer storing strings in string table resources, and
reference the string via an integer resource ID, and not embedd string
literals in code.


You may want to check the pointer value here.

if ( ppsa == NULL )
... error: pointer not valid

(you may return an E_POINTER error as HRESULT).

Moreover, considering that you use "SAFEARRAY *" parameters, I would get rid
of double level of indirection (SAFEARRAY **), and just use one level of
indirection, i.e.:

SAFEARRAY * psa = *ppsa;
ASSERT( psa != NULL ); // ASSERT in debug builds only
if ( psa == NULL ) // Check for release builds
{
// signal errors in release builds
return E_POINTER; // or something similar
}

So, you can use 'psa' instead of '*ppsa' in following lines, e.g.:


becomes:

SafeArrayGetLBound( psa, 1, ... );
SafeArrayGetUBound( psa, ... );



In general, I would use a BSTR class wrapper (like CComBSTR or _bstr_t)
instead of "raw" BSTR.


'hr' should be defined at the top of the method, so you can assign it from
whatever function call you make.

For example, also SafeArrayGetLBound and SafeArrayGetUBound return an
HRESULT. For robust code, I think you should check hr also in these cases.
So 'HRESULT hr' should be local to the whole method.

HRESULT hr; // <-- in the top of method

...

hr = SafeArrayGetLBound( psa, ... );
if ( FAILED(hr) )
{
...
return hr;
}

...

hr = ...
if ( FAILED(hr) )
...






If you have an error, you should not return 0. You should return at least an
error code (generic) like E_FAIL. But you should return a more specific
error code, if possible.

Please consider all above as IMHO. I like freely expressing opinions (and
elaborate them), but I don't like when personal opinions become "religious"
wars or flames :)

Giovanni
Click to expand...

Ahh, beautiful. Thanks a lot, I appreciate it!
 
G

Giovanni Dicanio

Ahh, beautiful. Thanks a lot, I appreciate it!
Click to expand...

You're welcome.
And I'm glad you appreciated.

Just a final note:

This could not be the case of the particular method you posted, but in other
contexts you may have some needs to do proper cleanup in case of error
(FAILED(hr)), and you may need to check error conditions several times, e.g.

HRESULT hr;

hr = CallSomething(...);
if ( FAILED(hr) )
{
...cleanup1
return hr;
}

...
hr = CallSomething2(...);
if ( FAILED(hr) )
{
...cleanup2
return hr;
}

etc.

This C++ code could be kind of boilerplate (lots of duplicated cleanup -
neither beautiful nor quality code, IMHO).
You may choose to jump using a "goto" statement at the end of the method, to
do a "centralized" cleanup there, e.g.

<code>

hr = CallSomething();
if ( FAILED(hr) )
goto Error;
...

hr = CallSomething2();
if ( FAILED(hr) )
goto Error;

...

return S_OK; // All right

//
// Cleanup already allocated resources, and return error code
//

Error:
// Centralized cleanup here
if ( resource1 is allocated )
release resource1;

if ( resource2 is allocated )
release resource2;
...

if ( resourceN is allocated )
release resourceN;

return hr;

</code>

To avoid that, I would prefer using what is called RAII (Resource
Acquisition Is Initialization) in C++, i.e. you can wrap your "dynamic"
resources in C++ classes, providing a destructor that does the cleanup job.
In this way, when the wrapper class goes out of scope (e.g. at the end of
the method body), before the "}", the C++ compiler calls the destructors, so
allocated resources are safely and elegantly cleaned up, because their own
destructors are called (the wrapper class instances are on the stack).
In this case, a "return hr;" could be just fine, because the compiler will
automatically call the destructors, and you need neither the duplicated
cleanup, nor the goto to "Error" label.

Giovanni
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads

C++ Client needs to fetch a string array from a C# COM server. 2

Top