Access violation reading location 0xFEEEFEEE

[GaiusBaltar]

I'm trying to make it possible to have a 2 dimensional linked list with syntax like with vectors:

List<List<std::string>> words(5, List<std::string>(5, "Hello"));

I coded everything so that this kind of syntax would be possible, but now, when I execute my program, I get the following error:

First-chance exception at 0x00DA6636 in Advanced Linked Lists.exe: 0xC0000005: Access violation reading location 0xFEEEFEEE.
Unhandled exception at 0x00DA6636 in Advanced Linked Lists.exe: 0xC0000005: Access violation reading location 0xFEEEFEEE.

I assume that a nullptr is being used at some point, but it's hard to wrap my head around my code currently and I would really like to move on with my project.

main.cpp

#include "stdafx.h"
#include <iostream>
#include "List.h"

int main(){
	List<List<std::string>> words(5, List<std::string>(5, "Hello"));

	/*words[0][0] = "Hello";

	words[0].displayAt(0);*/

	system("pause"); // disregard this line...
	return 0;
}

Node.h

#pragma once

template<class T>
class Node
{
private:
	Node* next;
	T data;
public:
	Node(T val) :data(val), next(0){}
	Node() :data(0), next(0){}
	T& getData(){ return data; }
	void setData(T value){ data = value; }
	Node* getNext(){ return next; }
	void setNext(Node* node){ next = node; }

	~Node(){ /* No need */ }
};

List.h - Only methods being used are defined

#pragma once

#include "Node.h"

template<class T>
class List
{
private:
	typedef Node<T>* _NewNode;
	_NewNode head;
	_NewNode tail;
public:
	List(int length, T fillWith);
	List(int length);
	List() { head = NULL; tail = NULL; }
	void append(T val);
	void append();
	void resize(int length);												
	void remove(T val);
	void removeAt(int index);
	void clear();
	void edit(int index, T val);
	int length();
	void display();
	void display(T val);
	void displayAt(int index);
	T& get(int index);
	void move(int index1, int index2);
	void sort();


	T& operator[] (const int &i){
		return get(i);
	}

	~List(){
		clear();
	}
};

template<class T>
List<T>::List(int length, T fillWith){
	this->head = NULL;
	this->tail = NULL;
	for (int i = 0; i <= length; i++){
		append(fillWith);
	}
}

template<class T>
List<T>::List(int length){
	this->head = NULL;
	this->tail = NULL;
	for (int i = 0; i <= length; i++){
		append();
	}
}

template<class T>
void List<T>::append(T val){
	if (head == NULL){
		head = new Node<T>(val);
		tail = head;
	}
	else{
		tail->setNext(new Node<T>(val));
		tail = tail->getNext();
	}
}

template<class T>
void List<T>::append(){
	if (head == NULL){
		head = new Node<T>();
		tail = head;
	}
	else{
		tail->setNext(new Node<T>());
		tail = tail->getNext();
	}
}

template<class T>
void List<T>::clear(){
	if (head == NULL)
		std::cout << "List is empty.\n";
	else{
		List::_NewNode cur = head;
		while (head->getNext() != NULL){
			cur = head->getNext();
			head->setNext(cur->getNext());
			delete cur;
		}
		delete head;
		head = NULL;
	}
}

template<class T>
T& List<T>::get(int index){
	T tmp = 0;
	if (head == NULL)
		return tmp;
	List::_NewNode cur = head;
	int i = 0;
	while (i < index && cur->getNext() != NULL){
		cur = cur->getNext();
		i++;
	}
	if (cur == NULL || i < index){
		return head->getData();
	}
	else{
		return cur->getData();
	}
}

[MiiNiPaa]

You did not provided copy constructor for your list.
That means when you do List<List<std::string>> words(5, List<std::string>(5, "Hello")); a temporary list is created (bold), gets copied by default copy constructor (shallow copy, that means pointers are copied but not data in them) and then temporary is destroyes. After that all pointers inside inner list are pointing to the freed memory. Which VS catches by deliberately setting freed memory for some specific value.

[GaiusBaltar]

How would you recommend I change my code?

Should I add a copy constructor to my List class like so:

List(int length, T& fillWith);

Or do I have to do something else?

Edit: Apparently I have to do something else, because List(int length, T& fillWith); seems to deliver the same output.

Edit2: Now I removed the List(int length, T fillWith); constructor and the compiler says:

none of the 4 overloads could convert all the argument types

Edit3: I did some research into copy constructors and it seems that I am only aloud to pass in a reference of List? making the previous List(int length, T& fillWith); wrong?

[MiiNiPaa]

Should I add a copy constructor to my List class like so:

Copy constructor has a specific purpose: it constructs a copy of another object. And it has specific signature:

class foo
{
    foo(const foo& origin)

There is a Rule of Three: Copy constructor, Assigment operator, Destructor. If you have to provide your own implementation for one of them, you should implement others too. (C++11 add Move constructor and Move assigment to the list making it a Rule of Five)

So yes. You should implement Copy constructor and Assigment operator.

[GaiusBaltar]

Wow, this project is becoming a huge learning experience for me. Thanks, it seems I'll have to do some more research on constructor types before I move on.

[GaiusBaltar]

I implemented a copy constructor:

template<class T>
List<T>::List(const List& list){
	head = list.head;
	tail = list.tail;
}

But the code still won't work. When I run the program, I get an unhandled exception at this line in my "Node" class:

Node* getNext(){ return next; }

With the same error message as before:

Access violation reading location 0xFEEEFEEE

The line causing the error in my main function is as follows:

List<List<char>> words(5, List<char>(5, 'H'));

Also, this works, but apparently the destructor is called 3 times before the program exits and once after:

List<List<char>> words(0, List<char>());

Output:

List is empty.
List is empty.
List is empty.
Press any key to continue . . .

[MiiNiPaa]

This is still is not a proper copy constructor.

Let me explain.
Your temporary list holds a pointer "head". It points to some memory area A. Lets call that pointer temp@head
your real list holds head pointer too. It is currently pointing to the, say, null area. Lets call that pointer real@head
When you assign temp@head to real@head, both those pointers point to the same memory A.
Then temporary list is destroyed and in deletes all memory it owns, including A. So now real@head points to deleted memory. And when you trying to access it: CRASH

[GaiusBaltar]

I am totally lost right now.
I know generally what's causing the error, I just don't know how to fix it.
And I don't get why the copy constructor is being called in the first place if I type:

List<List<char>> words (5, List<char>(5, 'H'))

Shouldn't the constructor List(int length, T fillwith) be called and not the copy constructor?

[MiiNiPaa]

Your constructor: List(int length, T fillWith);
Note thatit dies not take two ints and char. It takes int and T, which in your case is List<char>.
Before we can pass that T, we need to create it first. In your case constructor (int, char) is called. Now when we do have T, we need to pass it. That is where copy constructor is called.

Basically List<List<char>> words (5, List<char>(5, 'H')) is equal to:

List<char> temp(5, 'H');
List<List<char>> words (5, temp);

but with temp being temporary.

http://www.learncpp.com/cpp-tutorial/912-shallow-vs-deep-copying/

[GaiusBaltar]

I read the article.
My problem now is that I don't know how to apply something specific like strncpy to something indefinite like T.

Also, if I write:

List<char> temp(5, 'H');
List<List<char>> words (5, temp);

I get:

Access violation reading location 0xFEEEFEEE

In my Node class at line:
Node* getNext(){ return next; }

[MiiNiPaa]

All five created sub lists are pointing to the same memory. It is completely possible that one of them wrecks it.

My problem now is that I don't know how ...

What do you need to copy list?
You need to copy all nodes preserving relationship between them. So for each node in origin list you append a copy of it to your list. Simple enough.
To do that you need to make Node properly copyable too. Well it is not that hard. You just need to copy data and set next to 0. Like that:

template<class T>
Node<T>::Node(const Node& origin) : data(origin.data), next(nullptr) {}

Try to implement List copy constructor yourself.

[GaiusBaltar]

template<class T>
List<T>::List(List& list){
	if (list.head)
	{
		for (int i = 0; i < list.length(); i++){
			append(list.get(i));
		}
	}
}

There, that's what I came up with.
And once again it boils down to:

Access violation reading location 0xCCCCCCCC

in my Node class at line:
Node* getNext(){ return next; }

... At least the memory location changed I suppose.

[MiiNiPaa]

1) Signature for copy constructor is const reference:
List<T>::List(const List& list)

[cire]

If you're using VC++, the value 0xCCCCCCCC is used to fill uninitialized memory allocated on the stack. So, you might look for a value you're using that you didn't initialize or assign any meaningful value (such as the head and tail values that you didn't set prior to calling append in your copy constructor.)

This is handy to know: http://stackoverflow.com/a/127404

[GaiusBaltar]

When I write List<T>::List(const List& list) I get the error:

cannot convert 'this' pointer from 'const List<char>' to 'List<char> &'

I believe it's because of my length() function for some reason.

[MiiNiPaa]

And it points to which line?

From what I see you will need to mark your get function as const : T& get(int index) const; (same in definition) and maybe length.

[GaiusBaltar]

Ok, I changed/added the following methods:

int length() const;	
T getVal(int index) const; // needed get() for [] operator overload. 

Copy constructor now looks like this:

template<class T>
List<T>::List(const List& list){
	if (list.head)
	{
		for (int i = 0; i < list.length(); i++){
			append(list.getVal(i));
		}
	}
}

Now I get the 0xCCCCCCCC error.
The program always breaks at this line:

Node* getNext(){ return next; }

Edit: Oh... thanks cire, it works.

[MiiNiPaa]

Show call stack:
http://stackoverflow.com/questions/945193/how-do-i-find-the-stack-trace-in-visual-studio
http://msdn.microsoft.com/en-us/library/a3694ts5.aspx

BTW: you can have both const and non-const versions of same function/operator. (That is how operator[] works for vector: you still can get value from const vector but cannot change it)

[GaiusBaltar]

Thanks so much for sticking with me for over 5 hours MiiNiPaa. I've been working on this project for a few days now and it's been seriously frustrating 80% of the time. Well, it works now and I believe it should be possible for me to finish alone. Thanks again and good day.