0

I was writing a function that allow someone to expand an array of char*, and while doing some test I noticed that when I put more than 3 elements, the second one become something corrupted.

This is the function itself:

    void Data::PushBack_String(char** PtrToPtr, char* Ptr, unsigned short Index)
    {
         unsigned short String_Length;
         for(String_Length = 0; Ptr[String_Length] != '\0'; ++String_Length);
                                                            ++String_Length;

         char* NewPtr = (char*)malloc(String_Length);
         strcpy(NewPtr, Ptr);   

         PtrToPtr = (char**)realloc(PtrToPtr, Index);
         PtrToPtr[Index] = NewPtr;
    }

Also noticed that when the number of elements exceed 17 elements the program crashes.

EnryFan
  • 422
  • 3
  • 15
  • 1
    I can think of no reason outside of academic coursework why you're not using a `std::unordered_map` for this task. – WhozCraig Oct 15 '13 at 17:31
  • I know there are easiest way to so this, but I want to learn how to manipulate dynamic arrays. – EnryFan Oct 15 '13 at 18:03
  • The way you calculate `String_Length` is wrong. You increment `String_Length` twice in each loop. It goes pass the end of the string if the length is odd. – Derek Ledbetter Oct 15 '13 at 18:11
  • @DerekLedbetter note the semi-colon after the for-loop closing parenthesis. the additional increment is outside and is the OP's way of accounting for the null-char. – WhozCraig Oct 15 '13 at 18:23

2 Answers2

0

The proper use of malloc is like malloc(String_Length * sizeof(char)+1). You should add 1 to account for the ending '\0' in the string.

Igor Popov
  • 2,588
  • 17
  • 20
0

You may try using strdup() instead. It'll cover the one byte for NULL termination you have forgotten. And it's much more readable.

Dariusz
  • 21,561
  • 9
  • 74
  • 114