// Now All Things are packed in to the Object
obj.Name = bstrName;
obj.Name2 = bstrname2;
I don't quite understand what do you mean by saying that things are packed since you're just copying pointers to the strings, and at the moment when you call SysFreeString obj.Name and obj.Name2 will point to an invalid block of memory. Although this code is not safe, it looks like if the source of your problem is class CFoo. You should show us more details of your code
I suggest you to use a CComBSTR class which will take a responsibility for releasing the memory.
UPDATE
#include <atlbase.h>
using namespace ATL;
...
{
CComBSTR bstrname(_T("Some Name"));
CComBSTR bstrname2(_T("Another Name"));
// Here one may work with these variables if needed
...
// Copy the local values to the Obj's member Variable
bstrname.Copy(&obj.Name);
bstrname2.Copy(&obj.Name2);
}
UPDATE2
First of all one should free bstrCity and bstrStreetName with SysFreeString or use CComBSTR instead within this block:
if (SUCCEEDED(hr))
{
BSTR bstrStreet = ::SysAllocString(table.m_pRecordData->Street);
pAddress->put_StreetName(bstrStreet);
BSTR bstrCity = ::SysAllocString(table.m_pRecordData->City);
pAddress->put_CityName(bstrCity);
// SysFreeString(bstrStreet)
// SysFreeString(bstrCity)
}
Consider to amplify the loop's condition !iRet with iCount < lRecCount.
for(...; !iRet /* && (iCount < lRecCount) */; ...)
Also, here:
m_sCityName = ::SysAllocString(bstrCityName);
you allocate memory but never release it since internally CComBSTR& operator = (OLESTR ..) allocates a new storage itself. One should rewrite is as follows:
m_sCityName = bstrCityName;
Everything else, looks good for me
UPDATE3
Well, Heap corruption is often a consequence of writing some values outside of the allocated memory block. Say you allocate an array of length 5 and put some value to the 6th position
Best Answer
You have to take care of all
BSTR
s except the one you actually pass as the "out" parameter. TheBSTR
you pass out need not be freed - the caller becomes responsible for freeing it, and your code is responsible for all otherBSTR
s it could have allocated.If you really need those temporary
BSTR
s you should use a wrapper class likeATL::CComBSTR
or_bstr_t
for those temporaryBSTR
s (but not for the one you pass out). I guess in the case you describe you'll be much better off just rewriting your code in such way that you don't need more that oneBSTR
creation on any control path.Here's some pseudocode: