CODEWITHSUNDEEP
c++arrayspointersobject
Dallas Phillips
Dallas Phillips

Reputation: 381

Segmentation fault when creating an array of objects

Below is code that I am trying to get to work. I want to create an array of BankTeller objects. To my understanding when a new BankTeller() is called, it creates a pointer to that object, which I save into tellers. I then attempt to insert to that psotion but get segmentation fault when I use gdb to step through and the program crashes.

BankTeller **tellers; 
BankModel(int count){
    srand (time(NULL)); //ignore this
    *tellers = new BankTeller[count];
    for(int i=0; i <= count; i++){
        tellers[i] = new BankTeller();
    }

    return;
}

Upvotes: 0

Views: 1844

Answers (4)

code_dredd
code_dredd

Reputation: 6085

I think your code is more complicated than it probably needs to be. I'd consider using std containers, like std::vector or whatever fits your needs better. Generally, you should avoid multiple levels of indirection unless it's really necessary, which in this case it does not appear to be.

Understanding Pointers

You began by declaring BankTeller **tellers, which is a pointer to a pointer to BankTeller. The line causing the segfault in your code is *tellers = new BankTeller[count];. This line returns a pointer to an array of BankTeller objects, but your declaration with the double ** says it should be getting an array to pointers to BankTeller objects. The value assigned is still interpreted as an address (which it isn't) and ends up trying to access an invalid memory location, which triggers the segfault.

Instead it should be *tellers = new BankTeller*[count];. Notice the * before the opening bracket. This line gets you an array of pointers to BankTeller objects.

Simple Example

To illustrate, forget about BankTellers and lets go back to primitives.

#include <iostream>
using namespace std;

int main()
{
        const size_t max = 3;
        int **nums;

        cout << "Creating arrays...";
        nums = new int*[max];             // <<---- not: nums = new int[max];
        for(size_t i = 0; i < max; ++i)
                nums[i] = new int(i);
        cout << "done" << endl;

        cout << "Contents: ";
        for(size_t i = 0; i < max; ++i)
                cout << *nums[i] << ' ';  // <<---- dereferenced twice to reach actual value
        cout << endl;

        cout << "Deleting arrays...";
        for(size_t i = 0; i < max; ++i)
                delete nums[i];
        delete[] nums;
        cout << "done" << endl;
        return 0;
}

Notice that this is the same situation described previously. To run it, put the code in a file called test.cpp and use this (GNU/Linux) command:

➜  /tmp  g++ test.cpp -o test && ./test
Creating arrays...done
Contents: 0 1 2
Deleting arrays...done
➜  /tmp

If you want to go over it in the debugger, add -ggdb to the g++ command above to make sure you add debugging symbols to the binary. You can then use b <linenumber> (e.g. b 10) to setup a breakpoint and p <variable_name>, (e.g. p nums, p *nums, etc.) to print the addresses and values.

But again, you don't need to use raw pointers like this. You can, and should, use containers from the Standard Template Library.

Your Code Refactored

I've re-written the sample code below, using std::vector instead of double-pointers.

#include <iostream>
#include <vector>
using namespace std;

class BankTeller
{
public:
        BankTeller() {
                cout << "Created BankTeller\n";
        }
        ~BankTeller() {
                cout << "Destroyed BankTeller\n";
        }
};

class BankModel
{
public:
        BankModel(size_t count) {
                // remember to throw exception if count <= 0
                for(size_t i = 0; i < count; ++i)
                        _tellers.push_back(new BankTeller());
                cout << "Created BankModel\n";
        }

        ~BankModel() {
                // consider using iterators
                for(size_t i = 0; i < _tellers.size(); ++i) {
                        delete _tellers[i];
                        _tellers[i] = 0;
                }
                _tellers.clear();
                cout << "Destroyed BankModel\n";
        }

private:
        vector<BankTeller*> _tellers;
};

int main() {
        BankModel *model = new BankModel(5);
        delete model;
        return 0;
}

Building and running it in my system (GNU/Linux) looks as follows:

➜  /tmp  g++ tellers.cpp -o tellers
➜  /tmp  ./tellers
Created BankTeller
Created BankTeller
Created BankTeller
Created BankTeller
Created BankTeller
Created BankModel
Destroyed BankTeller
Destroyed BankTeller
Destroyed BankTeller
Destroyed BankTeller
Destroyed BankTeller
Destroyed BankModel

Hopefully, this helps you in understanding both pointers and the benefits of using the STL.

Upvotes: 2

Samboy786
Samboy786

Reputation: 101

Try this - 

BankTeller **tellers;

tellers = new BankTeller* [count];

for(int i=0; i <= count; i++)
        {
            tellers[i] = new BankTeller();
        }

You can easily see the difference here.

Upvotes: 0

sedavidw
sedavidw

Reputation: 11691

Your tellers variable is a pointer to a pointer. So when you do

*tellers = new BankTeller[count]

You have to make sure that memory has been properly allocated to a BankTeller*

Upvotes: 1

SergeyA
SergeyA

Reputation: 62553

Welcome to the dynamic arrays of C++! Correct way of writing this is:

BankTeller* tellers = new BankTeller[count];

You do not need a pointer to pointer for this.

However, do not do this at all! Use std::vector<BankTeller>

Upvotes: 0

Related Questions