Search code examples
c++mallocfreerealloc

How to free memory for an array allocated by malloc?


I have 2 variants of code:

1st:

void PrintMem(const int* memarr,const size_t size) {
    for (size_t index = 0; index < size; ++index) {
        std::cout << '<'<<(index+1)<<"> "s<<*(memarr + index) << std::endl;
    }
}

void FillMem(int* memarr, const size_t size) {
    srand(time(0));
    for (size_t index = 0; index < size; ++index) {
        *(memarr + index) = rand() % 100;
    }
}

int main() {
    const int size_iter = 10000000;
    int n = 30;
    int* ptr = nullptr;
    int size = size_iter;
    for (int i = 1; i <= n; ++i) {
        size = size_iter * i;
        if (i == 1) {
            ptr = (int*)malloc(size * sizeof(int));
        }
        else {
            ptr = (int*)realloc(ptr, size * sizeof(int));    
        }
        if (ptr == nullptr) {
            printf("memory allocation error\n");
            break;
        }
        std::cout << '[' << i << ']';
        printf(" address: %p", (void*)ptr);
        std::cout << ", size: "s << size;
        std::cout << " *********************" << std::endl;
        FillMem(ptr, size);
        //PrintMem(ptr, size);

    }
    if (ptr != nullptr) {
        free(ptr);
    }
}

2nd:

void PrintMem(const int* memarr,const size_t size) {
    for (size_t index = 0; index < size; ++index) {
        std::cout << '<'<<(index+1)<<"> "s<<*(memarr + index) << std::endl;
    }
}

void FillMem(int* memarr, const size_t size) {
    srand(time(0));
    for (size_t index = 0; index < size; ++index) {
        *(memarr + index) = rand() % 100;
    }
}

int main() {
    const int size_iter = 10000000;
    int n = 30;
    int* ptr = nullptr;
    int size = size_iter;
    for (int i = 1; i <= n; ++i) {
        size = size_iter * i;
        int* new_ptr = nullptr;
        if (i == 1) {
            new_ptr = (int*)malloc(size * sizeof(int));
        }
        else {
            new_ptr = (int*)realloc(ptr, size * sizeof(int));    
        }
        if (new_ptr == nullptr) {
            printf("memory allocation error\n");
            break;
        }
        ptr = new_ptr;
        std::cout << '[' << i << ']';
        printf(" address: %p", (void*)ptr);
        std::cout << ", size: "s << size;
        std::cout << " *********************" << std::endl;
        FillMem(ptr, size);
        //PrintMem(ptr, size);

    }
    if (ptr != nullptr) {
        free(ptr);
    }
}

What is the correct way to free the array memory?

if (ptr != nullptr) {
    free(ptr);
}

or:

if (ptr != nullptr) {
    for (int i = 0; i < size; ++i) {
        free((ptr + i));
   }
   free(ptr);
}

I tried both variants.

The 2nd variant with int* new_ptr will be better I think, because it will at least keep the previous iteration of memory resize.

I just need to know how to optimize this, and if it is correct to free only ptr or do I need to free each memory block?


Solution

  • You are calling malloc() only one time to create the array, and the calling realloc() multiple times to reallocate the array. There is only 1 array, so you need to call free() only one time to free that array. Do not try to free() the individual elements, as they were not malloc'ed individually. One free() per successful malloc()/realloc().

    Also, you don't need to check for nullptr before calling free(), as it already handles that internally.

    Also, if realloc() fails, the original array is untouched, but you are overwriting your ptr variable unconditionally, so you will leak the existing array. You need to check for a realloc() failure before you reassign your ptr variable.

    On a side note, some other minor nitpicks with your remaining code:

    • You should use memarr[index] instead of *(memarr + index).

    • don't call srand() multiple times. Call it one time in main().

    • "> "s should be just "> ", there is no need to force it to std::string just to print it, as operator<< can handle string literals just fine (as evident in some of your other prints).

    • you shouldn't mix printf() with std::cout. Stick with one or the other.

    Try something more like this instead:

    void PrintMem(const int* memarr, const size_t size) {
        for (size_t index = 0; index < size; ++index) {
            std::cout << '<' << (index+1) << "> " << memarr[index] << '\n';
        }
    }
    
    void FillMem(int* memarr, const size_t size) {
        for (size_t index = 0; index < size; ++index) {
            memarr[index] = rand() % 100;
        }
    }
    
    int main() {
        srand(time(0));
        const int size_iter = 10000000;
        int n = 30;
        int* ptr = nullptr;
        for (int i = 1; i <= n; ++i) {
            int size = size_iter * i;
            if (i == 1) {
                ptr = static_cast<int*>(malloc(size * sizeof(int)));
                if (ptr == nullptr) {
                    std::cerr << "memory allocation error\n";
                    break;
                }
            }
            else {
                int *new_ptr = static_cast<int*>(realloc(ptr, size * sizeof(int)));
                if (new_ptr == nullptr) {
                    std::cerr << "memory reallocation error\n";
                    break;
                }
                ptr = new_ptr;
            }
            std::cout << '[' << i << ']';
            std::cout << " address: " << static_cast<void*>(ptr);
            std::cout << ", size: " << size;
            std::cout << " *********************\n";
            FillMem(ptr, size);
            //PrintMem(ptr, size);
        }
        free(ptr);
    }
    

    That being said, you really shouldn't be using malloc/realloc() in C++ at all. Use std::vector instead, and let it handle the memory for you, eg:

    #include <vector>
    
    void PrintMem(const std::vector<int> &arr) {
        for (size_t index = 0; index < arr.size(); ++index) {
            std::cout << '<' << (index+1) << "> " << memarr[index] << '\n';
        }
    }
    
    void FillMem(std::vector<int> &arr) {
        for (size_t index = 0; index < arr.size(); ++index) {
            memarr[index] = rand() % 100;
        }
    }
    
    int main() {
        srand(time(0));
        const int size_iter = 10000000;
        int n = 30;
        std::vector<int> arr;
        for (int i = 1; i <= n; ++i) {
            int size = size_iter * i;
            arr.resize(size);
            std::cout << '[' << i << ']';
            std::cout << " address: " << static_cast<void*>(arr.data());
            std::cout << ", size: " << size;
            std::cout << " *********************\n";
            FillMem(arr);
            //PrintMem(arr);
        }
    }
    

    You should also consider using a C++-style random-number generator from the <random> library, instead of using the C-style rand().

    Also, consider using range-for loops, and standard algorithms like std::for_each(), std::generate(), etc.

    In short, avoid using C-isms in C++ whenever possible. C and C++ may have had a common heritage at one time, but they have evolved into very different languages.