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?

Answer 1

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.