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());