Dynamically Growing an Array in C++

Question

I have an array of pointers of CName objects. I have the following constructor which initializes my array to size one. Then when I add an object I grow the array by 1 and add the new object. It compiles fine, however when I try to print them I just get segmentation fault error. Can you look and see if I'm doing anything wrong?

//constructor
Names_Book::Names_Book()
{
    grow_factor = 1;
    size = 0;
    cNames = (CName**)malloc(grow_factor * sizeof(CName*));
    cNames[0] = NULL;
}

void Names_Book::addCName(CName* cn)
{
    int oldSize = size;
    int newSize = size + 1;

    CName** newCNames = (CName**)malloc(newSize * sizeof(CName*));

    for(int i=0; i<newSize; i++)
    {
        newCNames[i] = cNames[i];
    }

    for(int i=oldSize; i<newSize; i++)
    {
        newCNames[i] = NULL;


    }
    /* copy current array to old array */
    cNames = newCNames;

    delete(newCNames);

    size++;

}

Answer 1

To have dynamically growable array in C++, you should use std::vector or at least look at its implementation. See: http://gcc.gnu.org/onlinedocs/libstdc++/latest-doxygen/a00730.html

Answer 2

There are a few things wrong with this function:

void Names_Book::addCName(CName* cn)
{
    int oldSize = size;
    int newSize = size + 1;

    CName** newCNames = (CName**)malloc(newSize * sizeof(CName*));

    for(int i=0; i<newSize; i++)
    {
        newCNames[i] = cNames[
    }

    for(int i=oldSize; i<newSize; i++)
    {
        newCNames[i] = NULL;


    }
    /* copy current array to old array */
    cNames = newCNames; //right here you just leaked the memory cNames was pointing to.

    delete(newCNames);  // right here you delete the new array you just created using the wrong call.

    size++;

}

Near the end you do two things quite wrong. (Commented above.)

Those last two lines should be:

free(cNames);
cNmaes = newCNames;

Also, you should do a realloc rather than slowly copying elements one by one....

With that said, you should use vector. Don't try to (poorly) rewrite what already exists.

Answer 3

The first loop should be to oldSize:

for(int i=0; i<oldSize; i++)

cNames isn't big enough for newSize.