Is this CComSafeArray usage bad?

Question

I have a COM function:

GetData(SAFEARRAY ** pRetVal)

and following legacy code:

CComSafeArray<double> saDataArray;
hr = pmyInterface->GetData(&saDataArray.m_psa);
SafeArrayLock(saDataArray);

I doubt if that is good to manually manage locks. Will that code crash when m_psa was returned as NULL by GetData?

How about following code? Is that better?

LPSAFEARRAY psa;
CComSafeArray<double> saDataArray;
hr = pmyInterface->GetData(&psa);
saDataArray.Attach(psa);

EDIT: I tested both of the code above. There is one difference. If GetData returns NULL, directly Attach it without NULL check will invoke a exception. And the first version will return a E_INVALIDARG. My question still remains, do you prefer the later version since it use SafeArray object to maintain the counting, rather than mixing it?

EDIT2: If for whatever reason I choose the first version, is that okay to ignore the E_INVALIDARG return value? Will that have any side effect when some code later use this saDataArray?

There is no `SafeArray` type in the ATL libs, or anywhere else in MS tools. You may have meant `LPSAFEARRAY`. Also, ATL::CComSafeArray provides a method to get the address of the internal pointer for out-val usage. [CComSafeArray::GetSafeArrayPtr()](http://msdn.microsoft.com/en-us/library/1s10dhw5(v=vs.71).aspx) — WhozCraig, Apr 22 '13 at 09:33
I don't know if it might be a possible duplicate of [this question](http://stackoverflow.com/questions/1778491/how-does-one-return-a-local-ccomsafearray-to-a-lpsafearray-output-parameter)... — Mr.C64, Apr 22 '13 at 09:41
I've edited your question just to better format code inside question text. — Mr.C64, Apr 22 '13 at 10:04
You'd be interested in handling `hr` errors as soon as they appear. If you skip the first one, you are likely to get `E_INVALIDARG` on the next API call to `SafeArrayLock`. — Roman R., Apr 22 '13 at 10:06

score 2 · Accepted Answer · edited May 23 '17 at 11:57

You wrote:

SafeArray * psa;
CComSafeArray<double> saDataArray;
hr = pmyInterface->GetData(&psa);
saDataArray.Attach(psa);

But I think the actual code should be:

LPSAFEARRAY psa; // not "SafeArray *"
hr = pmyInterface->GetData(&psa);
CComSafeArray<double> saDataArray;
saDataArray.Attach(psa);

See this question for further details.

EDIT: Updating answer based on your question edits.

I really don't like your first code:

CComSafeArray<double> saDataArray;
hr = pmyInterface->GetData(&saDataArray.m_psa);
SafeArrayLock(saDataArray); // <--- Explicit lock on a CComSafeArray-wrapped array

In fact, once the raw SAFEARRAY is given to a C++ RAII wrapper (CComSafeArray), from that point in time on I would only use this wrapper and its methods to manipulate the array.
If you want to do "manual" handling of the array, just .Detach() it from the C++ wrapper, and use Win32 API function calls. But mixing both is not good quality code, IMO.

Note that the second approach is different, since you first use the raw SAFEARRAY, make the GetData() method fill it, and then you .Attach() it to the CComSafeArray C++ RAII wrapper, transferring ownership ("move semantics") to that wrapper. Then it's OK to use the wrapper methods to manipulate the array.

In addition, in production quality code, I would not ignore error HRESULTs.

That's right, it is LPSAFEARRAY. Seems majority prefer using CComSafeArray to handle the lock. But my question remains, is it okay to ignore the E_INVALIDARG? — Archer, Apr 22 '13 at 09:49
No, if you get E_INVALIDARG, from Attach(), that's because the author of that function considers it part of its contract that its input parameter be non-null. So you should guard your call to attach with a null pointer check rather than calling and ignoring the result.This will be safer (as you won't be ignoring catch other potential error codes), and more readable anwyay. — Jay Carlton, Apr 08 '14 at 21:11

Is this CComSafeArray usage bad?

1 Answers1