How to prevent ptr_map releasing data when it fail

2019-08-02 16:56发布

问题:

In my question insert or update in ptr_map I wanted to use the following code

typedef boost::ptr_map<const std::string, Entry> KeyEntryMap;
KeyEntryMap m;
void insertOrUpate(const char* key, Entry* entry) {
    std::pair<KeyEntryMap::iterator, bool> re = m.insert(key, entry);
    if (!re.second) {
        m.replace(re.first, entry);
    }
}

However, the following code from ptr_map insert releases (by auto_type) the entry object if an insert fails.

    std::pair<iterator,bool> insert_impl( const key_type& key, mapped_type x ) // strong
    {
        this->enforce_null_policy( x, "Null pointer in ptr_map_adapter::insert()" );
        auto_type ptr( x );                                         // nothrow

        std::pair<BOOST_DEDUCED_TYPENAME base_type::ptr_iterator,bool>
             res = this->base().insert( std::make_pair( key, x ) ); // strong, commit      
        if( res.second )                                            // nothrow     
            ptr.release();                                          // nothrow
        return std::make_pair( iterator( res.first ), res.second ); // nothrow        
    }

I guess this is because ptr_map wants to ensure the ptr to insert is not leaked at the case. Does it mean we have to create a new entry object when calling replace? can we let ptr_map stop releasing it?

The alternative is using find first.

    KeyEntryMap::iterator p = m.find(key);
    if (p == m.end()) {
        m.insert(key, entry);
        return;
    }

    m.replace(p, entry);

Does this have more cost for insert case? It needs searching the map twice.

回答1:

Yes that's expected. Pointer containers take ownership of all pointers you give to them.

Likewise, m.replace(...) frees the element previously existing in the map unless you "catch" it:

KeyEntryMap::auto_type previous = m.replace(re.first, entry);

It's an invariant/contract that leads to code that's easy to get right (in the face of exceptions, not leaking memory etc.)

One devious little trick you could use iff you are willing to allow nullable mapped-types:

typedef boost::ptr_map<const std::string, boost::nullable<Entry> > KeyEntryMap;

struct Demo {
    KeyEntryMap m;

    void insertOrUpate(const char* key, Entry* entry) {
        auto it = m.insert(key, nullptr).first;
        /*auto previous =*/ m.replace(it, entry);
    }
};

Other than that, get the insertion point, and do the check:

void insertOrUpate(const char* key, Entry* entry) {
    auto range = m.equal_range(key);
    if (range.empty()) {
        m.insert(range.end(), key, entry);
    } else {
        m.replace(range.begin(), entry);
    }
}

This way, there's still only one query.

DEMO

Live On Coliru

#include <boost/ptr_container/ptr_map.hpp>
#include <string>

struct Entry {
    int id = [] { static int idgen = 0; return idgen++; }();
};

typedef boost::ptr_map<const std::string, Entry> KeyEntryMap;

struct Demo {
    KeyEntryMap m;

    void insertOrUpate(const char* key, Entry* entry) {
        auto range = m.equal_range(key);
        if (!range.empty()) {
            m.replace(range.begin(), entry);
        } else {
            m.insert(range.end(), key, entry);
        }
    }

    friend std::ostream& operator<<(std::ostream& os, Demo const& d) {
        for (auto e : d.m) os << e.first << ":" << e.second->id << " ";

        return os;
    }
};

#include <iostream> 

int main() {
    Demo d;
    for (auto key : { "a", "a", "c", "a", "b" }) {
        d.insertOrUpate(key, new Entry());
        std::cout << d << "\n";
    }
}

Prints

a:0 
a:1 
a:1 c:2 
a:3 c:2 
a:3 b:4 c:2 


标签: c++ boost