Clang error: pointer being freed was not allocated

Question

I wrote this implementation of Linked list:

template<typename T>  // implementation: Linked_list 
class Linked_list {  
    private:
        Node<T>* head;
        Node<T>* tail;
        Node<T>* current;
        int size;


        void init() 
        {
            head = tail = current = new Node<T>();
            size = 0;
        }


        Node<T>* search_previous()
        {
            if (current == head) {
                return nullptr;
            }
            Node<T>* previous_node = head;
            while (previous_node->next != current) {
                previous_node = previous_node->next;
            }
            return previous_node;
        }


    public:
        Linked_list() 
        {
            init();
        }


        void clear() 
        {
            while (head != nullptr) {
                current = head;
                head = head->next;
                delete current;

            }
            init();
        }


        ~Linked_list() 
        {
            clear();
            delete head;
        }


        void append(T p_element) 
        {
            tail->next = new Node<T>(p_element);
            tail = tail->next;
            ++size;
        }


        void insert(T p_element)
        {
            current->next = new Node<T>(p_element, current->next);
            if (current == tail) {
                tail = tail->next;
            }
            ++size; 
        }


        T remove() 
        {
            if (current->next == nullptr) {
                throw std::runtime_error("No element to remove");
            }
            T removed_element = current->next->element;
            Node<T>* temporary_pointer = current->next;
            current->next = current->next->next;
            if (temporary_pointer == tail) {
                tail = current;
            }
            delete temporary_pointer;
            --size;
            return removed_element;
        }


        T get_element() 
        {
            if (current->next == nullptr) {
                throw std::runtime_error("No element to get");
            }
            return current->next->element;
        }


        void go_to_start() 
        {
            current = head;
        }


        void go_to_end() 
        {
            current = tail;
        }


        void go_to_pos(int p_pos) 
        {
            if ((p_pos < 0) || (p_pos >= size)) {
                throw std::runtime_error("Index out of bounds");
            }
            current = head;
            for (int index = 0; index < p_pos; ++index) {
                current = current->next;
            }
        }


        void next() 
        {
            if (current != tail) {
                current = current->next;
            }
            else {
                throw std::runtime_error("There's no next positition");
            }
        }


        void previous() 
        {
            if (current != head) {
                current = search_previous();
            }
            else {
                throw std::runtime_error("There's no previous positition");
            }
        }


        int get_pos() 
        {
            int pos = 0;
            Node<T>* temporary_pointer = head;
            while (temporary_pointer != current) {
                temporary_pointer = temporary_pointer->next;
                ++pos;
            }
            return pos;
        }


        int get_size() 
        {
            return size;
        }


        void concat(Linked_list<T> p_list)
        {
            for (p_list.go_to_start(); p_list.get_pos() < p_list.get_size(); p_list.next()) {
                append(p_list.get_element());
            }
        }

};

And here's the node:

template<typename T>
class Node {
    public:
        T element;
        Node<T>* next;


        Node(T p_element, Node<T>* p_next = nullptr)
        {
            element = p_element;
            next = p_next;
        }


        Node(Node<T>* p_next = nullptr)
        {
            next = p_next;
        }
};

The problem that I have is that when I try to use the method concat I get this message from Clang:

proofs(13417,0x7fff7bb9f000) malloc: * error for object 0x7fe10b603170: pointer being freed was not allocated * set a breakpoint in malloc_error_break to debug Abort trap: 6

What can I do for fix it?

`void concat(Linked_list p_list)` Passing by value, and your `Linked_list` class does not follow the rule of 3. That is a recipe for disaster. — PaulMcKenzie, Aug 18 '15 at 00:25
Declare the parameter as a reference, then the copy will not be done. However, it s only a band-aid approach to the real issue, and that is that the class shouldn't be copied. You should turn off copying (you can do this in one or two ways, depending on whether you're using C++ 11 or not) or provide the appropriate copy constructor and assignment operators for your class. — PaulMcKenzie, Aug 18 '15 at 00:39

score 1 · Accepted Answer · edited May 23 '17 at 12:14

The obvious error is this:

    void concat(Linked_list<T> p_list)

You are passing a Linked_list by value. That means that a temporary copy of the linked list is created and destroyed. Since the destructor deletes the memory, it is also deleting the memory of the linked list that you are making the copy of.

Since your Linked_list class does not have a user-defined assignment operator or copy constructor to handle the members that point to dynamically allocated memory, the class cannot be safely copied (if you debugged, you should have seen that a destructor was called that you didn't expect, and that is the temporary being destroyed, thus corrupting the original object).

To prevent this, either pass by reference (not value),

   void concat(Linked_list<T>& p_list)

or provide appropriate copy constructor and assignment operator.

See What is the rule of Three

Clang error: pointer being freed was not allocated

1 Answers1