Pergunta

I'm using VS2012. I'd rather be allocating memory for CString off of the heap, so given the code below:

  1. Is CString csToken allocating memory from the stack or heap?
  2. Do I need to free memory that csToken is using, or will it automatically be freed at the function termination?

    TCHAR *sAllCodes = (TCHAR *) calloc(50000,sizeof(TCHAR));       // Dynamically use Heap memory to hold all pipe-delimited codes
    TCHAR *sCode = (TCHAR *) calloc(128,sizeof(TCHAR)); // Dynamically use Heap memory to hold a single code
    DWORD i = 0;
    
    LoadAllCodes(&sAllCodes);       // Custom function
    
    CString csToken;        // Declare the CString variable
    csToken.Append(sAllCodes);  // Assign the string to the Cstring variable
    vector<CString> vAllCodes;  // Declare the vector variable
    vAllCodes = Split(csToken,L"|");        // Split the CString into a vector array
    
    while ( i < (DWORD) vAllCodes.size())
        {
        if (vAllCodes[i].StringLength() > 0)        // If there is a string in the vector item
            {
            _stprintf_s(sCode,128,L"%s",vAllCodes[i].GetString());      // Get the vector item and copy it into a string
    
            // Do work on sCode here...
            }
        i++;
        }
    
    free(sAllCodes);sAllCodes=NULL;
    free(sCode);sCode=NULL;
    
Foi útil?

Solução

Your csToken variable is an instance of CString allocated on the stack, so you don't need to do anything to delete it: its destructor will do proper string cleanup when the scope of this variable terminates.

However, CString internally maintains a reference to the actual string, that is allocated on the heap.
CString actually uses a "COW" technique and reference counting, so several CString instances can share the same string, they can reference to the same string. However, these are CString implementation details.

The "take away" for a basic usage of CString is that if you have CString str you don't have to pay attention to str cleanup: it will be automatically managed by CString destructor.


Moreover, I'd like to take this occasion to make a few notes on your code, with the spirit of improving it.

Use std::vector

You have these allocations at the top of your posted code:

TCHAR *sAllCodes = (TCHAR *) calloc(50000,sizeof(TCHAR));
TCHAR *sCode = (TCHAR *) calloc(128,sizeof(TCHAR));

and then you have the corresponding free calls at the bottom:

free(sAllCodes);sAllCodes=NULL;
free(sCode);sCode=NULL;

Note however that this code is not exception safe: in fact, if any code between the calloc() and the free() calls happens to throw a C++ exception, the memory allocated on the heap for sAllCodes and sCode will not be released.

In a more modern and more practical C++ style, you could use std::vector instead of calloc() to allocate the memory on the heap, and automatically free it, thanks to std::vector destructor (just like for CString!).

Just substitute the calloc() calls with:

std::vector<TCHAR> allCodes(50000);
std::vector<TCHAR> code(128);

and just delete the free() calls! :)
Besides being simpler and shorter, your code becomes also exception-safe; in fact, if a C++ exception is thrown, vector destructor is automatically called, and the allocated memory is released.

If you want to access vector data, you can use its data() member function.

If you want to set the initial vector content to all 0s, you can use the vector v(count, 0) constructor overload.

Use C++-style casts

In your code you have a C-style (DWORD) cast. C-style casts are bad; consider using C++-style casts instead. In your case, you might want a static_cast<DWORD>(...).

Use CString::IsEmpty() to improve readability

You have an if in your code like this:

if (vAllCodes[i].StringLength() > 0)

CString offers a convenient IsEmpty() method, that is more readable:

if (! vAllCodes[i].IsEmpty()) 
    ....

Notes on _stprintf_s() and "magic numbers"

You use _stprintf_s() like this:

_stprintf_s(sCode,128,L"%s",vAllCodes[i].GetString()); 

But note that if you use the TCHAR model, for coherence you should use _T("%s") in the format specifier.
If you want to just use Unicode strings and L"%s" (which I suggest), then use the corresponding Unicode-only swprintf_s(), and get rid of TCHAR and just use wchar_t instead.

Note also that, instead of using the "magic number" 128, you may want to use sCode.size() (assuming sCode becomes an instance of std::vector, as I suggested above). So, if you change the size of the vector, you don't have to update also the "magic number" 128 (with these magic numbers, there are basically bugs waiting to happen :)
The updated code might be something like:

swprintf_s(code.data(), code.size(), L"%s", vAllCodes[i].GetString()); 

Outras dicas

  1. CString csToken variable is allocated on the stack, but once some string is assigned to it, it allocates its internal buffers on the heap.

  2. You don't need to release any memory occupied by csToken explicity, it will be released once csToken variable goes out of scope, and its destructor is called.

Anything that is allocated on the stack will be freed once a function goes out of scope. Anything on the heap must be explicitly freed.

csToken hasn't been allocated with the new keyword, so it is on the stack, rather than the heap.

Finally, I see that you are using C++, and it is very taboo to use free() and malloc() in C++. You should use the new and delete keywords.

EDIT:

This line can be rewritten:

TCHAR *sAllCodes = (TCHAR *) calloc(50000,sizeof(TCHAR));

as:

TCHAR *sAllCodes = new TCHAR(5000);

This is how to use the new keyword.

Licenciado em: CC-BY-SA com atribuição
Não afiliado a StackOverflow
scroll top