Access violation writing location when changing private part of a class, c++

Question

I'm writing a Deque (STL is not accepted, the main idea is to write Deque on your own). Getting an:

Unhandled exception at 0x009932C1 in Deque.exe: 0xC0000005:
Access violation writing location 0xFFFFFFFC.

The problem is in line:

buffer[tail++] = element;

So, I'm trying to change an element, that is private in class. What problem can it be?

Working in Visual Studio 2012, language — C++. The code:

#include "stdafx.h"
#include <iostream>
#include <cassert>
#include <deque>

using namespace std;

class Deque
{
public:
    void PushBack( int element );
    bool IsEmpty() const { return head == tail; }
    void clearDeque();
    void setBufSize( size_t size )
    {
        bufferSize = size;
        int* buffer = new int[bufferSize];
        for (size_t i = 0; i < bufferSize; i++)
            buffer[i] = 0;
    }
    int getDeque()
    {
        while ( head != tail )
        {
            if ( head == bufferSize )
            {
                head = 0;
            }
            return buffer[head++];
        }
    }
private:
    int* buffer;
    int bufferSize;
    int head;
    int tail;
};

void Deque :: PushBack( int element )
{
    buffer[tail++] = element;
    if ( ( tail == bufferSize ) && ( tail != head ) )
        tail = 0;
}

int main()
{
  Deque myDeque;
  deque <int> TrueDeque;
  myDeque.setBufSize(15);
  myDeque.PushBack(44);
  return 0;
}

Answer 1

(In addition to the problems others have noted about failure to initialize member variables)

You're using multiple variables with the same name, presumably unintentionally.

In this member function

void setBufSize( size_t size )
{
    bufferSize = size;
    int* buffer = new int[bufferSize];
    for (size_t i = 0; i < bufferSize; i++)
        buffer[i] = 0;
}

buffer is a local variable. You aren't using or updating the member variable with the same name.

Try a simple assignment buffer = new int[bufferSize]; instead of a variable declaration. Or even better, use a std::vector instead.

Answer 2

You should in constructor initialize every member. In your code you use tail without initializint it first so it has some random value.

Answer 3

Where exactly are you initializing tail? (or head)

I'm suggesting you to do it in your constructor so you can take advantage of RAII paradigm if you need other resources as well.

Answer 4

Your class does not initialize its data members. Since all of them are of primitive type (int and int*), reading their values yields undefined behaviour, i.e. anything can happen, including random crashes. Specifically, in your case, head and tail are read but never assigned or initialized.

Your class needs a constructor with an initializer list. For example:

Deque::Deque() :
    buffer(0),
    bufferSize(0),
    head(0),
    tail(0)
{
}

In fact, get rid of setBufSize and move its functionality directly into the constructor (and in its initializer list, where applicable).

Answer 5

You initialized neither head nor tail.

Also function setBufSize does not check whether buffer was already allocated. In this case you have a memory leak when you will call the function more than one time.

  void setBufSize( size_t size )
  {
    bufferSize = size;
int* buffer = new int[bufferSize];
for (size_t i = 0; i < bufferSize; i++)
  buffer[i] = 0;
  }

I think there is a typo in statement

int* buffer = new int[bufferSize];

There should be

buffer = new int[bufferSize];

Also you could at least initialize head and tail in this member function if your class has no explicit constructor or add initializers to the class definition. For example

//...
private:
    int* buffer = nullptr;
    int bufferSize = 0;
    int head = 0;
    int tail = 0;

Your class need to define a destructor, the copy assignment operator ans a copy constructor or define the last two as deleted.