HEAP error Invalid address specified to RtlValidateHeap

Question

I have troubles with memory. I'm using struct this way:

Package.h file

#pragma once
#include <cstdlib>

struct Package {
    char *data;
    long long int *packageNumber;
    long long int *allPackages;

    Package(const int sizeOfData);
    ~Package();
};

Package.cpp

#include "Package.h"

Package::Package(const int sizeOfData) {
    void *ptr = malloc(2 * sizeof(long long int) + sizeOfData * sizeof(char));
    packageNumber = (long long int*) ptr;
    allPackages = (long long int*) ((long long int*)ptr + sizeof(long long int));
    data = (char*)((char*)ptr + 2 * sizeof(long long int));
}

Package::~Package() {
    free(data);
    free(packageNumber);
    free(allPackages);
}

And in method:

for (int j = 0; j < this->bufforSize || i * bufforSize + j < allPackages; j++) {
            Package package(this->packageSize);
            this->file->read(package.data, this->packageSize);
            *package.allPackages = allPackages;
            *package.packageNumber = i * this->bufforSize + j;
            this->dataPacked->push_back(package);
        }

after end of brackets it throws error: "HEAP[zad2.exe]: Invalid address specified to RtlValidateHeap( 00000056FEFE0000, 00000056FEFF3B20 )" I have no idea what I'm doing wrong. Please help, Michael.

EDIT: Now it is working for first iterate of loop. Helps that I changed destructor to this:

Package::~Package() {
    free(packageNumber);
}

But now destructor is executed two time on same struct object in 2'nd iterate of loop.

Ramires

Why tag this as C when it is obvious that you're using C++. And if this is C++, why not use new [ ] and delete[ ] or just a container such as std::vector? – Oaken

Why are you calling free 3 times? There is only one call to malloc being done. – Oaken

Becouse I have to do it in continuous memory fragment. Whole struct have to be in one place. – Ramires

There is no magic in malloc as opposed to using new char [ ] or std::vector<char>. They all do basically the same thing, which is allocate a contiguous chunk of memory in bytes. The difference, at least with malloc and std::vector<char> is that you no longer have to manually manage the allocation and deallocation, which is why you have an error now. – Oaken

@Oaken Thanks, but I want to allocate memory in C-way. It looks better for me to allocate n-size array of char + m-size int for example. – Ramires

but I want to allocate memory in C-way And the new problem you're having now with the destructor is because you are doing things the C way and trying to mix that with C++. If you're going to use std::vector to store your objects, then you have to decide whether you want to use standard containers for your data members, or use raw memory and adhere to the "rule of 3", What you want is the latter, but your class fails to implement the rule of 3. – Oaken

https://mcmap.net/q/15703/-what-is-the-rule-of-three – Oaken

Answer 1

Read the description of free:

The behavior is undefined if the value of ptr does not equal a value returned earlier by std::malloc(), std::calloc(), or std::realloc().

Then take a look at your code, pay attention to the comments I added.

void *ptr = malloc(2 * sizeof(long long int) + sizeOfData * sizeof(char));
packageNumber = (long long int*) ptr; // you got this from malloc
allPackages = (long long int*) ((long long int*)ptr + sizeof(long long int)); // the value of this pointer is not equal to anything returned by malloc
data = (char*)((char*)ptr + 2 * sizeof(long long int)); // the value of this pointer is not equal to anything returned by malloc either

And finally in the destructor:

free(data); // was not allocated with malloc -> undefined behaviour
free(packageNumber); // was allocated with malloc -> OK
free(allPackages); // was not allocated with malloc -> undefined behaviour

You try to delete pointers that you didn't get from malloc. This results in undefined behaviour. The error is due to the undefined behaviour. Do realize that free(packageNumber) frees the entire block of memory allocated with malloc. That includes the memory pointed by data and allPackages.

There is an easy rule of thumb: Call free exactly once per every call to malloc/calloc. Same applies to delete+new and delete[]+new[].

Minotaur

It's great explanation. It helps, but there is related to this one more error now. It appers after this. Why my program executed second time destructor in second iterate of loop? – Ramires

@Ramires You're copying the object into a vector. I'd be willing to bet that you forgot to follow the rule of three and didn't implement a correct copy constructor. – Minotaur

A vector makes copies of your object. Since your Package struct cannot be safely copied, by placing it in a vector, all of the bugs associated with copying Package objects become exposed. If you want to store Package objects in a container such as std::vector, you need to ensure it is safely copyable by implementing the rule of 3 (since you have members that are pointers to dynamically allocated memory). – Oaken

Answer 2

allPackages = (long long int*) ((long long int*)ptr + sizeof(long long int));

When you use a long long int pointer(in our case it's ptr after being casted) and you want to advance sizeof(long long int) bytes, you just need to do ptr++;

But I suggest you to rewrite your code and use 3 mallocs instead of one.

Suburbanite

I suggest not writing any malloc calls. Just change data to std::vector<char>, and you get the same results, all without managing any memory. – Oaken