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 and learn good C++. — GManNickG
– GManNickG, Commented Oct 11, 2010 at 3:36

Arun · Accepted Answer · 2012-10-28 15:50:05Z

7

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

edited Oct 28, 2012 at 15:50

user283145

answered Oct 11, 2010 at 2:28

Arun

20.6k12 gold badges54 silver badges61 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

JoshD · Accepted Answer · 2010-10-11 02:37:44Z

4

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.

edited Oct 11, 2010 at 2:37

answered Oct 11, 2010 at 2:28

JoshD

12.9k3 gold badges46 silver badges54 bronze badges

1 Comment

Daryl Hanson Over a year ago

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.

No one in particular · Accepted Answer · 2010-10-11 02:30:46Z

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, 2010 at 2:30

No one in particular

2,6722 gold badges17 silver badges20 bronze badges

Collectives™ on Stack Overflow

Dynamically Growing an Array in C++

3 Answers 3

Comments

1 Comment

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related