Memory error with pointer

Question

I'm trying to place a value for pointer in my own class vector, but receiving the memory error. Can anybody help me, please?

class myVector
{
    int * vector;
    int size;
public:
    myVector()
    {
        size = 0;
        vector = nullptr;
    }


    void pushBack(int data)
    {
        if (size == 0)
        {
            *vector = data;
            size++;
        }
        else
        {
            int * tmp = new int[size + 1];
            for (int i = 0; i <= size; i++)
                tmp[i] = vector[i];
            tmp[size + 1] = data;
            vector = tmp;
            delete[] tmp;
        }
    }

For one thing, you don't want to `delete[]` what you just assigned to `vector`. That's just a dangling pointer. The `delete[]` should be in a destructor. — Fred Larson, Apr 28 '16 at 20:56
what happens to `*vector = data;` when `size==0` as in your if statement in `pushBack`? that should give you the answer. — Biruk Abebe, Apr 28 '16 at 21:01
@bkVnet: Yes, that's the first hurdle, anyway. There are plenty more. — Fred Larson, Apr 28 '16 at 21:05
@FredLarson Yes there are. I was just addressing one cause for the "memory error". — Biruk Abebe, Apr 28 '16 at 21:06
@bkVnet I've already thought about that. But I can't find the answer — inside133, Apr 28 '16 at 21:46
@inside133 It is you can't dereference a null pointer, i.e before it points to a valid memory location. To do that you must first allocate memory and make it point there in your `if (size == 0)` block or in your constructor which ever you choose. — Biruk Abebe, Apr 28 '16 at 21:49

score 2 · Answer 1 · answered Apr 28 '16 at 21:49

2

There are a lot of issues in the code in my opinion. But I will address just the issue you have asked. As others have pointed out, you have created a pointer vector and initialized it to nullptr. And then you are trying to store data in a nullptr. This should fix the memory issue you are facing.

MyVector {
...

MyVector() : size(0), vector(new int[1]){ }

...
}

answered Apr 28 '16 at 21:49

mrtyormaa

862
1
7
17

Yeah, I thought about the same but couldn't understaand how exactly initialize the vector by data. Thank you – inside133 Apr 28 '16 at 22:48

Nils · Answer 2 · 2016-04-29T04:29:49.293

1

When using pushBack you try to assign data to your pointer, but the pointer is set to nullptr in your constructor.

Assign data to a nullptr is not a good idea.

vector = new int[1];
*vector = data;

Should work in your case. Another nice idea is to initialize vector in the constructor.

Furthermore:

delete[] tmp;

is dangerous, the deletion may be added to your destructor in another way.

edited Apr 29 '16 at 04:29

answered Apr 28 '16 at 21:08

Nils

2,665
15
30

3

`vector = &data;` would store the address of a local variable. – François Moisan Apr 28 '16 at 21:33
*vector = data;would assign data to nullptr. – Nils Apr 28 '16 at 21:45
2

@Nils yes but `vector = &data` will store the address of a local variable. That local variable will cease to exist after the function it is declared in(`pushback`) returns and the pointer member will end up with an invalid memory address of a variable that doesn't exist anymore. – Biruk Abebe Apr 28 '16 at 22:04
Of course, you are right. I fix it. Thank you for your comment! – Nils Apr 29 '16 at 04:32

Eissa N. · Answer 3 · 2016-05-02T02:43:39.690

Well, to design such a class efficiently, you need to consider many things such as defining capacity like in STL vector. But, a quick implementation would look something like as follows. You may experiment with the code here.

#include <iostream>
using namespace std;

class myVector
{
    int* vector_;
    int size_;
public:
    myVector() : size_(0), vector_(nullptr)
    {}

    ~myVector() {
        delete[] vector_;
    }

    int size() const {
        return size_;
    }

    int operator[](int i) const {
        return vector_[i];
    }

    void pushBack(int data)
    {
        int* tmp = new int[size_ + 1];
        for (int i = 0; i < size_; i++)
            tmp[i] = vector_[i];
        tmp[size_] = data;
        delete vector_;
        vector_ = tmp;
        ++size_;
    }
};

int main() {
    myVector vec;
    vec.pushBack(10);
    vec.pushBack(2);
    vec.pushBack(7);

    for(int i = 0; i < vec.size(); ++i)
        cout << vec[i] << endl;

    return 0;
}

Note that the member vector_ is deleted in the destructor. Allocating memory to tmp in the pushBack and removing it there leave you with a dangling pointer.

should that be `delete[] vector_;` in the destructor since you used `new[]`? — Biruk Abebe, May 01 '16 at 21:56

score 0 · Answer 4 · answered Apr 29 '16 at 05:28

I don't know if this helps but the else part in the parameterized constructor has an error. When you create the new temp with size 'size_+1', the indexing of this temp ranges from 0 to "size_", so the loop itself goes out of bounds for "vector" and then assigning to temp[size_+1] may result in some sort of memory error. Answer posted by "user2229960" fixes this error by running the loop from 0 to "size_-1" and then assigning the new value to temp[size_].

Memory error with pointer

4 Answers4