tags:

views:

197

answers:

8

Hello, so say I have this:

const char* src = "hello";

Calling strlen(src); returns size 5...

Now say I do this:

char* dest = new char[strlen(src)];
strcpy(dest, src);

That doesn't seem like it should work, but when I output everything it looks right. It seems like I'm not allocating space for the null terminator on the end... is this right? Thanks

+10  A: 

Yes, you should allocate at least strlen(src)+1 characters.

Juliano
+12  A: 

You are correct that you are not allocating space for the terminator, however the failure to do this will not necessarily cause your program to fail. You may be overwriting following information on the heap, or your heap manager will be rounding up allocation size to a multiple of 16 bytes or something, so you won't necessarily see any visible effect of this bug.

If you run your program under Valgrind or other heap debugger, you may be able to detect this problem sooner.

Greg Hewgill
haha thanks, I must have been getting lucky... that didn't seem right when I was doing it.
Polaris878
I'd say you were getting unlucky. Ideally this woudl ALWAYS cause an error, but unfortunately in the real world some errors don't show up right away.
Dolphin
+6  A: 

That doesn't seem like it should work, but when I output everything it looks right.

Welcome to the world of Undefined Behavior. When you do this, anything can happen. Your program can crash, your computer can crash, your computer can explode, demons can fly out of your nose.

And worst of all, your program could run just fine, inconspicuously looking like it's working correctly until one day it starts spitting out garbage because it's overwriting sensitive data somewhere due to the fact that somewhere, someone allocated one too few characters for their arrays, and now you've corrupted the heap and you get a segfault at some point a million miles away, or even worse your program happily chugs along with a corrupted heap and your functions are operating on corrupted credit card numbers and you get in huge trouble.

Even if it looks like it works, it doesn't. That's Undefined Behavior. Avoid it, because you can never be sure what it will do, and even when what it does when you try it is okay, it may not be okay on another platform.

Chris Lutz
+1 just for linking to the jargon file ... ahh ... memories
D.Shawley
+1  A: 

Alternatively, you could also use dest = strdup(src) which will allocate enough memory for the string + 1 for the null terminator (+1 for Juliano's answer).

Paradigm
And once again I make the "Note that `strdup()` is a non-standard function and may not be avaliable on any given platform" comment.
Chris Lutz
Thanks Chris, excellent point.
Paradigm
+3  A: 

The best description I have read (was on stackoverflow) and went like this:

If the speed limit is 50 and you drive at 60. You may get lucky and not get a ticket but one day maybe not today maybe not tomorrow but one day that cop will be waiting for you. On that day you will pay and you will pay dearly.

If somebody can find the original I would much rather point at that they were much more eloquent than my explanation.

Martin York
At least in that case you know __why__ you are paying dearly. In the C case, you might see "random" crashes or changes in logic and the root cause is not always easy to find.
Graeme Perrow
+1  A: 

This is why you should always, always, always run valgrind on any C program that appears to work.

Norman Ramsey
+1  A: 

Yeah, everyone has covered the major point; you are not guaranteed to fail. The fact is that the null terminator is usually 0 and 0 is a pretty common value to be sitting in any particular memory address. So it just happens to work. You could test this by taking a set of memory, writing a bunch of garbage to it and then writing that string there and trying to work with it.

Anyway, the major issue I see here is that you are talking about C but you have this line of code:

char* dest = new char[strlen(src)];

This won't compile in any standard C compiler. There's no new keyword in C. That is C++. In C, you would use one of the memory allocation functions, usually malloc. I know it seems nitpicy, but really, it's not.

BobbyShaftoe
No, his question is clearly tagged C++ too. It's just his question title that mentions C (also the fact that `strlen()` and `strcpy()` are C functions). The allocation isn't important - the important part is the incorrect allocation length and the undefined behavior.
Chris Lutz
@Chris Lutz, I'm not sure what you mean... The title of the question is "Allocate room for null terminating character when copying strings in C?" I've seen a couple questions tonight talking about using "new" in C so I just thought I'd point that out to avoid confusion for anyone reading this SO thread that may be new to the C programming language.
BobbyShaftoe
I think a lot of people talk about C and C++ interchangeably because there is so much overlap between the languages. I know I've been guilty of referring to C when I really meant C++. Those two extra syllables just take _so long_ to say.
Graeme Perrow
Yeah this is it... I'm mixing C and C++ code... bad!
Polaris878
+2  A: 

strcpy will copy the null terminated char as well as all of the other chars.

So you are copying the length of hello + 1 which is 6 into a buffer size which is 5.

You have a buffer overflow here though, and overwiting memory that is not your own will have undefined results.

Brian R. Bondy