Freeing memory on the heap. Should I and how?

Question

I'm writing a CESetup.dll for a Windows Mobile app. It must be unmanaged, which I have little experience with. So I'm unsure of whether I should free the memory I allocate and how I do it.

Here's the function I've written:

    Uninstall_Init(
    HWND        hwndParent,
    LPCTSTR     pszInstallDir
)
{
    LPTSTR folderPath = new TCHAR[256];
    _stprintf(folderPath, _T("%s\\cache"), pszInstallDir);
    EmptyDirectory(folderPath);
    RemoveDirectory(folderPath);

    _stprintf(folderPath, _T("%s\\mobileadmin.dat"), pszInstallDir);
    DeleteFile(folderPath);

// To continue uninstallation, return codeUNINSTALL_INIT_CONTINUE
// If you want to cancel installation,
// return codeUNINSTALL_INIT_CANCEL
return codeUNINSTALL_INIT_CONTINUE;
}

As I understand it, folderPath is allocated on the heap. EmptyDirectory() is my own function that removes all content in the directory. RemoveDirectory() and DeleteFile() are system calls.

My question is should I deallocate folderPath before the function exits? If I should, how do I do it?

Answer 1

I think you want to use this:

delete [] folderPath;

It looks like you're allocating an array of TCHARs, which makes sense since it's a string. When you allocate an array, you must delete using the array delete operator (which you get by including the brackets in the delete statement). I'm pretty sure you'll get a memory leak with Treb's solution.

Answer 2

There is a common misperception I have seen with people who are unused to C/C++ programming - when they see a function with a pointer parameter, they think the variable must be allocated with new. That is not the case, a local variable is suitable and preferred, since you don't have to worry about its lifetime.

You could simplify your life tremendously by doing

TCHAR folderPath[256];

My preferred solution would be to use std::string, but I've put that in a separate answer.

Answer 3

Yes, you should free the memory. None of the functions you call will do it for you, and nor should they - it wouldn't make any sense. The memory was allocated with the vector new operator and so should be freed with the vector delete operator, i.e.:

delete [] folderPath;

Answer 4

It is generally better to use std::string, or in your case std::basic_string. In this case it eliminates a potential buffer overflow when your final path is greater than 256 characters.

    Uninstall_Init(
    HWND        hwndParent,
    LPCTSTR     pszInstallDir
)
{
    std::basic_string<TCHAR> folderPath = pszInstallDir;
    folderPath.append(_T("\\cache"));
    EmptyDirectory(folderPath.c_str());
    RemoveDirectory(folderPath.c_str());
    folderPath = pszInstallDir;
    folderPath.append(_T("\\mobileadmin.dat"));
    DeleteFile(folderPath.c_str());
// To continue uninstallation, return codeUNINSTALL_INIT_CONTINUE
// If you want to cancel installation,
// return codeUNINSTALL_INIT_CANCEL
return codeUNINSTALL_INIT_CONTINUE;
}

Answer 5

Yes, you should. By calling

 delete[] folderPath;

at the end of your function. All memory assigned with new must be freed with delete.