CODEWITHSUNDEEP
c++memory-managementnew-operator
Grub
Grub

Reputation: 883

Using 'new' to allocate memory dynamically in C++?

I am working on some C++ code and am having some problems with the function described below. I haven't used much C++ before, at least not for a long time and so i'm trying to learn as I go along to a large extent. The win32api doesn't help much with the confusion factor either...

The function is succesfully called twice, before failing when called at a later stage when it is called in the application.

PTSTR getDomainFromDN(PTSTR dnPtstr) {

size_t nDn=wcslen(dnPtstr);
size_t *pnNumCharConverted = new size_t;

wchar_t *szTemp = new wchar_t[10];          // for debugging purposes
_itow_s((int)nDn,szTemp,10,10);             // for debugging purposes

AddToMessageLog(EVENTLOG_ERROR_TYPE,szTemp);        // for debugging purposes (displays an integer value before failing)
AddToMessageLog(EVENTLOG_ERROR_TYPE,TEXT("Marker A"));  // for debugging purposes
char *dn = new char[nDn];
    // !!!!!!!!!!!! all goes wrong here, doesn't get to next line, nDn does have a value when it fails (61)
AddToMessageLog(EVENTLOG_ERROR_TYPE,TEXT("Marker B"));  // for debugging purposes

wcstombs_s(pnNumCharConverted,dn,nDn+1,dnPtstr,nDn+1);

...more code here...

delete[] dn;
delete pnNumCharConverted;

return result
}

At first i thought it was a memory allocation problem or something as it fails on the line char *dn = new char[nDn];, the last marker showing as 'Marker A'. I used delete[] on the pointer further down to no avail. I know that nDn is a value because I print this out to a message log using _itow_s for debugging. I also know that dnPtrstr is a PTSTR.

I tried using malloc as well with free() in the old C style but this doesn't improve things.

Upvotes: 0

Views: 1594

Answers (3)

Stack Overflow is garbage
Stack Overflow is garbage

Reputation: 247909

I tried sanitizing your code a bit. One of the big tricks to C++ is to not explicitly use memory management when it can be avoided. Use vectors instead of raw arrays. Strings instead of char pointers.

And don't unnecessarily allocate objects dynamically. Put them on the stack, where they're automatically freed.

And, as in every other language, initialize your variables.

PTSTR getDomainFromDN(PTSTR dnPtstr) {
    std::wstring someUnknownString = dnPtstr;

    size_t numCharConverted = 0;

    std::wstring temp; // for debugging purposes
    std::ostringstream sstr;
    sstr << temp;
    AddToMessageLog(EVENTLOG_ERROR_TYPE,sstr.str().c_str());        // for debugging purposes (displays an integer value before failing)

    AddToMessageLog(EVENTLOG_ERROR_TYPE,TEXT("Marker A"));  // for debugging purposes
    std::vector<char> dn(someUnknownString.size());

    AddToMessageLog(EVENTLOG_ERROR_TYPE,TEXT("Marker B"));  // for debugging purposes

    wcstombs_s(&numCharConverted, &dn[0], dn.size(), someUnknownString.c_str(), dn.size());

    ...more code here...

    return result
}

This might not have solved your problem, but it has eliminated a large number of potential errors. Given that I can't reproduce your problem from the code you've supplied, this is really the best I can do.

Now, if you could come up with sane names instead of dnPtstr and dn, it might actually be nearly readable. ;)

Upvotes: 2

bayCoder
bayCoder

Reputation: 1355

If you are sure that "char *dn = new char[nDn];" is failing, TRY "set_new_handler" -> http://msdn.microsoft.com/en-us/library/5fath9te(VS.80).aspx

On a side note, few things:

  1. The very first operation "size_t nDn=wcslen(dnPtstr);" is not 100% correct. You are doing wcslen on dnPtstr assuming dnPtstr to be unicode. However, this is not the case since it could be PWSTR or PSTR based on whether UNICODE is defined or not. So, use _tcslen(). Its better if you give some time to understand UNICODE, NON-UNICODE stuff since they would help you a lot in Windows C++ development.
  2. Why are you using so many "new" if you are using these variables only in this function (I am assuming it). Prefer stack over heap for local variables unles you have a definite requirement.

Upvotes: 0

Vusak
Vusak

Reputation: 1470

i think your problem is this line:

wcstombs_s(pnNumCharConverted,dn,nDn+1,dnPtstr,nDn+1);

because you are telling wcstombs_s to copy up to nDn+1 characters into dn which is only nDn characters long.

try changing the line to:

wcstombs_s(pnNumCharConverted,dn,nDn,dnPtstr,nDn);

or perhaps better yet:

wcstombs_s(pnNumCharConverted,dn,nDn,dnPtstr,_TRUNCATE);

im not sure how you are debugging this or how AddToMessageLog is implemented, but if you are just inspecting the log to trace the code, and AddToMessageLog is buffering your logging, then perhaps the error occurs before that buffer is flushed.

Upvotes: 1

Related Questions