-2

In my implementation of ArrayList I have a method for resizing the array. It looks like this:

template<typename T>
int ArrayList<T>::changeSize(int newsize)
{
    T* tmp = (T*)(T*)realloc(internal_array,sizeof(T)*newsize);
    if(tmp == NULL)
    {
        return 0;
    }
    internal_array=tmp; 
    Capacity = newsize;
    return 1;
}

This part of code causes a memory leak when using it, any idea, what could be wrong here? EDIT: Handling realloc being NULL still didn't help, updated the code and added adress sanitizer output:

================================================================= ==4615==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 16 byte(s) in 1 object(s) allocated from:

#0 0x7f0c9d1b5f40 in realloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdef40)
#1 0x557b3b693fb2 in ArrayList<int>::changeSize(int) (/media/mdvorak/Maxtor/Vysoká Škola/PA1/progtest5/a.out+0x2fb2)
#2 0x557b3b6939c4 in ArrayList<int>::Add(int) (/media/mdvorak/Maxtor/Vysoká Škola/PA1/progtest5/a.out+0x29c4)
#3 0x557b3b692671 in readInput(ArrayList<int>*) (/media/mdvorak/Maxtor/Vysoká Škola/PA1/progtest5/a.out+0x1671)
#4 0x557b3b693339 in main (/media/mdvorak/Maxtor/Vysoká Škola/PA1/progtest5/a.out+0x2339)
#5 0x7f0c9cd07b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)

Direct leak of 4 byte(s) in 1 object(s) allocated from:

#0 0x7f0c9d1b5f40 in realloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdef40)
#1 0x557b3b693fb2 in ArrayList<int>::changeSize(int) (/media/mdvorak/Maxtor/Vysoká Škola/PA1/progtest5/a.out+0x2fb2)
#2 0x557b3b693e0b in ArrayList<int>::Remove(int) (/media/mdvorak/Maxtor/Vysoká Škola/PA1/progtest5/a.out+0x2e0b)
#3 0x557b3b692d07 in findMinDistances(ArrayList<int>) (/media/mdvorak/Maxtor/Vysoká Škola/PA1/progtest5/a.out+0x1d07)
#4 0x557b3b6934a3 in main (/media/mdvorak/Maxtor/Vysoká Škola/PA1/progtest5/a.out+0x24a3)
#5 0x7f0c9cd07b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)

SUMMARY: AddressSanitizer: 20 byte(s) leaked in 2 allocation(s).

Swordfish
  • 12,971
  • 3
  • 21
  • 43
Michal Dvořák
  • 197
  • 3
  • 10
  • 1
    How do you determine that it leaks? What do you do if `realloc()` fails? Curious: what is `T`? – Swordfish Nov 21 '18 at 02:08
  • 3
    The correct way to call realloc is to save the return value in a temporary variable and check it for NULL. That way if realloc has failed, you haven't lost your original memory, like in this answer: https://stackoverflow.com/a/44789446/1212725 – bruceg Nov 21 '18 at 02:21
  • @Swordfish, it's a generic type, but in my program it's an integer (`int`) – Michal Dvořák Nov 21 '18 at 11:03
  • Updated my question, checking the realloc is `NULL` didn't help. – Michal Dvořák Nov 21 '18 at 11:20
  • Are you sure that `internal_array` holds either `NULL` or a valid pointer value returned by `malloc()`, `calloc()`, `realloc(NULL, ...)` before you call `realloc()` with it as the firest parameter? – Swordfish Nov 21 '18 at 13:42
  • From the error messages you now added, it is clear that this is C++-code, not C. \*grrrrr\* And whats the implementation of `ArrayList<>`, `readInput()`, `findMinDistances()`? and how does a `main()` look like to reproduce? – Swordfish Nov 21 '18 at 13:44
  • At least on Linux, compile with all warnings and debug info (`g++ -Wall -Wextra -g`) and try [valgrind](http://valgrind.org/). BTW, avoid raw pointers in C++11 (and avoid `realloc` or `malloc`). Use smart pointers and C++ containers. Also, show a [MCVE] in your question – Basile Starynkevitch Nov 21 '18 at 13:50

1 Answers1

2

You are reading the error wrong:

Direct leak of 16 byte(s) in 1 object(s) allocated from:

#0 0x7f0c9d1b5f40 in realloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdef40)
#1 0x557b3b693fb2 in ArrayList<int>::changeSize(int) (/media/mdvorak/Maxtor/Vysoká Škola/PA1/progtest5/a.out+0x2fb2)
#2 0x557b3b6939c4 in ArrayList<int>::Add(int) (/media/mdvorak/Maxtor/Vysoká Škola/PA1/progtest5/a.out+0x29c4)
#3 0x557b3b692671 in readInput(ArrayList<int>*) (/media/mdvorak/Maxtor/Vysoká Škola/PA1/progtest5/a.out+0x1671)
#4 0x557b3b693339 in main (/media/mdvorak/Maxtor/Vysoká Škola/PA1/progtest5/a.out+0x2339)
#5 0x7f0c9cd07b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)

It clearly says that the memory allocated at ArrayList<int>::changeSize(int) is the memory that leaks. It does not say what operation caused it to leak.

This means that changeSize(int) is ok. The problem lies elsewhere. You must make sure that the rule of 5 is followed:

  1. Destructor frees the memory.
  2. Copy constructor copies contents of internal_array, not the pointer.
  3. Assignment operator copies the contents, and not the pointer. Consider the copy-and-swap idiom.
  4. Move constructor copies the pointer, but nullifies the other object.
  5. Move assignment swaps the pointers, or assigns one pointer and frees the other.

The bug is probably due to missing, or wrong, one of the above. A good way to make it easier to catch such bugs during compile time, is to use std::unique_ptr with a custom deleter instead of freeing manually.

Please note that your code will crash and/or leak memory with anything complex, like T=vector or T=big num. You can add static_assert(std::is_trivial<T>::value); to make sure it is not instantiated with, e.g., std::map.

Michael Veksler
  • 8,217
  • 1
  • 20
  • 33