Should out params be set even if COM function fails?

Question

When implementing a COM interface I always assign to the out parameters on success but should I do so also on error?

HRESULT CDemo::Div(/*[in]*/ LONG a, /*[in]*/LONG b, /*[out,retval]*/ LONG* pRet)
{
    if (pRet == NULL)
        return E_POINTER;

    if (b == 0)
    {
        *pRet = 0; // is this redundant?
        return E_INVALIDARG;
    }

    *pRet = a/b;
    return S_OK;
}

At one time I was bit on the nose by not initializing an out parameter and assuming that if I initialized the variable it will remain that value if I don't change it inside the method. However I used this method from .NET and since the marshaller sees that this is an [out] parameter it discarded the initial value I placed on the call site and put in garbage after the function returned (it was fun debugging that, not).

Is assigning to an out param even on failure overcompensation or should I really do it?

---

Edit: Even though formally one should not access out params if the function failed I often see (and sometimes write) code like this (using the example from sharptooth's post):

ISmth *pSmth = NULL; 
pObj->GetSmth(&pSmth); // HRES is ignored
if (pSmth) // Assumes that if GetSmth failed then pSmth is still NULL
{ 
    pSmth->Foo();
    pSmth->Release();
}

This works fine in un-marshalled code (same thread apartment) but if a marshaller is involved is it smart enough to only set the return value if the function succeeded?

Answer 1

The rule is that the calling party is not allowed to do anything with the out parameters value if the call fails. The server therefore should not provide valid values and should not pass ownership of any resources to the out parameters.

For example if you have

HRESULT GetSmth( [out] ISmth** );

method then it's expected that the server calls AddRef() on the ISmth** variable prior to returning. It must not call AddRef() if it is going to return a failure code because the client is not allowed to use the returned out parameter value and therefore will not call Release() and you'll get a memory leak.

Answer 2

I'm not sure I 100% agree with sharptooth. I certainly agree that for a failed COM call you cannot and must not assign any resource ownership to any out parameters. This includes memory allocation or AddRef'ing a COM object.

However I see nothing wrong (and in fact encourage) setting purely out parameters to empty values as long is does not transfer any resource ownership. For instance there is nothing technically illegal about your code setting pRet to point to 0. This transfers no resource ownership over to pRet and is merely a helper to some caller who did not properly check for success of the call.

Answer 3

While the other answers are not wrong, they miss a very important point -- a COM server that intends to return a failure HRESULT MUST set all [out] parameters to NULL. This is not merely a matter of good style, it is required by COM and not adhering to it can cause random crashes when there is marshaling involved.

That said, the *pRet = 0; in the original code is not redundant but correct and required.

Answer 4

This article explain more about callee/caller responsibility :
CORBA programming: Argument passing considerations for C++ bindings