|
Anyone know a good report writer tool with graphic editor and source code
for VC++??
Tanks
DL
|
|
|
|
|
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
|
|
|
|
|
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
|
|
|
|
|
I agree with Walter. I set up a test with this code:
<br />
class CTestSQLStringDlg<br />
{<br />
public:<br />
void fred();<br />
void OnButton1();<br />
};<br />
<br />
void CTestSQLStringDlg::fred()<br />
{<br />
CString string ="hello";<br />
}<br />
<br />
void CTestSQLStringDlg::OnButton1() <br />
{<br />
int i=0;<br />
while(i<10) <br />
{<br />
CMemoryState m1, m2, mdiff;<br />
m1.Checkpoint();<br />
fred();<br />
m2.Checkpoint();<br />
mdiff.Difference(m1, m2);<br />
mdiff.DumpStatistics();<br />
TRACE("\n");<br />
++i;<br />
}<br />
<br />
}<br />
<br />
void main(int argc, char* argv[])<br />
{<br />
CTestSQLStringDlg ts;<br />
ts.OnButton1();<br />
}
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
|
|
|
|
|
Well,
CString is not the greatest thing since the sliced bread 
"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
|
|
|
|
|
<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  
|
|
|
|
|
lol,
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.
Cheers
Alfadhly
|
|
|
|
|
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?
|
|
|
|
|
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.
Jim
|
|
|
|
|
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:
<br />
CString str;<br />
LPTSTR pChars = str.GetBuffer(0);
strcpy(pChars, _T("Hockey"));<br />
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:
<br />
CString str;<br />
LPTSTR pWork;<br />
int nCount;<br />
<br />
<br />
pWork = str.GetBuffer(16384);<br />
for(nCount =0; ncount <16384; nCount++)<br />
{<br />
*pWork++ = _T('x'); <br />
}<br />
str.ReleaseBuffer(16384);
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!
It's a slow day at work today, in case you hadn't guessed that already.
|
|
|
|
|
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).
|
|
|
|
|
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
|
|
|
|
|
> 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.
|
|
|
|
|
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.
|
|
|
|
|
> 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 you create a local buffer for storing strings, you (should) know enough to include the terminating NUL character at the end of the string. When dealing with the pointer returned from GetBuffer(), is this always the case?
Also, you do not find any use in the ability to quickly detect buffer overruns in debug builds by using local buffers?
> Your arguments just don't make sense.
Read them again. Eventually, you will get it.
-=- James.
|
|
|
|
|
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 won't inform you if you overrun memory?
|
|
|
|
|
> The GetBuffer() pointer is valid until you call ReleaseBuffer. Period.
> 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*
Uh, no... The pointer becomes invalid under certain situations, like reallocation of the internal buffer (assign a much longer string into it), copying another CString into it, etc. (Friendly Tip: test such things before publishing them.) Period.
> 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.
A pointer allocated via "new" does not become invalid the next time "new" is called. Nor does it become invalid after scope where "new" was called ends (unlike a CString object going out of scope). A pointer obtained via GetBuffer() is a different story, even by your own words. This is exactly the reason why the argument about not using GetBuffer got started. Because the pointer returned by that function requires special handling (of the object).
> so you can't call any functions of the CString object once you've called
> GetBuffer() until you call ReleaseBuffer().
Make up yer mind. Which is it? "until you call ReleaseBuffer()", or "can't call any functions of the CString object"? Talking about not making sense...
>> None of what you claim is any more dangerous than calling
>> new and delete to create dynamic memory.
You (and I both) just mentioned one reason why using GetBuffer() to obtain a pointer is more dangerous than using "new".
> 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.
If you have no idea, then you also have no idea if the data you are going to be reading is binary or not. Hence, using a CString-based buffer, to hold data that could contain NUL characters is stupid. (And, yes, I do know that you can get both bits of info at runtime.)
> You seem to have these preconcieved notions that all programs should know
> ahead of time exactly how much memory they need to allocate.
No, I have experienced notions that you should know (or at least have an idea about) what you are getting yourself into before starting a project. Serious developers know how important research is (or as our local Guru puts it, "Think first, code later").
(Lots of misreading going on here... Am I not writing clearly enough?)
> String objects are fairly efficient due to reference counting.
> I can copy them by value or reference and they're roughly the same.
VERY roughly... The implementation is not the best that I have seen.
> 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.
In practice, it is not. How many characters to you test before you decide? The argument was not for a text .vs. binary file, it was for not using a CString-based buffer for data that you know nothing about in the first place.
> My argument about less code is an argument about readability of the code.
So it is not really about "less code", as you said, which was incorrect.
> 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
While in C++, yes, NULL is 0. However, NUL can be char-sized, or wchar_t-sized. NULL is (usually) int-sized. A literal "0" is (usually) int-sized. Note the difference? Good. I knew you could.
> 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.
Been reading the reference again, eh?
> The point is that you *ALWAYS* had best know whether there is a null there
> or not, or you are incompetant.
Correct. So why bother confusing newer developers by giving them the temptation to NOT manually put one in by using the CString-based buffers?
> And what makes you think that using CString won't inform you if you
> overrun memory?
Mind-reading now? I never said, or thought for that matter, that the debug features of VC would not catch things like that. I said that overruns in local buffers (stack-based) are easier to catch, and typically caught sooner, than heap-allocated ones. Your stack overruns can be found at the close of scope where the buffer was allocated. Heap based ones are usually found at "delete" time for that buffer, or if the heap is trashed badly enough, at the next heap operation.
Also, if you actually overrun the CString buffer, the debug-heap catches it, not CString itself.
BTW: There is such a thing as quitting while you (think you) are ahead...
-=- James.
|
|
|
|
|
>Uh, no... The pointer becomes invalid under certain situations,
>like reallocation of the internal buffer (assign a much longer
>string into it), copying another CString into it, etc.
>(Friendly Tip: test such things before publishing them.) Period.
No, the pointer cannot become invalid if you never use any of the CStrings member functions (including operators) while you have a reference to the internal data. As I've already said, none of those operations are valid after you've called GetBuffer() until you've called ReleaseBuffer(). Yes, you could call them, but you could also go and delete a pointer after you've allocated it and then try to use it. Just because the compiler will allow you to do something, doesn't mean it's a valid operation.
My personal policy is, once you call GetBuffer(), forget you have a CString object. Don't use it until you've called ReleaseBuffer(). This policy is no different from a policy of never calling delete on a pointer you have a reference to. Further, my policy is to never hold on to a pointer retrieved from GetBuffer for any longer than needed and to always release it in the same scope. Typically this is about 2-3 lines of code between GetBuffer and ReleaseBuffer. Even further, I only do this with local non-static variables, to prevent any possibility of multithreaded corruption.
This policy is no more dangerous than using new/delete, and is in fact less dangerous due to the stringent conditions of it's use.
>A pointer allocated via "new" does not become invalid the next
>time "new" is called. Nor does it become invalid after scope
>where "new" was called ends (unlike a CString object going out
>of scope). A pointer obtained via GetBuffer() is a different
>story, even by your own words. This is exactly the reason why
>the argument about not using GetBuffer got started. Because
>the pointer returned by that function requires special
>handling (of the object).
Consider the following code:
{
char *p = new char;
m_p = p;
auto_ptr<char> pptr(p);
}
dosomethingwith(m_p);
In other words, you can do stupid things with new/delete just like you can do stupid things with GetBuffer()/ReleaseBuffer().
Here's a better one:
char *p = new char[100];
dosomethingswith(p);
dosomethingelsewith(p};
void dosomethingwith(char*p)
{
delete [] p;
}
Of course it's contrived, but unless you know exactly what a function is doing with a pointer you pass it, it could very well be deleting your pointer and reallocating it.
But you made an argument about the safety of local arrays, Your arguments about the CString going out of scope and deleting the pointer apply equally as well to local arrays. consider this:
{
char x[100];
m_x = x;
}
dosomethingwith(m_x);
The point here is that you have to take care either way. Neither way is inherantly more or less dangerous, and if you take precautions to prevent problems, it's quite safe.
>Make up yer mind. Which is it? "until you call ReleaseBuffer()",
>or "can't call any functions of the CString object"? Talking
>about not making sense...
Huh? This statement makes no sense. My original statement makes perfect sense. What's so difficult to understand about saying "Don't call any functions of the CString object until calling ReleaseBuffer()?" Unless you are being anal retentive and pointing out that ReleaseBuffer is a function of CString, which rates a big ol' *DUH*.
>If you have no idea, then you also have no idea if the data you
>are going to be reading is binary or not. Hence, using a
>CString-based buffer, to hold data that could contain NUL
>characters is stupid. (And, yes, I do know that you can get
>both bits of info at runtime.)
There is absolutely nothing wrong with using CStrings to store strings with embedded 0's. In fact, MFC does this quite often (for instance, in common file dialogs). That's why CString::GetLength() doesn't use strlen, and that's why ReleaseBuffer() offers you an option of specifying the length.
Further, if you're reading string data from the registry, it should be text. If you're reading DWORD data, then it could be anything.
>No, I have experienced notions that you should know
>(or at least have an idea about) what you are getting
>yourself into before starting a project. Serious
>developers know how important research is (or as our
>local Guru puts it, "Think first, code later").
I fail to see what this has to do with knowing how much memory your program is going to need when you write it. There is such a thing as generic applications which work generic data. Do you really think the author of RegEdit knew exactly how much data there would be in the registry when wrote it? Sure, you might know maximums, but allocating maximum memory everytime you need buffer space is stupid when you don't need it.
>However, NUL can be char-sized, or wchar_t-sized.
>NULL is (usually) int-sized. A literal "0" is
>(usually) int-sized. Note the difference?
>Good. I knew you could.
This difference is irrelevant in any circumstance except when the size of the data is important. 0 can and is implicitly converted to any type, since it can fit in even the smallest data type. Is sizeof(NULL) and sizeof(NUL) different? yes. Is NULL == NUL? yes. In any event, NUL isn't even a definition in C++, or C for that matter. It's a data communications thing.
>So why bother confusing newer developers by giving them
>the temptation to NOT manually put one in by using the
>CString-based buffers?
This just makes no sense whatsoever. C++ is not a "newbie" language and has may ways to shoot yourself in the foot. C++ is not Pascal, and should not and does not go about limiting developers "for their own good".
>BTW: There is such a thing as quitting while you
>(think you) are ahead...
Your arguments are hypocritical. The same issues are valid with either case. One is not more inherantly dangerous than the other.
|
|
|
|
|
> No, the pointer cannot become invalid if you never use any of the CStrings member
> functions (including operators) while you have a reference to the internal data.
I know that, that is what I said. My first response paragraph countered your statement that the pointer is valid until you call ReleaseBuffer(), which, as you just confimed, was incorrect.
> My personal policy is, once you call GetBuffer(), forget you have a CString object.
> Don't use it until you've called ReleaseBuffer(). [...Other good tips deleted...]
That statement, and the others that we both have made show why the use of CStrings in that manner should be avoided.
> Consider the following code: [...Deleted...]
A valid point, but I was talking direct use, without using a class to wrap the pointer. Besides, if you need auto_ptr to handle cleanup for you, you have no business messing with pointers in the first place! 
>> Make up yer mind. Which is it? "until you call ReleaseBuffer()",
>> or "can't call any functions of the CString object"?
>
> Huh? This statement makes no sense. My original statement makes perfect sense.
That was referring to the original "...until you call ReleaseBuffer()" statement. Read it again if you do not understand.
> There is absolutely nothing wrong with using CStrings to store strings with embedded 0's.
I never said there was.
> This difference is irrelevant in any circumstance except when the size of the
> data is important.
Which also depends on which platforms, compilers, etc. you are working with. As such, you should never assume that any difference is irrelevant. For example, is the size of an "int" the same as the size of a pointer?
>This just makes no sense whatsoever. C++ is not a "newbie" language [...]
Right! This entire discussion between us is actually about that fact: the original poster had a simple problem with CString. Problems like this one, IMHO, point to a less experienced developer. Also, given the apparent (mis)use of CString, more experience with it would have been helpful.
While it was nice to have someone that I can fence with on the other side of this thread, make sure we do not lose sight of the intent in these messages. CString is a useful component of MFC, but it can be *overused* as well as *misused*. We need to not give less experienced developers so much rope that they hang themselves. First teach 'em about *how* CString works internally, and then tell them about "shortcuts" like GetBuffer(). Agreed?
Peace!
-=- James.
|
|
|
|
|
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.
|
|
|
|
|
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
|
|
|
|
|
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. 
Jim
|
|
|
|
|
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.
|
|
|
|
|
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
|
|
|
|
|
Actually, this is more dangerous in some ways than just using GetBuffer()/ReleaseBuffer() seperately. Consider this code:
CString s;
...
CStringBuffer buf(s, 100);
sprintf(buf, "%s %d", "Whatever", 666);
s = "something else";
sprintf(buf, "%s %d", "Whatever", 666);
Now, buf could well be pointing off into a dangling pointer.
I like to leave GetBuffer()/ReleaseBuffer() as close as possible together to avoid the possibility of this. Of course, this would be no different than reassigning a char * to new data, but because you hide it in a class, it's not quite so obvious.
|
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
