views:

190

answers:

1

I have a small memory leak in my code regarding Strings in CLI/C++. I've tried to fix the leak by deleting my unsigned char array, but when I do, I get a Memory Access Violation.

I assume this is because the System::String is a ref type, and because of that, the memory is associated with both 'testString' and 'uch' in the code below. Is that correct? If so, how can I separate the memory so that I can free up the memory and still return a System::String?

myStatus get_testString(String^% testString)
{
    uchar* uch = 0;
    bool b = MgdStringToUChar(testString, uch);
    myStatus s = m_NativeMsg->get_testString(uch);
    testString = (reinterpret_cast<const char*>(uch));

    delete []uch;//this line causes an error       
    uch=0; 
    return s;
}

static bool MgdStringToUChar(System::String^ s, uchar*& uch)
{
    pin_ptr<const wchar_t> wch = PtrToStringChars( s ); 
    int len = (( s->Length+1) * 2); 
    size_t * st = 0; 
    char *ch = new char[ len ]; 
    bool result = wcstombs_s(st,ch, len, wch, len ) != -1; 
    if(!result) 
        throw (gcnew Exception("Could not parse string in :: MgdStringToUChar")); 
    uch = new uchar[len]; 
    int i=0; 
    while(i<len+1) 
    { 
        uch[i] = ch[i]; 
        i++; 
    } 
    delete st; 
    st=0; 
    delete [] ch; 
    ch=0; 
    return true; 
};
+1  A: 

Ok, for a start your MgdStringToUChar function has some problems. The first parameter to wcstombs_s should be a pointer to a real size_t not a pointer to null. If you want to ensure that ch is null terminated then you should use _TRUNCATE or len-1 as the count parameter too.

The copy from the ch buffer to the resultant uchar buffer was also probably going over the end of the buffer.

static bool MgdStringToUChar(System::String^ s, uchar*& uch)
{
    pin_ptr<const wchar_t> wch = PtrToStringChars( s ); 
    int len = (( s->Length+1) * 2); 
    size_t st = 0;
    char *ch = new char[ len ]; 

    bool result = wcstombs_s(&st, ch, len, wch, _TRUNCATE) != -1; 
    if(!result) 
        throw (gcnew Exception("Could not parse string in :: MgdStringToUChar")); 

    uch = new uchar[st+1]; 
    uch[st] = NULL;

    for(int i = 0; i < st; i++) 
    { 
        uch[i] = ch[i]; 
    } 

    delete [] ch; 

    return true; 
};

You could also do this more simply like this given that you seem to need the basic ANSI conversion:

static bool MgdStringToUChar(System::String^ s, uchar*& uch)
{
    char* ch = (char*)(void*)Marshal::StringToHGlobalAnsi(str);

    size_t st = strlen(ch);
    uch = new uchar[st + 1]; 
    uch[st] = NULL;

    for(int i = 0; i < st; i++) 
    { 
        uch[i] = ch[i]; 
    } 

    Marshal::FreeHGlobal(ch);

    return true; 
};

This should work a bit better. However I'm still concerned about what get_testString is doing to the data in uch. Without seeing the full picture it's difficult to see what's going on here.

Simon Steele
Yikes, that was awful dumb of me! I copied that routine from my char* method without thinking unsigned would not need as much space. Thanks for the optimization, however, I am still getting the error on the same line. Is it possible that that de-allocation is failing because it doesn't know the size of the array since that's set in the function and not in the code block where it's de-allocated?
TomO
RE: get_testString(); The trouble is, I am calling methods described in .h and .lib files. I only have the signature, so I really don't know what happens in the native code. I only know what I need at the wrapper level (my C++/CLI code).
TomO
So as it turns out, you were correct in that we don't know what's going on in the native method. Because of that, I found a const in one of the header files I am going to use to allocate space for a locally declared unsigned char array and avoid de-allocating the pointer altogether. I'll just let it scope out after the function call ends.
TomO