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++;

}

This isn't C++, this is bad; sorry. :/ You should get [a book](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) and learn good C++. — GManNickG, Oct 11 '10 at 03:36

score 7 · Answer 1 · edited Oct 28 '12 at 15:50

7

To have dynamically growable array in C++, you should use std::vector or at least look at its implementation.

edited Oct 28 '12 at 15:50

answered Oct 11 '10 at 02:28

Arun

19,750
10
51
60

JoshD · Answer 2 · 2010-10-11T02:37:44.330

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.

JoshD hit it. The seg fault is happening because of the delete. You just got rid of the buffer you had allocated. You should get rid of cNames, and then copy newCNames to cNames to avoid the memory leak. — Daryl Hanson, Oct 11 '10 at 02:30

score 1 · Answer 3 · answered Oct 11 '10 at 02:30

1

The first loop should be to oldSize:

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

cNames isn't big enough for newSize.

answered Oct 11 '10 at 02:30

No one in particular

2,682
2
17
20

Dynamically Growing an Array in C++

3 Answers3

Linked

Related