1

I am trying to make a c++ program with a class which holds integers on the "heap" and has only one method, pop() which returns the first item in the class and removes it. This is my code so far:

#include <iostream>
using namespace std;

class LinkList {
int *values; //pointer to integers stored in linklist
int number; // number of values stored in linklist
public:
LinkList(const int*, int); // Constructor (method declaration)
int pop(); // typically remove item from data structure (method declaration)

};

LinkList::LinkList(const int *v, int n){
number = n;
*values = *v;
int mypointer = 1;
while (mypointer<n) {
    *(values+mypointer) = *(v+mypointer);
    mypointer++;
}
}


int LinkList::pop() {
if (number>0) {
    int returnme = *values; //get the first integer in the linklist
    number--;
    values++; //move values to next address
    return returnme;
}
else {return -1;}
}



int main() {
int test[] = {1,2,3,4,5};
LinkList l1(test,5);
cout << l1.pop() << endl;
LinkList l2(test,5);
cout << l2.pop() << endl;
return 0;
}

The issue is that its failing at the line *values = *v, if i remove the 4th and 5th lines from the main method, I no longer get this issue, so its go to be a memory management thing.

What I want to do is to get values to point to a continuous bit of memory with integers in. I have tried to use arrays for this but keep just getting random memory addresses returned by pop()

Background: normal I programming in java, I've only be using C/C++ for 2 months, I'm using eclipse IDE in ubuntu, I can make very basic use of the debugger but currently I dont have functioning scroll bars in eclipse so I can't do somethings if they dont fit on my screen.

Mark Johnson
  • 14,224
  • 4
  • 28
  • 34
Heidi
  • 249
  • 1
  • 3
  • 9

4 Answers4

4

You are dereferencing an uninitialized pointer (values) at the line *values = *v; which is undefined behavior (UB). What this line says is "get the integer that values points to and assign to it the value pointed by v". The problem with this logic is that values doesn't yet point to anything. The result of this UB is the crash that you receive.

There are many other problems with this code, such as passing a const int* to the constructor with the intent of modifying those values. The biggest problem is that this is not an actual linked list.

Marlon
  • 19,924
  • 12
  • 70
  • 101
  • Technically, the values in the const int pointer are not modified later on, but copied into internal storage in the "LinkList". So this is correct and just an attempt at building `std::vector`'s two-iterator constructor, but you are right about the other errors. – thiton Dec 29 '11 at 02:37
1

Perhaps the problem is that you're incrementing an integer - mypointer, rather than a a pointer. If the integer requires more than one byte of space, then this might lead to errors. Could you try declaring a pointer and incrementing that instead?

nindalf
  • 1,058
  • 2
  • 12
  • 15
1
*values = *v;

You dereference the values pointer in this line before initializing it. This is the source of the later errors, and the non-errors in the first three lines of main are simply due to luck. You have to allocate space via values = new int[n] and deallocate it in the destructor via delete[] values. std::vector does this work in a clean and exception-safe way for you.

thiton
  • 35,651
  • 4
  • 70
  • 100
  • how does `delete[] values` remove the values held in the array when it does know how long the array is ? – Heidi Dec 29 '11 at 17:58
  • @Heidi: [See this question](http://stackoverflow.com/questions/975792/how-does-delete-know-the-size-of-an-array) – thiton Dec 29 '11 at 18:18
1

The values member variable is a pointer to uninitialized memory. Before you start copying numbers into it you have to point it to valid memory. For example:

LinkList::LinkList(const int *v, int n){
    number = n;
    values = new int[n]; // allocate memory
    int mypointer = 0;
    while (mypointer<n) {
        *(values+mypointer) = *(v+mypointer);
        mypointer++;
    }
}

LinkList::~LinkList() {
    delete values; // release memory
}

Also, why do you call this a linked list while in fact you are using a memory array to store your numbers?

Miguel Grinberg
  • 65,299
  • 14
  • 133
  • 152