Is garbage initial value okay for `[out] BSTR*` parameter?

Question

According to MSDN:

For [out] parameters, the method or property allocates the memory and the caller is responsible for freeing the memory.

Which of these are okay:

[...]
STDMETHOD(Method)([out] BSTR* psValue)
[...]

BSTR myBStr1;
Method(&myBStr1);
::SysFreeString(myBStr1);

BSTR myBStr2 = SysAllocString(L"MyStringValue");
Method(&myBStr2);
::SysFreeString(myBStr2);

BSTR myBStr3 = NULL;
Method(&myBStr3);
::SysFreeString(myBStr3);

Answer 1

Yes, garbage is acceptable. The method is responsible to initialize the value. The caller is only responsible for freeing.

MSDN on out attribute:

An [out]-only parameter is assumed to be undefined when the remote procedure is called and memory for the object is allocated by the server.

#1, #3 are OK. #2 is going to create a memory leak.

This rule is actually creating some confusion. If a method returns an error, you might be not sure whether the values are already initialized or not. If you attempt to release garbage, it is going to give an access violation or corrupt memory. If you skip memory freeing and partially successful method left something meaningful there (well, it is the problem on server side but still), then you leak memory. It is certainly safer to initialize with NULL before the call, and CComBSTR constructor, for instance, would do it for you. On server side, you might want to start your method with NULL initialization of [out] values to not accidentally leave them uninitialized later.

Answer 2

Look at the rule from the perspective of a marshalled call (that is, what happens when the object lives in a different COM apartment, or when it lives in a different process or machine); you will understand how the rule works, and why it is the only sensible rule there can be.

The key fact is that for an [out] parameter, the marshaller will not marshal the initial value to the callee.

That means that it doesn't matter what you put in the variable before making the call. It can be garbage or NULL. Nobody is going to look at it.

Note: It also means that if the variable contains before the call something that needs to be released, you have to release it yourself before the call, or it will be leaked. The marshaller is not going to release it for you (because being the parameter [out] the marshaller must assume it's garbage) and the callee couldn't release it even if it wanted (because it will never get the existing value from the mashaller).

Second, after the call returns, the callee has no way to know what you do with the return value, so the only one that can release the parameter, is the caller.

The situation with error conditions is a bit dicier.

In principle, COM requires that an [out] parameter must be set to a consistent ("marshallable"?) value before returning (either NULL or equivalent, or a valid value), even in the presence of an error. This is also "the only sensible rule": the marshaller has no way to know that the callee is in an error situation. It's also not going to guess based on the HRESULT, because even an "error class" HRESULT doesn't mean the method doesn't intend to return values to the caller. The marshaller is going to marshal the value of the parameter back to the caller - error or no error - so the parameter better be valid, or the marshaller itself will crash.

In practice... well, some objects are better written than others, and some COM classes are never called via marshalling so the developers of the class never catch the problem. A class that doesn't comply with these rules can not be safely used across apartments.

The canonical way to make sure the parameter's values stay consistent even in the face of an error situation, looks something like this:

STDMETHODIMP Class::Method(BSTR *pValue)
{
  *pValue = NULL;        // 1) Initialize all the [out] parameters before
                         //    doing anything, to make sure they are always
                         //    consistent

  BSTR tempValue;        // 2) Use a temporary to hold any result value

  ...                    // 3) Use the temporary for all computations
  ...                    //    Finish all other processing

  *pValue = tempValue;   // 4) Only after all possible error conditions
                         //    are behind us, do put the new value
                         //    in the output parameter

  return S_OK;
}

Obviously, if an out parameter is just a value that doesn't need releasing (like an int, etc.) then none of this matters.