SysFreeString() WHEN???

We've a discussion and a potential problem:

We've a server that is a client to a DCOM component. The server has a GetName([out] BSTR * bname) method:

We've been calling with the following:

BSTR bname = NULL;
IDbSrv * pdb;
CoCreateInstanceEx( GUID, &pdb); ...

pdb->GetName( &bname );

SysFreeString( bname); // Is this required???

We're worried about global leaks if not freeing the string.

in IDL:

Library ...

class IDbSrv
{
HRESULT GetName( [out] BSTR * bname );
}



in the server

CIDbSrv::GetName( BSTR * bname )
{
CComVariant var;
CString name;
HRESULT hr = S_FALSE;
if ( protecteddb.GetName( name ) )
{
var = name;
*bname = var.bstrVal;
hr = S_OK;
}
return hr;

}


One says that BSTR is NOT necessary as C++ will clean it up, another says it MUST be SysFree'd.

Accurate input or guidance with this POORLY documented BSTR would be appreciated to prevent LEAKING and double frees.

My understanding is that you should free it. The system couldn't know when you have finished with it, and BSTRs are simple beasties without destructors. Somewhere in MSDN there's a list of rules on this (but unfortunately I've forgotten where). There are also example in Don Box's "Essential Com" book.

Also, I think you could simplify the server side to:

CIDbSrv::GetName( BSTR * bname )
{
CString name;
HRESULT hr = S_FALSE;
if ( protecteddb.GetName( name ) )
{
*bname = name.AllocSysString();
hr = S_OK;
}
return hr;
}

Not that it makes that much difference...


Roger

...And I guess you either ought to not free it in the "not found case" or return a blank/null string from the server function for it to free.

Here are the rules that we follow for using BSTRs:

1) It is the callee's responsibility to SysAlloc() a new copy if they want to store it internally
2) It is the callee's responsibility to SysAlloc() to return a copy of an internally stored BSTR
3) It is the caller's responsibility to SysFree() a BSTR that was returned from a call
4) Don't have BSTR parameters be [in, out] (This really gets confusing) only declare them [in] or [out, retval]

To make this easier, we use the _bstr_t smart BSTR class - It will automatically SysFree() when it goes out of scope, and SysAlloc()'s and SysFree()'s during assignment if necessary. Here is some samples.

Assume the following in the ATL server class:
private:
bstr_t mbstrtInternal1;
bstr_t mbstrtInternal2;

Also assume 2 methods
PassIn - takes a BSTR and stores it
PassOut - Returns an internal BSTR

// Server code
PassIn(BSTR bstrVal)
{
// Make sure something passed in (Optional)
if (bstrVal == NULL || SysStringLen(bstrVal) == 0) {
return E_INVALIDARG;
}

// Make copy of passed in BSTR (bstr_t autmatically does this)
mbstrtInternal1 = bstrVal;

return S_OK;
}

PassOut(BSTR *pbstrVal)
{
// Validate that a pointer was passed in
if (pbstrVal == NULL) {
return E_POINTER;
}

*pbstrVal = mbstrtInternal2.copy();
}

// Client Code

...
bstr_t bstrtString;
BSTR bstrOut;
IObj *pObj;
HRESULT hr;

CoCreateInstance(..., &pObj);

bstrtString = L"Chad Yost";

hr = pObj->PassIn(bstrtString);
hr = pObj->PassOut(&bstrOut);
bstrtString = bstr_t(bstrOut, false);

...

Just a little warning about using the _bstr_t class. A couple of the programmers that I work with have found leaks within that class. If you need more info, I can check when I'm back at the office.