new without delete => bad ?

Printable View

10-12-2005
drrngrvy

new without delete => bad ?

I'm not sure if I'm missing something, or just reading into the paranoia about C++ and garbage collection too much.

Say I have the code below:

Code:

void f( int env_cl ) { char *buf = new char[env_cl]; for( int i=0; i<= env_cl && buf[i] != EOF; i++ ) { buf[i] = std::cin.get(); } buf[i] = 0x00; this->unencode( buf ); }

Since the function isn't returned to after calling 'unencode()', from what I gather the variable 'buf' will just stay there for as long as the OS allows it to. I assumed that would leak memory over time, but I can't seem to create that leak.

I am missing something, right? You have to 'delete' each variable created on the free store don't you?

PS. I know using strings would solve this problem, but it would create others, not to mention the fact that I would still not know the answer.
10-12-2005
Daved

Yes, that is a memory leak (assuming unencode doesn't call delete []). You probably won't notice the leak in this case since it is only leaking one byte for each character it gets from the standard input. But if you gave it large enough input you would eventually notice the memory usage growing and growing.

BTW, it should be i < env_cl in your for loop control (not <=). In fact, it should be i < (env_cl-1) so that room is left for the null character.
10-12-2005
ryeguy

But the memory is still deallocated upon program termination right?
10-12-2005
sand_man

The OS usually does that but it doesn't have to.
10-12-2005
drrngrvy

I've changed the code a little, but another strange thing cropped up:

Code:

void f( int env_cl ) { char *buf = new char[env_cl+1]; for( int j=0; j< env_cl && buf[i] != EOF; j++ ) { buf[j] = std::cin.get();//>> buf; } buf[env_cl] = 0x00; strcpy( env_qs, buf ); delete [] buf; this->unencode( env_qs ); }

By the time the code reaches delete [], buf is apparently gone... In none of the documentation I've read does it say that strcpy deletes the char * it's copying from.

Wierd. I just hope buf has actually been deleted.
10-12-2005
Daved

Sure it has. You use new[] to allocate space for it and then call delete[] to free the memory. I don't what you are doing to determine whether buf is gone or not, but you can't really tell by looking at it.
10-13-2005
rockytriton

windows 95 and higher will delete the memory upon program termination
10-13-2005
hk_mp5kpdw

Quote:

Originally Posted by drrngrvy

Since the function isn't returned to after calling 'unencode()', from what I gather the variable 'buf' will just stay there for as long as the OS allows it to.

The variable buf itself is allocated on the stack and will get popped off once the function ends just like any other local variable. It is the memory, allocated via new, and not the variable that persists after the function call has completed. Without passing the address of this memory back or somehow keeping track of it otherwise, you lose your handle to it and lose the ability to delete it later on therefore causing a memory leak within the program.
10-13-2005
dpro

Quote:

Originally Posted by rockytriton

windows 95 and higher will delete the memory upon program termination

Just to quote this, it is true, however don't let this lull you into thinking it'll be ok. Ocassionally and I admit I "worked hard" at this... you can cause really funky errors where windows can't cope (and occassionally blue screens), however those are extremely rare from what I've seen. However the warning remains, don't let windows protect you from your own mistakes :)
10-13-2005
drrngrvy

Thanks for the input guys. One problem though:

Quote:

Originally Posted by Daved

Sure it has. You use new[] to allocate space for it and then call delete[] to free the memory. I don't what you are doing to determine whether buf is gone or not, but you can't really tell by looking at it.

I meant that the line delete [] buf causes a null reference exception (or whatever it's called). As in, after using strcpy() to copy buf to env_qs, it's illegal to call delete [] on buf. I really don't know why that happens... Any ideas.
10-13-2005
Daved

>> As in, after using strcpy() to copy buf to env_qs, it's illegal to call delete [] on buf.

That shouldn't happen. The strcpy function shouldn't change buf, and you should still be able to delete[] it successfully. If you are getting a crash there, it is due to some other problem in your program.
10-13-2005
drrngrvy

OK thanks. I solved the memory leak, but the code isn't as obvious as I thought it could have been. I can see why people refer to memory management as 'niggly'.

I ran into a few (as yet unresolved) issues along the way, but I'll keep them to myself since they're not on topic. One that is though: aren't there any delete-like operators that will work on both free-store and non free-store variables? It would make things cleaner if there was one.
10-13-2005
Daved

You don't need to delete non free-store variables, and it would make the language way too slow to have built-in capability to figure out when you are using delete on a non-free store memory.

The easiest way to not worry about these things is to take advantage of good C++ design and available tools to leave the memory management to existing classes. That is why the C++ string class is recommended over C style strings, because it manages the memory for you - no leaks, fewer worries. The same is true for vector versus C style arrays.

Most often the "other" problems created by C++ solutions are just a little extra work to learn the appropriate syntax. But doing so will save you from many of these types headaches in the future.
10-13-2005
ChaosEngine

Quote:

Originally Posted by drrngrvy

aren't there any delete-like operators that will work on both free-store and non free-store variables? It would make things cleaner if there was one.

why would you need to delete non-heap variables?

Manual memory management is, in general, a receipe for disaster. Good coding practice in C++ is to use the RAII idiom. Specifically have a look at the standard library auto_ptr and (even more useful) the boost shared_ptr classes.

Hope that helps
10-13-2005
Ancient Dragon

I realize you are concentrating on memory allocation, but haven't you noticed the error in this loop???

Code:

for( int j=0; j< env_cl && buf != EOF; j++ ) { buf[j] = std::cin.get();//>> buf; }

Hint: when is buf ever going to be EOF? Answer: It won't!
10-13-2005
drrngrvy

Reading what you're all saying makes sense. I feel stupid for thinking it was a good idea. I'm looking into those links ChaosEngine. The reason I wanted to delete the non-heap variable was to save me having to add in an extra assignment and conditional but I suppose that's just OTT efficiency. I can't help myself, despite not even knowing how to pull it off properly. :)

that was a typo I think ancient dragon, or a copio maybe. What I have now may not be much better:

Code:

void CGI::accept_post() { if( getenv("CONTENT_LENGTH") ){ char *endptr; env_cl = (int)strtol(getenv("CONTENT_LENGTH"), &endptr, 10); } if( env_cl > 1 && env_cl <= MAX_BUF_SIZE ){ // leave in the '1' env_qs = new char[env_cl+1]; // FREE-STORE VARIABLE for( int i=0; i< env_cl && env_qs[i] != EOF; i++ ) { env_qs[i] = std::cin.get(); } env_qs[env_cl] = 0x00; this->unencode( env_qs ); }else{ this->header(); std::cout<< "Don't do that."; } }

env_qs is a char * at class scope. ~CGI() deletes it.

Daved, you've almost convinced me to use strings, but I don't like either how the c-string functions don't work on them - I'm guessing calling c_str() and casting to a char * every time you need one is highly inefficient (guessing being the key word) - or how few people seem to use them. It makes me a bit wary. Shaky knees and all.
10-13-2005
ChaosEngine

Quote:

Originally Posted by drrngrvy

Reading what you're all saying makes sense. I feel stupid for thinking it was a good idea.

don't. we all gotta learn sometime.

Quote:

Originally Posted by drrngrvy

I'm looking into those links ChaosEngine.

good, they'll save you much pain and suffering!

Quote:

Originally Posted by drrngrvy

Daved, you've almost convinced me to use strings, but I don't like either how the c-string functions don't work on them - I'm guessing calling c_str() and casting to a char * every time you need one is highly inefficient (guessing being the key word) - or how few people seem to use them. It makes me a bit wary. Shaky knees and all.

first, if you do use strings (and you should) don't use the c-string functions with them (unless you really need a c-string for a 3rd party lib or something). Use the std::string methods, which replace the str* functionality
example

Code:

std::string s1 = "hello, world!"; std::string s2 = s1; // instead of strcpy if (s1 == s2) doSomething(); // instead of strcmp size_t len = s1.size() // instead of strlen

second, calling c_str() is pretty efficient. It's usually inline and optimised away.

and those "few people" that use them? they're known as "good C++ programmers"!!! :D
10-13-2005
Daved

>> how few people seem to use them

I'm not sure what makes it seem as though few people use them. My company tends to be a bit behind the times but everybody here uses the string class or MFC's CString, and nearly every experienced C++ programmer I've seen on forums and newsgroups uses and recommends the string class. I think a lot of books, tutorials and teachers simply use outdated examples and designs, and there are tons of converted C programmers out there who haven't dived into C++ completely. But that doesn't mean there aren't tons of C++ programmers that use these tools every day.
10-13-2005
Ancient Dragon

Quote:

Originally Posted by drrngrvy

or how few people seem to use them.

Those are mostly people in academic environment where they try to teach both C and C++ strings. It only takes a few minutes to learn std::string class, and after that you will probably rarely, if ever, revert to C strings. Same goes for arrays -- normally would use std::vector instead of C arrays because they are just sooo damned convient and prevent many common errors with c arrays.
10-14-2005
dpro

Quote:

Originally Posted by Ancient Dragon

Those are mostly people in academic environment where they try to teach both C and C++ strings. It only takes a few minutes to learn std::string class, and after that you will probably rarely, if ever, revert to C strings. Same goes for arrays -- normally would use std::vector instead of C arrays because they are just sooo damned convient and prevent many common errors with c arrays.

Totally on target about std::string :) Love them so much compared to old-style char arrays :). Obviously there are things that are not perfect about std::string, but that applies to pretty much everything else as well :)
10-14-2005
drrngrvy

I'm not really sure why I got that impression from; I don't study Comp Sci or anything. Although I had noticed that books seem to always use std::strings, I thought that might have been just for simplicity, since the same books always use namespace std, which I gather is a bit silly.

I know this question isn't strictly on topic anymore, but I'm still a wee bit wary and I don't want to start off too deluded. From what I've seen - since I've looked into the string class functions now - I've realised how damn simple/intuitive strings are, but surely that implies that they aren't quite as efficient as c-style strings. Maybe they are for complex string manipulations, but surely generality has to imply some compromise?

example:

Code:

void cStyle( char *cptr ) { std::cout<< "blah blah wombat" << cptr << std::endl; } // compared to void stdStr( std::string str ) { std::cout<< "blah blah wombat" << str << std::endl; }

Ok, so I was going to ask if those two would be different, for the sake of arguement, but in the eyes of progress, I tried it myself. It turns out that the two are actually different and, on MSVC at least, the pass-by-string function was (suprisingly) faster. The difference was consistently about 3 seconds per 100,000 calls to the function.

Gah! I suppose I'll have to admit it soon...
!(std::string < char *);
10-14-2005
Daved

The reason strings can be faster is that they employ advanced optimizations that you normal wouldn't take advantage of with your character arrays. However, in many instances the string class will be slower because of its genericity. The idea is to use the clearest, simplest tool unless you test your application and find that string processing is slowing it down.

BTW, if you have any concerns about performance you'd normally want to pass that std::string by (const) reference to avoid a copy. MSVC might have optimized away the copy or used reference counting, but you shouldn't count on it and it is a simple change.