0

I've just finished C++ The Complete Reference and I'm creating a few test classes to learn the language better. The first class I've made mimics the Java StringBuilder class and the method that returns the string is as follows:

char *copy = new char[index];
register int i;
for(i = 0; i <= index; i++) {
    *(copy + i) = *(stringArray + i);
} //f

return copy;

stringArray is the array that holds the string that is being built, index represents the amount of characters that have been entered.

When the string returns there is some junk after it, such as if the string created is abcd the result is abcd with 10 random characters after it. Where is this junk coming from? If you need to see more of the code please ask.

Ghost
  • 103
  • 4
  • 3
    Note, there is no need to use `register` here (or indeed anywhere...) – Oliver Charlesworth Jun 02 '13 at 23:31
  • Probably not, the book just recommends using register for for loop ints. – Ghost Jun 02 '13 at 23:32
  • 1
    `i <= index` out of range – billz Jun 02 '13 at 23:33
  • 1
    The you should be learning from a better book ;) http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list – Oliver Charlesworth Jun 02 '13 at 23:33
  • Better than the Complete Reference? It's quite highly praised. – Ghost Jun 02 '13 at 23:37
  • 3
    I wonder how you picked the most un-recommended book http://accu.org/index.php?module=bookreviews&func=search&rid=214 to learn from :( I suggest erase it from your memory and use some real thing, possibly you will avoid some of the trouble. – Balog Pal Jun 02 '13 at 23:37
  • @Ghost: praised by those actually know anything about C++? – Balog Pal Jun 02 '13 at 23:39
  • @Ghost: if you search the page I linked to for that book title, you will find quite the opposite... – Oliver Charlesworth Jun 02 '13 at 23:41
  • Thanks for that, I'll have a look at some of those other books (I've already ordered Effective C++.) – Ghost Jun 02 '13 at 23:45
  • `char *copy = new char[index];` allocates and array with `index` elements numbered from `0` to `index-1`, so when you loop like `for(i = 0; i <= index; i++)` and then access element `i` (with the unnecessarily verbose `*(copy + i)`) you are guaranteed to access and element off the end of the allocated space. Bad. The idomatic loop over an array is `for (int i=0; i – dmckee --- ex-moderator kitten Jun 02 '13 at 23:54
  • @OliCharlesworth, "-1 on recommending bullshildt." I've never seen that one before, haha. – chris Jun 03 '13 at 01:28

2 Answers2

1

You need to null terminate the string. That null character tells the computer when when string ends. 

char * copy = new char[ length + 1];
for(int i = 0; i < length; ++i) copy[i] = stringArray[i];
copy[length] = 0; //null terminate it

Just a few things. Declare the int variable in the tighest scope possible for good practice. It is good practice so that unneeded scope wont' be populate, also easier on debugging and kepping track. And drop the 'register' keyword, let the compiler determine what needs to be optimized. Although the register keyword just hints, unless your code is really tight on performance, ignore stuff like that for now.

dchhetri
  • 6,926
  • 4
  • 43
  • 56
0

Does index contain the length of the string you're copying from including the terminating null character? If it doesn't then that's your problem right there.

If stringArrary isn't null-terminated - which can be fine under some circumstances - you need to ensure that you append the null terminator to the string you return, otherwise you don't have a valid C string and as you already noticed, you get a "bunch of junk characters" after it. That's actually a buffer overflow, so it's not quite as harmless as it seems.

You'll have to amend your code as follows:

char *copy = new char[index + 1];

And after the copy loop, you need to add the following line of code to add the null terminator:

 copy[index] = '\0';

In general I would recommend to copy the string out of stringArray using strncpy() instead of hand rolling the loop - in most cases strncpy is optimized by the library vendor for maximum performance. You'll still have to ensure that the resulting string is null terminated, though.

Timo Geusch
  • 24,095
  • 5
  • 52
  • 70
  • I tried index + 1, but that doesn't work. What's the correct way around it? The length of the string stored inside is typically longer than index. – Ghost Jun 02 '13 at 23:39