constant String Memory Usage

Lost User

Environment: VC++ 6.0, Windows NT 4.0 I'm trying to write a app that talks to a database via Embedded SQL statements. The app is designed to run continuously, looping through a small amount of code. The Problem is that while generating the SQL statements, memory is leaked by Constant Assigned Strings. eg. CString fred = "fred"; The following code written to check memory usage: void CTestSQLStringDlg::OnButton1() { while(1) { CMemoryState m1, m2, mdiff; m1.Checkpoint(); fred(); m2.Checkpoint(); mdiff.Difference(m1, m2); mdiff.DumpStatistics(); TRACE("\n"); } } void CTestSQLStringDlg::fred(){ CString string ="hello"; } The Results (irrelevant data removed) were, after two loops: mdiff - Total allocations: 18 bytes. m1 - Total allocations: 7240 bytes. mdiff - Total allocations: 18 bytes. m1 - Total allocations: 7258 bytes. As can be seen, each time through the loop, the Total Memory Allocations increases by 18 bytes. If a global CString is used, there is no memory leak ie: CString str = "hello"; void CTestSQLStringDlg::fred(){ CString string =str; } Although I can use this workaround, it is very impractical to Globally declare all of the constant strings I need. I would like to know why this memory leak occurs or an alternate practical work around?? Thanks Simon

Walter Gildersleeve

Simon, Are you sure that a) the memory is truly leaking, and b) it is coming from the static CString? (I have trouble believing that CString, one of the most popular members of the MFC library, would leak memory. Also consider that CString uses reference counting; seperate CString objects set to the same static data always point to the same m_pchData, until one or the other is changed). I suspect that what you're witnessing is a false memory leak; either that, or code in other parts of fred() is leaking memory. Using CMemoryStatus::DumpAllObjectsSince() shows no leaks when a static CString is alloted in fred(); perhaps using that function might better point out the exact location of any leaks which might exist. Walter Gildersleeve Freiburg, Germany walter.gildersleeve@pe-gmbh.de

Jim Howard

I agree with Walter. I set up a test with this code: class CTestSQLStringDlg { public: void fred(); void OnButton1(); }; void CTestSQLStringDlg::fred() { CString string ="hello"; } void CTestSQLStringDlg::OnButton1() { int i=0; while(i<10) { CMemoryState m1, m2, mdiff; m1.Checkpoint(); fred(); m2.Checkpoint(); mdiff.Difference(m1, m2); mdiff.DumpStatistics(); TRACE("\n"); ++i; } } void main(int argc, char* argv[]) { CTestSQLStringDlg ts; ts.OnButton1(); } I get no leak. CString is the most popular MFC class, I really doubt that it is leaking, because if it were then almost no MFC apps would work for long. Jim:confused:

Ghazi H Wadi

Well, CString is not the greatest thing since the sliced bread ;P "When CString::ReleaseBuffer is called and the length of the string is less than the allocated buffer length, the extra bytes are not released. This just means that your program might end up maintaining more memory than absolutely necessary. This will not cause a memory leak. All of the memory will be freed when the CString object is destroyed." This was the case for version 4.0 and earlier versions of VC. I don't know if this is reaccuring in VC 6.0 . It would be intersting though. Cheers Alfadhly

Jim Howard

<RANT> That particular bug is fixed in VC 6.0, but you can certainly cause problems by directly writing to the CString internal buffer. A "Releasebuffer" call just sends chills up my spine. I'd rather you use a few dozen goto's than write into the CString buffer. As I pointed in another post here a couple of days ago, I've been writing MFC code since 1994 and have never directly changed a CString internal buffer. There is never a good reason to do this. Not ever. Not even once. I never told any one to lie. Using the evil GetBuffer/ReleaseBuffer members combines the worst features of C++ and C. I'm not against a nonmodifable pointer to the buffer ( like STL's c_str() ), but casting away the constness of the buffer is worse than murder. I think that it is evil to muck about with the internal buffer of a string object. If you want to diddle with a buffer then make a proper buffer and party on. The fact that CString GetBuffer/ReleaseBuffer tempts you unzip it's pants and reach inside is the major reason that CString is not as good as sliced bread. </RANT> Jim :mad: ;)

Ghazi H Wadi

lol, ;P ;P Sometimes it too scary to question the authorities, isn't? Mike and his team recomended, posted it as a sample work around. So there. Personally , I will use it when I use goto... but perhaps _asm is better And don't shoot the masenger. :cool: Cheers Alfadhly

Erik Funkenbusch

I really don't understand what you problem is with GetBuffer/ReleaseBuffer. The reason you don't want to mess with the internal buffer is because you confuse CStrings internal state. GetBuffer/ReleaseBuffer provide a mechanism in which the internal state will stay correct. There is no other reason to not use the internal buffer, and there is no difference between using CStrings buffer and allocating one yourself on the heap, except that if you need to copy the data into a CString anyways, you save the hassle of an extra copy. GetBuffer/ReleaseBuffer were designed to be used this way, why not take advantage of it?

Jim Howard

When I started with VC there was the known bug mentioned above. Even though that bug has been fixed, I still consider GetBuffer/ReleaseBuffer to be evil. The reason is because they violate what seem to me to be good object oriented design critera. I think that an object ought to manage its own interal state. But if you call "GetBuffer" then you have to keep this in mind: "If you use the pointer returned by GetBuffer to change the string contents, you must call ReleaseBuffer before using any other CString member functions. ... Note that if you keep track of the string length yourself, you should not append the terminating null character. You must, however, specify the final string length when you release the buffer with ReleaseBuffer. If you do append a terminating null character, you should pass –1 for the length to ReleaseBuffer and ReleaseBuffer will perform a strlen on the buffer to determine its length. " Please gag me with a spoon. The way I read this is as follows: "If you call Getbuffer then we will lay these mines around. Try not to step on them." Call GetBuffer and instead of having a nice solid string object that can take care of itself, you now have to stop thinking about the problem you are trying to solve and start thinking about your string object causeing a catastrophe. Let's see now, am I sure there will be null at the end? And what is the lenght of my string now? I wrote it down on a postit note somewhere, where did I put it? Why kid yourself using a string class at all if you are going to track the null character and the length yourself? Wouldn't it be more honest to just directly instantiate a buffer and use standard 'C' on it? At least then everyone seeing the code would know that you don't mind living on the edge. It just seems to me that there would be fewer errors in the world if CString did not allow direct write access to its buffer. CString has all the members you need to make the string grow, shrink, jump, and dance all you want without worrying. Why not use them? Now please me excuse while I write an expose on the "goto" statement. ;P Jim

Jim Howard

After writing the above rant, I pulled out my battered copy of Mike Blaszczak's seminal book "MFC 4 Programming with Visual C++" and looked up his discussion of GetBuffer/Release buffer. He starts with a horrendous example: CString str; LPTSTR pChars = str.GetBuffer(0); //Mike, Mike we hardly knew ye strcpy(pChars, _T("Hockey")); He points out in the following text that this kind of code can get you fired. Then he provides an even more blood chilling example under the heading of "improving efficiency with Releasebuffer()". At least the above example blows up as soon as you touch it. This example actually "works" and is the perfect illustration of how fast down the slippery slope of evil an innocent programmer might slide if he or she partakes even a bite of the GetBuffer apple: CString str; LPTSTR pWork; int nCount; //We have a string class, but lets just snort some external counter //variables and pointers just to liven up the party! //Classes? Classes?!? WE DON'T NEED NO STINK'N STRING CLASS!!!!!!!! pWork = str.GetBuffer(16384); for(nCount =0; ncount <16384; nCount++) { //Pointer arithmetic...Whee!!!!!!! Eat your heart out, Java Dudes! //If you can't hack this, then just go write some VB, you luzer!!! *pWork++ = _T('x'); } str.ReleaseBuffer(16384); //Cut the red wire, or the blue wire? Of course he could have just said CString str(_T('x'),16384); but what fun would that be? I have to admit I my faith in MikeB was severely shaken by this example, but after taking a nitro pill and allow some time for my hands to stop shaking I continued reading his discussion. After a discussion of the Bad Things that can happen if you screw up the CString buffer, MikeB redeems himself by saying just what I was saying, only he says it a lot better than me: "The most efficient approach to string intensive work with MFC involves using CString objects to hold your strings. Whenever you need to dynamically construct strings, particularly when you are concatenating them together your best bet is build the string with regular character arrays or dynamically allocated buffers before turning those arrays over to a CString object for safe keeping. (emphasis added) MikeB, an MFC Deity, has spoken! Who are we mere mortals to contradict him! :-D It's a slow day at work today, in case you hadn't guessed that already.

Erik Funkenbusch

But that's just it, the pointer returned by GetBuffer *IS* a "regular character array". I also fail to see what's wrong with his example, other than it's complexity. The code correctly works in either UNICODE or ANSI and correctly sizes the string in ReleaseBuffer. You keep saying how something is bad without proving any real data to back it up, other than your opinion. Show me an example of how this is dangerous (at least more dangerous than using a seperate buffer).

Erik Funkenbusch

Your argument doesn't make a lot of sense to me. All of the things you argue about (having to remember how long your string is, or if it's null terminated) apply equally to a seperate buffer as well. Many Windows functions require a standard LPTSTR, such as Registry and file functions (some file operations are not possible using CFile) and I see nothing wrong with using a CString allocated buffer for them, then using CString to manipulate the data. Remembering to call ReleaseBuffer is no more a problem than having to remember to call delete or free on dynamicly allocated memory (which you'd have to do for the functions I mention, since you wouldn't know at runtime how much memory they take until you query them). In my opinion, once you've called GetBuffer(), you no longer have CString object available to you. Forget it exists until you've called ReleaseBuffer. The memory returned by GetBuffer is no different from memory returned by new or malloc and requires the same care you would use for new or malloced memory. There are no "land mines" that are different from newd memory, and there is no catastrophe waiting to happen that's different from managing memory allocated by new. Is your argument that you should use CStrings own functions for string operations? If so, I agree, but I do not agree with using a seperate buffer when you need to write to a standard C character array.

Lost User

I just think that mixing 'C' style buffer manipulation with C++ objects is very dangerous from a reliablity and maintainiblity point of view. I've done a fair amount of maintenace coding, and this experience makes me see Getbuffer as an ugly hack that combines the worst elements of C and C++. The horrific example from MikeB's book shows you how ugly and dangerous GetBuffer can be. What did his use of CString in that second example do except obstafcate? If somewhere along the way the external buffer count got messed up, or releasebuffer was called with void parameters then you have created a debugging nightmare. I think MikeB's advice to use standard buffers (if you need ultimate efficency) and then save them in CString is correct. It's an opinion and we all have them. Jim

Walter Gildersleeve

I tend to agree with Jim on this, if only from a OOP purist standpoint. True, calling ReleaseBuffer() is no big deal, mayber comparable to calling delete[] for every new[]. But an advantage of OOP is that ownership and maintenance issues are more transparent...don't screw with my private members (no pun intended), and I won't screw with yours. Forgetting to call ReleaseBuffer(), or placing it in the wrong place, can be problematic, even disasterous for a program. (Not very likely? For any moderately complex function, it is just as likely to bomb on ReleaseBuffer() logic as it is to bomb on delete[] logic.) So if casting to an LPCSTR doesn't work because of the need for a non-const char buffer, than I would suggest creating a local static buffer. (I tend to alleviate my anger at the need to do this by cursing the crappy programmer who wrote the function const-unaware;) .) Walter Gildersleeve Freiburg, Germany walter.gildersleeve@pe-gmbh.de

Lost User

> You keep saying how something is bad without proving any real data > to back it up, other than your opinion. Show me an example of how > this is dangerous (at least more dangerous than using a seperate buffer). Hmmm... How about trying to *think* about potential dangers, before jumping on the keyboard to counterattack? Watch, I will demonstrate: Think (try it)... How about because you now have more places to screw up, esp. with less experienced coders: o Should I call GetBuffer() or GetBufferSetLength()? Potential screwup #1. o The call to GetBuffer()/GetBufferSetLength() : when using char/TCHAR arrays, most people are used to including space for the NUL terminator. You do not have to specify NULs most of the time when using string objects. Potential screwup #2. o The call to ReleaseBuffer() : did you place a NUL in there, and/or did you specify the length of the new string in the call to ReleaseBuffer()? Are there embedded NULs? (After all, you asked for a BUFFER, not a STRING BUFFER.) Potential screwup #3. o Walking off a local buffer can usually be quickly detected in debug builds. It might be more difficult to see if you stepped on either a CString's internal state, or a CStringData's internal state. Potential screwup #4. o People that do not know any better will store the pointer returned by GetBuffer()/GetBufferSetLength(). Local buffers tend not to change their location/address. Potential screwup #5 Another general issue: o Using any string object (that uses dynamically allocated memory) is slower than using local buffers. That is VERY IMPORTANT when dealing with multiple-CPU systems, and running into heap contention. BTW: > I think MikeB's advice to use standard buffers (if you need ultimate > efficency) and then save them in CString is correct. It's an opinion > and we all have them. No, obtaining and using a local buffer *IS* faster (moving your stack pointer) than using a string object that dynamically allocates its memory. Welcome to the world of facts! :) Peace! -=- James.

Erik Funkenbusch

None of what you claim is any more dangerous than calling new and delete to create dynamic memory. While you're right about stack variables, I'm not talking about them. I'm talking about dynamically allocated memory. For instance, when reading a registry entry, you don't know how much space you need ahead of time, so you have to dynamicly allocate. Or when you need to read a 10 MB file into a string. You'd rather allocate memory, load the data into the allocated memory, then copy it into a string, then delete the original memory? Talk about inefficiency. It's much quicker, faster, and cleaner to just get a buffer of the right length, read the data into it, then release the buffer at the correct size. It takes about 1/3 of the code, and is 4x faster. All the things you mention about embedded NULL's are things you have to worry about with local buffers as well. Your arguments just don't make sense.

Lost User

> None of what you claim is any more dangerous than calling > new and delete to create dynamic memory. (As an example...) Fifth bullet point: storage of the returned pointer. When you allocate via "new", the returned pointer tends to be valid until the call to delete. It can be stored, copied, read from, written to, all without the pointer becoming invalid. The same is not true with the pointer returned from GetBuffer(). That pointer is likely to become invalid after any action is performed on the CString object. That is documented... You *did* read the documents, yes? So you should know of that difference. > For instance, when reading a registry entry, you don't > know how much space you need ahead of time No, but you damn well better have a good idea, or you did not think things through enough. You should know that you will be reading between 0 and 8192 bytes of data, and locally allocate enough for that much data. (Being dumb enough to store large amounts of data in the Registry is another problem.) > Or when you need to read a 10 MB file into a string. You'd rather > allocate memory, load the data into the allocated memory, then > copy it into a string, then delete the original memory? > Talk about inefficiency. No, I would rather allocate the memory once, and HOLD ON TO IT, rather than pass a string object around. Passing a string object around is inefficient. String objects contain overhead that you might not need. Besides, am I sure that the file contains only text data? > It's much quicker, faster, and cleaner to just get a buffer of the > right length, read the data into it, then release the buffer at the > correct size. It takes about 1/3 of the code, and is 4x faster. 1/3 of the code compared to what? Have you examined the internals of CString's implementation, and its reference counting code? That idea is similar to people using printf() to write out a static string and thinking that it is "just one line of code" without looking into what printf() is doing internally. Also, you still end up with the potential problem of passing the string object around, where passing the pointer would be quicker (with no overhead). > All the things you mention about embedded NULL's are things > you have to worry about with local buffers as well. I did not talk about embedded NULLs, I was talking about embedded NULs (you DO know the difference, yes?). And besides, looking at the second bullet point, the point I was trying to make was that when

Jim Howard

Many Windows functions require a standard LPTSTR, such as Registry and file functions (some file operations are not possible using CFile) and I see nothing wrong with using a CString allocated buffer for them, then using CString to manipulate the data. Of course we all agree (I think) that it's ok to pass character data into a function using CString's implied "const char*" cast. If a function wants to write into a buffer, I make a real buffer and pass it in to them. I think this is a much safer coding approach. I know you and I can keep our calls to release buffer straight, but what happens when that shifty looking guy standing over there modifies our code? I don't know about you, but he looks lazy to me and I bet he hasn't carefully studied the CString docs. Maybe he's used to using (insert name of any other string class in the world), and makes the childish asumption that like them, CString won't just hand him a pointer that has a ticking time bomb attached to it. I trust you to keep it straight, but I don't trust him. Remembering to call ReleaseBuffer is no more a problem than having to remember to call delete or free on dynamicly allocated memory (which you'd have to do for the functions I mention, since you wouldn't know at runtime how much memory they take until you query them). In my opinion, once you've called GetBuffer(), you no longer have CString object available to you. Forget it exists until you've called ReleaseBuffer. You have put your finger on the essential evil of GetBuffer much better than I. You're exactly correct! Once you call GetBuffer you no longer have an object! GetBuffer defeats the whole purpose of object oriented programming! I have nothing more to say on this subject, excuse me while I get back to a "goto" argument on another forum. :-O Jim

Erik Funkenbusch

The GetBuffer() pointer is valid until you call ReleaseBuffer. Period. Same as a new'd data is valid until you call delete (or delete []). It's the same concept. As long as you don't call ReleaseBuffer, the pointer can be stored, copied, read from, written to, all without the buffer becoming invalid. *JUST LIKE A NEW'D POINTER* And just like a new'd pointer, you have to be careful that you or another thread doesn't delete it before you're done with it, so you can't call any functions of the CString object once you've called GetBuffer() until you call ReleaseBuffer(). If you're dumb enough to go calling CString functions when you have a pointer to the data, you're too dumb to be using new, since that requires just as much caution. How exactly should I "have a damn well good idea" of how much data is in a registry key before hand? If my program wrote the data, fine, but if I'm reading reg keys that i didn't write, I have no idea. String objects are fairly efficient due to reference counting. I can copy them by value or reference and they're roughly the same. Only if I need to modify the string does it matter. You seem to have these preconcieved notions that all programs should know ahead of time exactly how much memory they need to allocate. That's simply not true of many kinds of apps, especially those that deal with data that were not written by the app. Your argument about know if it's text data or not is irrelevant, since it's quite simple to test the data in the file. My argument about less code is an argument about readability of the code. The less code you have, the easier it is to read (as long as you're not entering the obfuscated C contest) and understand. It has nothing to do with how much code gets executed. There is no difference between NUL and NULL in C++. Both are 0. In C, NULL is a (void*)0, while NUL is a literal 0. As for remembering whether to put a null at the end of the string, there are many C functions which do not put NUL's at the end. strncpy doesn't for instance, and you have to know this to put the null in yourself, while strcpy does. The point is that you *ALWAYS* had best know whether there is a null there or not, or you are incompetant. Even if you don't know, you don't have to guess. Put a null in anyways, whether you need it or not. The documentation is misleading, in that they are saying you don't *HAVE* to put a null in when specifying a length to ReleaseBuffer, not that you aren't supposed to. And what makes you think that using CString w

Erik Funkenbusch

GetBuffer doesn't defeat the purpose of OO programming. In fact, it does exactly what auto_ptr does when you assign one auto_ptr to another. It basically gives you ownership of the memory until you're done with it. I'll agree that it doesn't enforce this (and it probably should, but it's too late to change that). That's why i always use GetBuffer() and ReleaseBuffer() as close together as possible. Typically with a single operation between them. char *p = s.GetBuffer(filesize); ReadFile(handle, p, filesize, &bytesread, NULL); s.ReleaseBuffer(bytesread); is much cleaner, easier to maintain, and in my opionion safer than: char *p = new char[filesize] ReadFile (handle, p, filesize, &bytesread, NULL); s = p; delete [] p; For starters, if you're worried about lazy and incompetant maintainers, there's lots to go wrong with the second. They might not know the difference between delete and delete []. the copying of the data can cause s to reallocate, possibly causing an allocation failure. But you won't know this until *AFTER* you've read the data, rather than before. So now you have to figure out how to unread the data and reposition the file pointer to deal with that. Also, this *REQUIRES* at a minimum, 1 delete and one new and one copy. The first example may not require any newly allocated memory if the CString buffer is already long enough, and if that's the case a copy won't happen either.

Sven Axelsson

OK, so what's with the holy war? I haven't checked all messages in this thread, so forgive me if this has been posted already. Why not use something like:

class CStringBuffer
{
public:
CStringBuffer(CString &str, int len = 0)
: m_str(str) { m_buf = str.GetBuffer(len); }
~CStringBuffer() { m_str.ReleaseBuffer(); }
operator LPTSTR() { return m_buf; }
private:
CString &m_str;
LPTSTR m_buf;
};

to handle acquisition/release like

CString str;
CStringBuffer buf(str, 100);
sprintf(buf, "%s %d", "Whatever", 666);

I almost always use a wrapper class like that for API pairs like GlobalLock/GlobalUnlock and such. Works great! /sven axelsson, sweden