Errоr: Invalid address specified to RtlValidateHeap

Question

I am making a program that checks if stacks are equаl.

With this code, I am getting an error:

HEAP[ldf.exe]: Invalid address specified to RtlValidateHeap

It works until returning answer in func equal_stacks, so maybe a problem has to do something with deleting stacks that he gets when function starts?

Destructor was written by my teacher, so it is probably right. Maybe a problem is somewhere in equals_stacks function?

#include <iostream>

template<typename T>
class stackm{
    int capacity;
    int count;
    T* data;
public:
    stackm();
    ~stackm();
    void push(const T& elem);
    void push(T&& elem);
    void pop();
    T& top();
    const T& top() const;
    int size() const;
};

template<typename T>
stackm<T>::stackm() {
    capacity = 1;
    count = 0;
    data = new T[1];
}

template<typename T>
stackm<T>:: ~stackm() {
    delete[] data;
}

template<typename T>
void stackm<T>::pop() {
    --count;
}

template <typename T>
void stackm<T>::push(const T& elem) {
    if (this->count == this->capacity) {
        size_t new_capacity = capacity * 2;
        T* new_data = new T[new_capacity];
        for (size_t i = 0; i < this->capacity; ++i) {
            new_data[i] = std::move(this->data[i]);
        }
        delete[] this->data;
        this->capacity = new_capacity;
        this->data = new_data;
    }
    data[count++] = elem;
}

template <typename T>
void stackm<T>::push(T&& elem) {
    if (this->count == this->capacity) {
        size_t new_capacity = capacity * 2;
        T* new_data = new T[new_capacity];
        for (size_t i = 0; i < this->capacity; ++i) {
            new_data[i] = std::move(this->data[i]);
        }
        delete[] this->data;
        this->capacity = new_capacity;
        this->data = new_data;
    }
    data[count++] = elem;
}

template <typename T>
int stackm<T>::size() const {
    return count;
}

template <typename T>
T& stackm<T>::top() {
    return data[count - 1];
}

template <typename T>
const T& stackm<T>::top() const {
    return data[count - 1];
}

template<typename Stack>
bool equal_stacks(Stack s1, Stack s2) {
    if (s1.size() != s2.size()) {
        return false;
    }
    while (s1.size() != 0) {
        if (s2.top()  != s1.top()) {
            return false;
        }
        s1.pop();
        s2.pop();
    }
    return true;
}

int main() {
    stackm<int> stac1 = stackm<int>();
    stackm<int> sd23 = stackm<int>();
    sd23.push(23);
    sd23.push(45);
    std::cout << equal_stacks<stackm<int>>(stac1, sd23);
}

You're going to want to read this: [Rule of 3/5/0](https://en.cppreference.com/w/cpp/language/rule_of_three). And note, you're `equal_stacks` call in `main` needlessly specifies the template argument; that should be deducible, *and* you're arguments are passed by-value (this triggers problem of not implementing rule-of-3/5/0 correctly). — WhozCraig, Oct 03 '22 at 19:14
I pass stacks by value to not change thеm with pops used in func. — Catnip, Oct 03 '22 at 19:21
I understand that. Read the [linked article](https://en.cppreference.com/w/cpp/language/rule_of_three). Or [this one](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three/4172724#4172724) if you prefer SO posts. It's important. — WhozCraig, Oct 03 '22 at 19:26
@Catnip "*I pass stacks by value to not change thеm with pops used in func*" - Hint: you can compare for equality *without* popping any items at all, if you make `equal_stacks()` be a `friend` of the `stackm` class, or even change it into an `operator==` of the `stackm` class, so it can access the `count` and `data` members directly. — Remy Lebeau, Oct 03 '22 at 21:31

Thomas Weller · Answer 1 · 2022-10-03T19:37:28.707

0

You have delete[] data; in line 26 and delete data; in line 39. data is an array, so you delete it with delete[] only.

And you never use new_data anywhere. You probably want to assign it to data after line 39.

Also note that you have written a custom destructor, so you should follow the rule of 5 and specify how your class shall behave in case of copy constructor, copy assignment, move constructor and move assignment.

You also want to learn how to debug small programs.

edited Oct 03 '22 at 19:37

answered Oct 03 '22 at 19:16

Thomas Weller

55,411
20
125
222

I removed mistakes, but that error still happens – Catnip Oct 03 '22 at 19:27
@Catnip: you have not fixed the last paragraph. When a copy of your stack is created, you need to allocate a new pointer and copy all the values. Please, implement the rule of 5. Currently, only the pointer will be copied. When the copy goes out of scope, the copied pointer is deleted. When the original goes out of scope next, the original pointer is deleted. This results in the same memory being freed twice. – Thomas Weller Oct 03 '22 at 19:34
thomas, thanks for the article you shared – Catnip Oct 03 '22 at 19:51

Catnip · Answer 2 · 2022-10-05T11:58:07.740

I solved it by passing values to function by reference

#include <iostream>
template<typename T>
class stackm{
    int capacity;
    int count;
    T* data;
public:
    stackm();
    ~stackm();
    void push(const T& elem);
    void push(T&& elem);
    void pop();
    T& top();
    const T& top() const;
    int size() const;
};

template<typename T>
stackm<T>::stackm() {
    capacity = 1;
    count = 0;
    data = new T[1];
}
template<typename T>
stackm<T>:: ~stackm() {
    delete[] data;
}
template<typename T>
void stackm<T>::pop() {
    --count;
}
template <typename T>
void stackm<T>::push(const T& elem) {
    if (this->count == this->capacity) {
        size_t new_capacity = capacity * 2;
        T* new_data = new T[new_capacity];
        for (size_t i = 0; i < this->capacity; ++i) {
            new_data[i] = std::move(this->data[i]);
        }
        delete[] this->data;
        this->capacity = new_capacity;
        this->data = new_data;
    }
    data[count++] = elem;
}

template <typename T>
void stackm<T>::push(T&& elem) {
    if (this->count == this->capacity) {
        size_t new_capacity = capacity * 2;
        T* new_data = new T[new_capacity];
        for (size_t i = 0; i < this->capacity; ++i) {
            new_data[i] = std::move(this->data[i]);
        }
        delete[] this->data;
        this->capacity = new_capacity;
        this->data = new_data;
    }
    data[count++] = elem;
}

template <typename T>
int stackm<T>::size() const {
    return count;
}

template <typename T>
T& stackm<T>::top() {
    return data[count - 1];
}

template <typename T>
const T& stackm<T>::top() const {
    return data[count - 1];
}

template<typename Stack>
bool equal_stacks(Stack &s1, Stack &s2) {
    Stack temp1 = Stack();
    Stack temp2 = Stack();
    if (s1.size() != s2.size()) {
        return false;
    }
    while (s1.size() != 0) {
        if (s1.top() != s2.top()) {
            restoreStack<Stack>(s1, temp1);
            restoreStack<Stack>(s2, temp2);
            return false;
        }
        temp1.push(s1.top());
        temp2.push(s2.top());
        s1.pop();
        s2.pop();
    }
    restoreStack<Stack>(s1, temp1);
    restoreStack<Stack>(s2, temp2);
    return true;
}

I decided not to follow advice of WhozCraig and implement rulе of 3/5/0, because code of stack was given to me by teacher and i think i am not supposed to change it.

if that really was the code given by your instructor, you can tell them, in the nicest way possible, that their code is at-best incomplete, at worst naively so. You don't even need to actually *use* the stack operations `stackm s1,s2; s1 = s2;` will invoke undefined behavior. — WhozCraig, Oct 03 '22 at 20:53
@Catnip your code may compile and work as you expect, but it is modifying the `s1` and `s2` objects and NOT all of the `return` paths are restoring them to their previous state before exiting. There are much cleaner ways to implement `equal_stacks()` without modifying the source objects at all. — Remy Lebeau, Oct 03 '22 at 21:29
@Catnip I would be very surprised if your teacher actually taught you about `std::move()` (which you are not using everywhere you should be) but didn't teach you about the Rule of 3/5/0 and implementing proper move semantics. — Remy Lebeau, Oct 03 '22 at 21:46

Errоr: Invalid address specified to RtlValidateHeap

2 Answers2