C++ Dynamic Array Access Violation

Question

**** Sorry for the confusion regarding numCars in the original post. I modified the code to be consistent with the original ******

The following academic program is a simplified version of the original problem but it focuses on the issue that I have yet to resolve. There are 2 classes and a main method to this problem and the 2 classes consist of a Dealer class and a Car class. The Dealer class has a private Car* pointer that is initialized to a dynamic array in the Dealer's constructor. The error occurs in the main method when the Dealer's addCar method is invoked. In the main method I intentionally pass the Dealer variable to the addCar(Dealer& d) method to mimic the structure of the original application. The addCar method then invokes the Dealer's addCar(const Car& car) method where the access violation occurs when I execute cars[numCars++]=car; Can you explain why cars[numCars++]=car results in an access violation

/**********************************Dealer.h**************************/
#include &lt;cstdlib&gt;
#include "Car.h"

using namespace std;

class Dealer
{
    public:
        Dealer(int maxCars = DEFAULT_MAX_CARS)

:numCars(0) {cars = new Car[maxCars];}

        ~Dealer(){delete [] cars;}

        int getTotalCars() const { return numCars;}

        void addCar(const Car& car)
        {       
             cars[numCars++] = car; // Access Violation
        }

        Car* begin(){return cars;};

        Car* end(){ return cars + numCars;}

setNumCars(int count){numCars = count;}

    private:
        static const int DEFAULT_MAX_CARS = 10;
        Car* cars;
        int numCars;
};

/**********************************Car.h**********************/
#include &lt;cstdlib&gt;
#include &lt;string&gt;

using namespace std;


class Car{
    public:

        Car()
            : year(0), make(""), model("")
        {}

        Car(int year, string make, string model)
            : year(year), make(make), model(model)
        {}      

        string getMake() const {return make;}
        void setMake(string make){this->make=make;}

        string getModel() const {return model;}
        void setModel(string model){this->model=model;}

        int getYear() const {return year;}
        void setYear(int year){this->year=year;}

    private:
        int year;
        string make;
        string model;       
};


ostream& operator<< (ostream& out, const Car& car)
{
    out << car.getYear() << " " << car.getMake() << " " << car.getModel();
    return out;
}

/**********************************Main.cpp**********************/
#include &lt;cstdlib&gt;
#include &lt;iostream&gt;
#include "Dealer.h"

using namespace std;

void addCar(Dealer& d);

int main(int argc, char *argv[])
{
    Dealer d;

    addCar(d);  

    system("PAUSE");
    return EXIT_SUCCESS;
}

void addCar(Dealer& d)
{
    d = Dealer();

    d.addCar(Car(2007, "Honda", "Civic"));

    cout << d.getTotalCars() << " total cars" << endl;
}

Michael · Accepted Answer · 2009-09-22 01:26:32Z

4

void addCar(const Car& car)
{
     cars[numCars++] = car; // Access Violation
}

You never initialize numCars - it contains some value from the heap which is almost definitely non-zero. This causes you to read beyond the end of the cars array and into inaccessible memory. You should set numCars to 0 in your constructor.

On top of this, you should have some checks in addCar so that you don't overrun the cars array.

EDIT:

There are some other issues with the code - for instance, "d = Dealer();" creates a new Dealer and overwrites the one you pass by reference to addCars which doesn't seem to be what you want to do.

Try adding some additional tracing to the constructor/destructors to verify that the constructors you think are being called actually are - it appears that Dealer() should be invoking the constructor with a default argument you specified, but if not it is getting the default constructor.

edited Sep 22, 2009 at 1:26

answered Sep 22, 2009 at 1:13

Michael

55.6k5 gold badges129 silver badges145 bronze badges

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

2 Comments

Athens Holloway Over a year ago

you are correct about not initializing numCars but it was initialized in the original code. and I still get the access violation error if i change cars[numCars++]=car to cars[0|1|2|3|...]=car;

Athens Holloway Over a year ago

I modified the signature of addCars to expect a Dealer* to avoid creating a new instance of a Dealer but that didnt resolve the Access Violation error. The reinitialization of d in the addCar method looked suspicius to me too but changing it had no effect.

GManNickG · Accepted Answer · 2009-09-22 01:14:26Z

1

You're not initializing numCars anywhere, you should set it to 0:

Dealer(int maxCars = DEFAULT_MAX_CARS) :
numCars(0)
{
    cars = new Car[maxCars];
}

Do you have to use raw pointers? Why not wrap it up and use std::vector instead?

answered Sep 22, 2009 at 1:14

GManNickG

506k55 gold badges505 silver badges551 bronze badges

2 Comments

Athens Holloway Over a year ago

based on the requirements, i have to use the dynamic array

GManNickG Over a year ago

That's unfortunate. You might do the assignment with a vector, so everything else is working, then strip the vector away.

Geerad · Accepted Answer · 2009-09-22 01:14:43Z

1

Nothing in the above code initializes Dealer::numCars. It can therefore be any random garbage.

answered Sep 22, 2009 at 1:14

Geerad

3842 silver badges5 bronze badges

Comments

phoebus · Accepted Answer · 2009-09-22 01:15:57Z

1

Maybe I'm not seeing it but where do you initially set numCars?

answered Sep 22, 2009 at 1:15

phoebus

15k2 gold badges36 silver badges35 bronze badges

Comments

jussij · Accepted Answer · 2009-09-22 01:30:08Z

0

This looks like a memory leak to me since, you don't release the previous memory held by the cars pointer:

setNumCars(0) {cars = new Car[maxCars];}

and this code should really should guard against the overflow condition:

void addCar(const Car& car)        
{                                
   cars[numCars++] = car; // Access Violation        '
}

by doing something like this:

void addCar(const Car& car)        
{                                
   if (numCars < maxCars)
      cars[numCars++] = car;        '
   else
      // throw and exception .....
      // or better still grow the cars buffer
}

answered Sep 22, 2009 at 1:30

jussij

10.6k1 gold badge36 silver badges49 bronze badges

Comments

aJ. · Accepted Answer · 2009-09-22 02:18:43Z

0

cars[numCars++] = car; // Access Violation

I don't see any problems from the posted code. May be problem is elsewhere ?

Probably you can try following:

change arrays to vector and try using at() to catch out_of_range exception. something like:

   std::vector<int> myVec;
   try
   {
    int x = myVec.at(0);

   }
   catch(std::out_of_range& oor)
   {
        printf("\nout of range ");
   }

answered Sep 22, 2009 at 2:18

aJ.

35.7k22 gold badges89 silver badges132 bronze badges

1 Comment

Athens Holloway Over a year ago

i have to use the array and i agree that it looks like it should work. I have been comparing the code to other examples of dynamic arrays and i have not found any discrepencies yet.

Collectives™ on Stack Overflow

C++ Dynamic Array Access Violation

6 Answers 6

2 Comments

2 Comments

Comments

Comments

Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

2 Comments

2 Comments

Comments

Comments

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related