Delete on a Smart Pointer didn't work properly

Question

I've a problem with smart pointer. I created a simple class "Container", that replicate a double linked list. I used reference counting inside a smartp, all ok, but with a simple main.cpp file test I have a problem. The last tail adress is wrong, and it change after a simple pop_front.

Another problem is "delete pv", if I remove from comment tag it caused a segmentation fault. Can anyone help me? Thanks

This is the header file:

#ifndef CONTAINER_H
#define CONTAINER_H
#include<ostream>
using namespace std;

template<class K>
class container {
    friend class iteratore;
private:
    class nodo;
    class smartp {
    public:
        nodo* ptr;
        smartp(nodo*);
        smartp(const smartp&);
        ~smartp();
        smartp& operator=(const smartp&);
        nodo& operator*() const;
        nodo* operator->() const;
        bool operator==(const smartp&) const;
        bool operator!=(const smartp&) const;
    };
    class nodo {
    public:
        K info;
        smartp prev;
        smartp next;
        int refer;
        nodo();
        nodo(const K&, const smartp&, const smartp&);
    };
    smartp head;
    smartp tail;
public:
    container();
   // ~container();
    class iteratore {
        friend class container<K>;
            private:
                smartp punt;
                iteratore(smartp n): punt(n) {}
            public:
                iteratore(): punt(0) {}
                bool operator==(iteratore) const;
                bool operator!=(iteratore) const;
                iteratore& operator++();
                iteratore operator++(int);
                iteratore& operator--();
                iteratore operator--(int);
                K& operator*() const;
                K* operator->() const;
    };
    iteratore begin() const;
    iteratore end() const;
    K& operator[](iteratore) const;

    void push_back(const K&);
    void push_front(const K&);
    void insert(const iteratore&, const K&);
    void printDB();
    void pop_back();
    void pop_front();
    bool isEmpty() const;
    iteratore erase(iteratore);
};


// INIZIO SMARTP
template<class K>
container<K>::smartp::smartp(nodo* p = 0): ptr(p) {
    if(ptr) ptr->refer++;
}

template<class K>
container<K>::smartp::smartp(const smartp& s): ptr(s.ptr) {
    if(ptr) ptr->refer++;
}

template<class K>
container<K>::smartp::~smartp() {
    if(ptr != 0) {
        ptr->refer--;
        if(ptr->refer == 0) {
            cout << " ~smartp ";
            delete ptr;
        }
    }
}

template<class K>
typename container<K>::smartp& container<K>::smartp::operator=(const smartp& s) {
    if(this != &s) {
        nodo* t = ptr;
        ptr = s.ptr;
        if(ptr) ptr->refer++;
        if(t) {
            t->refer--;
            if(t->refer == 0) delete t;
        }
    }
    return *this;
}

template<class K>
typename container<K>::nodo& container<K>::smartp::operator*() const {
    return *ptr;
}

template<class K>
typename container<K>::nodo* container<K>::smartp::operator->() const {
    return ptr;
}

template<class K>
bool container<K>::smartp::operator==(const smartp& p) const {
    return ptr == p.ptr;
}

template<class K>
bool container<K>::smartp::operator!=(const smartp& p) const {
    return ptr != p.ptr;
}

// FINE SMARTP

template<class K>
container<K>::nodo::nodo(const K& i, const smartp& p, const smartp& n): info(i), prev(p), next(n), refer(0) {}

template<class K>
container<K>::nodo::nodo(): refer(0) {}


template<class K>
container<K>::container(): head(0), tail(0) {}
/*
template<class K>
container<K>::~container() {
    int i = 0;
    while(!isEmpty()) {
        cout << " " << i;
        pop_front();
        cout << " "  << i++;
    }
}
*/

template<class K>
void container<K>::push_back(const K& k) {
    nodo* aux = new nodo(k, tail, 0);
    if(isEmpty()) {   // lista vuota
        head = tail = aux;
    } else {
        tail->next = aux;

        tail = aux;
    }
    tail->refer--;
}

template<class K>
void container<K>::push_front(const K& k) {
    nodo* aux = new nodo(k, 0, head);
    if(isEmpty()) {   // lista vuota
        head = tail = aux;
    } else {
        head->prev = aux;
        head = aux;
    }
    head->refer--;
}

template<class K>
void container<K>::insert(const iteratore& it, const K& k) { // inserisce k prima dell'elemento puntato da it
    nodo* aux = new nodo(k, it.punt->prev, it.punt);
    if(it.punt->prev != 0) { // se it ha un nodo precedente (cioè se it non è la testa)
        it.punt->prev->next = aux;
    } else {
        head = aux;
    }    
    it.punt->prev = aux;
}

template<class K>
void container<K>::printDB() {
    cout << "/**/IND-H: " << &head.ptr->info << endl;
    smartp aux = head;
    while(aux.ptr != 0) {
        cout << aux.ptr->info;
        cout << " *IND:" << aux.ptr  << "* "<<endl;
        aux = aux.ptr->next;
    }
    cout << "/**/IND-T: " << &tail.ptr->info;
    cout << endl << endl;
}

template<class K>
void container<K>::pop_back() {
    if(!isEmpty()) {
        smartp aux = tail->prev;
        delete &tail;
        tail = aux;
        if(aux!=0) tail->next = 0;
        else head = aux;
    }
}

template<class K>
void container<K>::pop_front() {
    if(!isEmpty()) {
        smartp aux = head->next;
        delete &head;
        head = aux;
        if(aux!=0) head->prev = 0;
        else tail = aux;


    }
}

template<class K>
bool container<K>::isEmpty() const {
    return head == 0;
}

template<class K>
typename container<K>::iteratore container<K>::erase(iteratore it) {
    iteratore aux = it.punt->next;
    if(it.punt->prev == 0) pop_front(); // it = head
    else if(it.punt->next == 0) pop_back(); // it = tail
    else {
        it.punt->next->prev = it.punt->prev;
        it.punt->prev->next = it.punt->next;
        delete &(it.punt);
    }
    return aux;
}



template<class K>
bool container<K>::iteratore::operator==(iteratore i) const {
    return punt == i.punt;
}

template<class K>
bool container<K>::iteratore::operator!=(iteratore i) const {
    return punt != i.punt;
}

template<class K>
typename container<K>::iteratore& container<K>::iteratore::operator++() {
    if(punt != 0) punt = punt->next;
    return *this;
}

template<class K>
typename container<K>::iteratore container<K>::iteratore::operator++(int) {
    iteratore aux = *this;
    if(punt != 0) punt = punt->next;
    return aux;
}

template<class K>
typename container<K>::iteratore& container<K>::iteratore::operator--() {
    if(punt != 0) punt = punt->prev;
    return *this;
}

template<class K>
typename container<K>::iteratore container<K>::iteratore::operator--(int) {
    iteratore aux = *this;
    if(punt != 0) punt = punt->prev;
    return aux;
}

template<class K>
K& container<K>::iteratore::operator*() const {
    return punt->info;
}

template<class K>
K* container<K>::iteratore::operator->() const {
    return &(punt)->info;
}

template<class K>
typename container<K>::iteratore container<K>::begin() const {
    iteratore aux;
    aux.punt = head; // per amicizia
    return aux;
}

template<class K>
typename container<K>::iteratore container<K>::end() const {
    iteratore aux;
    aux.punt = 0; // per amicizia
    return aux;
}

template<class K>
K& container<K>::operator[](typename container<K>::iteratore it) const {
    return (it.punt)->info; // per amicizia
}



#endif // CONTAINER_H

and this the test main file:

#include <iostream>
#include"container.h"
using namespace std;

int main()
{
    container<int>* pv = new container<int>;
    cout << "Inserisco in coda: 9" << endl;
    pv->push_back(9);
    pv->printDB();


    cout << "Inserisco in coda: 5, 2" << endl;
    pv->push_back(5);
    pv->push_back(2);
    pv->printDB();

    cout << "Inserisco in testa: 1, 7" << endl;
    pv->push_front(1);
    pv->push_front(7);
    pv->printDB();

    cout << "Faccio pop_back()" << endl;
    pv->pop_back();
    pv->printDB();

    cout << "Faccio pop_front()" << endl;
    pv->pop_front();
    pv->printDB();

//  delete pv;

    cout <<"FINE"<< endl;


}

Answer 1

This construction is incorrect and has undefined behaviour:

delete &tail;
// ...
delete &head;

because &tail and &head weren't dynamically allocated using new.

Since you have smart pointers you shouldn't be using delete at all outside of the destructor of the smart pointer.