Помогите мне сделать этот код безопасным от исключений

Question

Итак, у меня есть этот библиотечный код, см ...

class Thing
{
public:

    class Obj 
    {
     public:
        static const int len = 16;

        explicit Obj(char *str)
        {
            strncpy(str_, str, len);
        }

        virtual void operator()() = 0;

    private:
        char str_[len];
    };

    explicit Thing(vector<Obj*> &objs) : objs_(objs) {}

    ~Thing() {
        for(vector<Obj*>::iterator i = objs_.begin(); i != objs_.end(); ++i) {
            delete *i;
        }
    }

private:
    vector<Obj*> objs_;
}

И в моем коде клиента ...

   class FooObj : public Thing::Obj
    {
        virtual void operator()() {
            //do stuff
        }
    }

    class BarObj : public Thing::Obj
    {
        virtual void operator()() {
            //do different stuff
        }
    }

vector<Objs*> objs;
int nStructs = system_call(*structs);
for(int i = 0; i < nStructs; i++) {
    objs.push_back(newFooObj(structs[i].str));
}
objs.push_back(newBarObj("bar1");
objs.push_back(newBarObj("bar2");

Thing thing(objs);
// thing does stuff, including call Obj::()() on the elements of the objs_ vector

Я застрял в безопасности исключений. В настоящее время, если какой-либо из конструкторов Obj сгенерирует или Thing сгенерирует, Objs уже в векторе будет утечка Вектор должен содержать указатели на Objs, потому что они используются полиморфно. И мне нужно обработать любые исключения здесь, потому что это вызывается из более старой кодовой базы, которая не знает исключений.

На мой взгляд, у меня есть следующие варианты:

1. Оберните код клиента в гигантский блок try и очистите вектор в блоке catch.
2. Поместите блоки try вокруг всех распределений, блоки catch которых вызывают общую функцию очистки.
3. Какая-то умная идея, основанная на RAII, о которой я еще не задумывался.
4. Punt. Реально, если конструкторы выкидывают, приложение в любом случае собирается загореться, но я бы хотел справиться с этим более изящно.

Answer 1

Взгляните на boost::ptr_vector

Answer 2

Поскольку ваш деструктор Thing уже знает, как очистить вектор, вы в большей степени подходите к решению RAII . Вместо создания вектора объектов Objs и последующей передачи его конструктору Thing, вы можете инициализировать Thing пустым вектором и добавить функцию-член для добавления новых объектов Objs по указателю на вектор.

Таким образом, если сработает конструктор Obj, компилятор автоматически вызовет деструктор Thing, должным образом уничтожив все Objs, которые уже были выделены.

Конструктор вещи становится неактивным:

explicit Thing() {}

Добавить push_back участника:

void push_back(Obj *new_obj) { objs_.push_back(new_obj); }

Тогда код выделения в вашем клиенте становится:

Thing thing(objs);
int nStructs = system_call(*structs);
for(int i = 0; i < nStructs; i++) {
    thing.push_back(newFooObj(structs[i].str));
}
thing.push_back(newBarObj("bar1");
thing.push_back(newBarObj("bar2");

Как предложил другой автор, умный тип указателя внутри вашего вектора также будет хорошо работать. Только не используйте STL auto_ptr; он не следует обычной семантике копирования и поэтому не подходит для использования в контейнерах STL. shared_ptr, предоставленный Boost и готовящимся C ++ 0x, подойдет.

Answer 3

Ответ 3 - Используйте умные указатели вместо Obj * в ваших векторах. Я бы предложил boost :: shared_ptr .

Answer 4

Вектор Obj может быть очень плох для производительности, поскольку вектор может перемещать объект при изменении размера и должен копировать их все Указатели на объекты лучше.

Я использовал Pointainers , который будет делать то, что вам нужно. Вот оригинальный код.

/*
 * pointainer - auto-cleaning container of pointers
 *
 * Example usage:
 * {
 *     pointainer< std::vector<int*> > v;
 *     // v can be manipulated like any std::vector<int*>.
 *
 *     v.push_back(new int(42));
 *     v.push_back(new int(17));
 *     // v now owns the allocated int-s
 *
 *     v.erase(v.begin());
 *     // frees the memory allocated for the int 42, and then removes the
 *     // first element of v.
 * }
 * // v's destructor is called, and it frees the memory allocated for
 * // the int 17.
 *
 * Notes:
 * 1. Assumes all elements are unique (you don't have two elements
 *    pointing to the same object, otherwise you might delete it twice).
 * 2. Not usable with pair associative containers (map and multimap).
 * 3. For ANSI-challenged compilers, you may want to #define
 *    NO_MEMBER_TEMPLATES.
 *
 * Written 10-Jan-1999 by Yonat Sharon <yonat@@ootips.org>
 * Last updated 07-Feb-1999
 *
 * Modified June 9, 2003 by Steve Fossen
 *  -- to fix g++ compiling problem with base class typenames
 */

#ifndef POINTAINER_H
#define POINTAINER_H

#ifdef NO_MEMBER_TEMPLATES
    #include <functional> // for binder2nd
#endif

template <typename Cnt>
class pointainer : public Cnt
{
public:
    // sf - change to fix g++ compiletime errors
#ifdef USE_USING_NOT_TYPEDEF
    // I get compile errors with this
    using typename Cnt::size_type;
    using typename Cnt::difference_type;
    using typename Cnt::reference;
    using typename Cnt::const_reference;
    using typename Cnt::value_type;
    using typename Cnt::iterator;
    using typename Cnt::const_iterator;
    using typename Cnt::reverse_iterator;
    using typename Cnt::const_reverse_iterator;
#else
    // this way works
    typedef typename Cnt::size_type                 size_type;
    typedef typename Cnt::difference_type           difference_type;
    typedef typename Cnt::reference                 reference;
    typedef typename Cnt::const_reference           const_reference;
    typedef typename Cnt::value_type                value_type;
    typedef typename Cnt::iterator                  iterator;
    typedef typename Cnt::const_iterator            const_iterator;
    typedef typename Cnt::reverse_iterator          reverse_iterator;
    typedef typename Cnt::const_reverse_iterator    const_reverse_iterator;
#endif

    typedef pointainer< Cnt > its_type;

    pointainer()                            {}
    pointainer( const Cnt &c )   : Cnt( c ) {}

    its_type &operator=( const Cnt &c )      { Cnt::operator=( c ); return *this;        }
    ~pointainer()                            { clean_all();                              }

    void clear()                             { clean_all();   Cnt::clear();              }
    iterator erase( iterator i )             { clean( i );    return Cnt::erase( i );    }
    iterator erase( iterator f, iterator l ) { clean( f, l ); return Cnt::erase( f, l ); }

    // for associative containers: erase() a value
    size_type erase( const value_type& v )
    {
        iterator i = find( v );
        size_type found( i != end() ); // can't have more than 1
        if( found )
            erase( i );
        return found;
    }

    // for sequence containers: pop_front(), pop_back(), resize() and assign()
    void pop_front()    { clean( begin() ); Cnt::pop_front();                 }
    void pop_back()     { iterator i( end() ); clean( --i ); Cnt::pop_back(); }

    void resize( size_type s, value_type c = value_type() )
    {
        if( s < size() )
            clean( begin() + s, end() );
        Cnt::resize( s, c );
    }

#ifndef NO_MEMBER_TEMPLATES
    template <class InIter> void assign(InIter f, InIter l)
#else
    void assign( iterator f, iterator l )
#endif
    {
        clean_all();
        Cnt::assign( f, l );
    }

#ifndef NO_MEMBER_TEMPLATES
    template <class Size, class T> void assign( Size n, const T& t = T() )
#else
    void assign( size_t n, const value_type& t = value_type() )
#endif
    {
        clean_all();
        Cnt::assign( n, t );
    }

    // for std::list: remove() and remove_if()
    void remove( const value_type& v )
    {
        clean( std::find( begin(), end(), v ));
        Cnt::remove( v );
    }

#ifndef NO_MEMBER_TEMPLATES
    template <class Pred>
#else
    typedef std::binder2nd< std::not_equal_to< value_type > > Pred;
#endif
    void remove_if(Pred pr)
    {
        for( iterator i = begin(); i != end(); ++i )
            if( pr( *i ))
                clean( i );

        Cnt::remove_if( pr );
    }

private:
    void clean( iterator i )                { delete *i; *i = 0;            } // sf add *i = NULL so double deletes don't fail
    void clean( iterator f, iterator l )    { while( f != l ) clean( f++ ); }
    void clean_all()                        { clean( begin(), end() );      }

    // we can't have two pointainers own the same objects:
    pointainer( const its_type& ) {}
    its_type& operator=( const its_type& ) { return  NULL;} // sf - return null to remove no return value warning..
};

#endif // POINTAINER_H

Answer 5

Хммм. Мне очень нравится идея коммодора Йегера; он аккуратно проясняет некоторые забавные запахи, которые этот код давал мне. Я не хочу вводить библиотеки Boost на этом этапе; это несколько консервативная кодовая база, и я бы предпочел внести ее в XXI век, корчась и жаловаться, чем пинать и кричать.

Answer 6

Не изменяя тип, хранящийся в objs, на тип, который можно копировать и управлять самим Obj *, вам нужно добавить блок try / catch для очистки objs в случае возникновения исключения. Самый простой способ сделать это:

vector<Obj *> objs;

try {...}
catch (...) 
{
    // delete all objs here
    throw;
}

Хотя вы все равно захотите очистить objs, если исключение также не выдается.

Answer 7

Вместо вектора указателей на Obj вы всегда можете использовать вектор Obj. В этом случае вам нужно убедиться, что вы можете безопасно скопировать Obj (опасно, когда он содержит указатели). Поскольку ваш Obj содержит только массив символов фиксированного размера, он должен быть безопасным.