COleVariant and strings

[wlof]

Hello,

I have a class which encapsulates a COleVariant. Here's a brief outlook of the class:

CEncaps::CEncaps() :
    m_pVariant(new COleVariant())
{
}

CEncaps::~CEncaps(void)
{
    if (m_pVariant)
    {
        delete m_pVariant;
        m_pVariant = NULL;
    }
}

BOOLEAN CEncaps::Read(CFile& file)
{
    BOOLEAN bOK = TRUE;

    switch (m_enumType)
    {
    case TYPE_BYTE:
        {
            m_pVariant->vt = VT_UI1;
            file.Read(&(m_pVariant->bVal), sizeof(BYTE));
        }
        break;
    // ...
    // All other numeric types are handled similarly to bytes
    // ...
    case TYPE_STRING:
        {
            char* pBuf = new char[m_nSize + 1];
            file.Read(pBuf, m_nSize);
            pBuf[m_nSize] = '\0';
            m_pVariant->SetString(pBuf, VT_BSTR);
        }
        break;
    }

    return bOK;
}

BOOLEAN CEncaps::Write(CFile& file)
{
    BOOLEAN bOK = TRUE;

    switch (m_enumType)
    {
    case TYPE_BYTE:
        {
            file.Write(&(m_pVariant->bVal), sizeof(BYTE));
        }
        break;
    // ...
    // All other numeric types are handled similarly to bytes
    // ...
    case TYPE_STRING:
        {
            BSTR bstr = m_pVariant->bstrVal;
            CString cstr(bstr);
            LPTSTR pBuf = cstr.GetBufferSetLength(m_nSize);
            file.Write(pBuf, m_nSize);
        }
        break;
    default:
        break;
    }

    return bOK;
}

(m_enumType and m_nSize are set by other methods not shown here for the sake of clarity.)

Now, my problem: this code works but produces memory leaks. I'm almost certain the leaks are caused only by strings. I find the BSTR type very confusing! I think I'm supposed to call SysFreeString() at some point, but my tries have been unsuccessful (produce error in the destructor).

So now I'm asking you code gurus I'd be really grateful if someone can help me correct this leak. Thanks in advance!

[VictorN]

1. I wonder why do you create the COleVariant object of your class (m_pVariant) in the heap?
Creating it on the stack would work pretty good too!

2. I see at least one place where you create the char array in the heap and do NOT delete it at all! it is in CEncaps::Read method:

    case TYPE_STRING:
        {
            char* pBuf = new char[m_nSize + 1];
            file.Read(pBuf, m_nSize);
            pBuf[m_nSize] = '\0';
            m_pVariant->SetString(pBuf, VT_BSTR);
        }
        break;

Why do you use char* type? What is wrong in using CString in this place?

[wlof]

1. I wonder why do you create the COleVariant object of your class (m_pVariant) in the heap?
Creating it on the stack would work pretty good too!

Out of habit, I guess. I usually use pointers. I tried changing it to use the stack, but this produces an error box at runtime which simply says "Incorrect variable type." :/

2. I see at least one place where you create the char array in the heap and do NOT delete it at all! it is in CEncaps::Read method:
Why do you use char* type? What is wrong in using CString in this place?

CFile::Read() expects a pointer to a buffer, that's why I used a char* buffer. Should I use the pointer returned by CString::GetBuffer()?

Also, I did not delete the array because I'm unsure of what COleVariant::SetString() does. Does it actually copy the contents of the buffer, or simply the pointer?

Thanks again for your help, VictornN.

[VictorN]

Out of habit, I guess. I usually use pointers. I tried changing it to use the stack, but this produces an error box at runtime which simply says "Incorrect variable type." :/

It only means you are using it incorrect. Fix it!

CFile::Read() expects a pointer to a buffer, that's why I used a char* buffer. Should I use the pointer returned by CString::GetBuffer()?

Yes! And then don't forget to call CString::ReleaseBuffer before using this CString object.

Also, I did not delete the array ...

The why do you wonder you have memory leaks?

I'm unsure of what COleVariant::SetString() does. Does it actually copy the contents of the buffer, or simply the pointer?

What is wrong for you to read MSDN if you "unsure" yet?

COleVariant::SetString
void SetString( LPCTSTR lpszSrc, VARTYPE vtSrc );

Parameters

lpszSrc
A null-terminated string to be copied into the new COleVariant object.

vtSrc
The VARTYPE for the new COleVariant object.
...

[wlof]

It only means you are using it incorrect. Fix it!

I fixed it. That was actually an unrelated bug, nothing to do with heap or stack.

Yes! And then don't forget to call CString::ReleaseBuffer before using this CString object.

I've added a delete [] pBuf instruction to fix the leak, but I'll switch to CString anyway.

The why do you wonder you have memory leaks?

I didn't wonder I had memory leaks, I wondered at which point I was supposed to release the memory used to store the strings. If I understand correctly, VARIANT uses a BSTR to store strings, and I don't quite get how BSTR works...

Anyway, my bug is fixed. Thanks for your help!