ベクトルから項目を削除するときのValgrindエラー

Question

これは大部分の人のように見えるかもしれません。しかし、私は多くの時間を費やしてこれを修正しました。 stackoverflowやその他のコーディングサイトで多くのソリューションを実装しました。最後に私はそれを修正することができましたが、私は古い実装で何が間違っていたのか分かりません。

私の古いコード、新しいコード、単体テスト、およびvalgrindエラーを見ている正確なエラーの原因を調べてください。

注：

• 私はユニットテスト（Googleのテストフレームワーク）から自分のコードをテストしていました。 C++ 11
• m_queue_を使用して
• コンパイルがのstd ::ベクトル
• 使用されるGoogleのC++コーディング標準
ある

テスト：

• キューが2つのSAPAの項目があります（作成しました新しい演算子で）
• そのIDで最初のアイテムを削除しています（キューには現在1つしかありません）
そのID
• セカンド削除によって残された唯一の項目 を削除する
• は私のキューアイテムの基本クラスを項目ここで

のm_id_をされてアクセスvalgrindのエラーを与えているようだ

class Item { 
public: 
    Item() { 
    type = Type::kInvalid; 
    } 

    virtual ~Item() {} 

    Type type; 
    string m_id_ = string(""); 
}; 

ここに子クラスがあります

class SAPA : public Item { 
public: 
    SAPA() { Item::type = Type::kSAPA; } 
    ~SAPA() {} 
}; 

特定の基準（RemoveIf）を満たしていれば、アイテムを削除するために古いコードが使用されます。 VALGRIND問題を引き起こしました。

This was proposed as a correct way to remove items from a vector in many posts.

void Queue::RemoveItems(const string& id) const { 
    vector<Item*>::iterator it = m_queue_.begin(); 
    while (it != m_queue_.end()) { 
    Item* item = *it; 
    if (item == nullptr) { 
     continue; 
    } 

    if (RemoveIf(item, id)) { 
     delete item; 
     item = nullptr; 
     it = m_queue_.erase(it); 
    } else { 
     ++it; 
    } 
    } 
} 

RemoveIf機能

bool Queue::RemoveIf(Item* item, 
        const string& id) const { 
**cout << id.c_str() << endl; <--- seems to cause the invalid read** 

    if (item->m_id_.compare(id) == 0) { 
    return true; 
    } 

    return false; 
} 

valgrindの出力は、サイズ8の不正な読み取りが申し訳ありませんが、これはいくつかのプロジェクト固有の名前が含まれていると言います。 FIXED valgrindの問題以下

> ==21919== Invalid read of size 8 
> ==21919== at 0x5880B90: std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::c_str() const (in 
> /usr/lib64/libstdc++.so.6.0.21) 
> ==21919== by 0xEC416C: Queue::RemoveIf(network::multiplexer::Item*, blf::String const&) const (network_multiplexer_queue.cc:99) 
> ==21919== by 0xEC3FFB: Queue::RemoveItems(blf::String const&) const (network_multiplexer_queue.cc:85) 
> ==21919== by 0xEC4FDC: Queue::OnTimer() const (network_multiplexer_queue.cc:228) 
> ==21919== by 0xFB05E0: (anonymous namespace)::NetworkMultiplexerTest_sapaTimeout_shouldBeHandled_successfully_Test::TestBody() 
> (network_multiplexer_comm_test.cc:1201) 
> ==21919== by 0x1232186: void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, 
> void>(testing::Test*, void (testing::Test::*)(), char const*) (in 
> /home/sajith/cioffi/cioffi-linux/build/unit_tests) 
> ==21919== by 0x122C547: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, 
> void>(testing::Test*, void (testing::Test::*)(), char const*) (in 
> /home/sajith/cioffi/cioffi-linux/build/unit_tests) 
> ==21919== by 0x12124B7: testing::Test::Run() (in /home/sajith/cioffi/cioffi-linux/build/unit_tests) 
> ==21919== by 0x1212D99: testing::TestInfo::Run() (in /home/sajith/cioffi/cioffi-linux/build/unit_tests) 
> ==21919== by 0x1213444: testing::TestCase::Run() (in /home/sajith/cioffi/cioffi-linux/build/unit_tests) 
> ==21919== by 0x1219F2E: testing::internal::UnitTestImpl::RunAllTests() (in 
> /home/sajith/cioffi/cioffi-linux/build/unit_tests) 
> ==21919== by 0x1233583: bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, 
> bool>(testing::internal::UnitTestImpl*, bool 
> (testing::internal::UnitTestImpl::*)(), char const*) (in 
> /home/sajith/cioffi/cioffi-linux/build/unit_tests) 
> ==21919== Address 0x6d24a00 is 16 bytes inside a block of size 112 free'd 
> ==21919== at 0x4C2A131: operator delete(void*) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) 
> ==21919== by 0xED3991: SAPA::~SAPA() (network_multiplexer_queue_item.h:82) 
> ==21919== by 0xEC4045: Queue::RemoveItems(blf::String const&) const (network_multiplexer_queue.cc:86) 
> ==21919== by 0xEC4FDC: OnTimer() const (network_multiplexer_queue.cc:228) 
> ==21919== by 0xFB05E0: (anonymous namespace)::NetworkMultiplexerTest_sapaTimeout_shouldBeHandled_successfully_Test::TestBody() 
> (network_multiplexer_comm_test.cc:1201) 
> ==21919== by 0x1232186: void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, 
> void>(testing::Test*, void (testing::Test::*)(), char const*) (in 
> /home/sajith/cioffi/cioffi-linux/build/unit_tests) 
> ==21919== by 0x122C547: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, 
> void>(testing::Test*, void (testing::Test::*)(), char const*) (in 
> /home/sajith/cioffi/cioffi-linux/build/unit_tests) 
> ==21919== by 0x12124B7: testing::Test::Run() (in /home/sajith/cioffi/cioffi-linux/build/unit_tests) 
> ==21919== by 0x1212D99: testing::TestInfo::Run() (in /home/sajith/cioffi/cioffi-linux/build/unit_tests) 
> ==21919== by 0x1213444: testing::TestCase::Run() (in /home/sajith/cioffi/cioffi-linux/build/unit_tests) 
> ==21919== by 0x1219F2E: testing::internal::UnitTestImpl::RunAllTests() (in 
> /home/sajith/cioffi/cioffi-linux/build/unit_tests) 
> ==21919== by 0x1233583: bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, 
> bool>(testing::internal::UnitTestImpl*, bool 
> (testing::internal::UnitTestImpl::*)(), char const*) (in 
> /home/sajith/cioffi/cioffi-linux/build/unit_tests) 

これは逆方向に反復し、新しいコードで、項目を削除します。あなたの明確化の後

auto it = m_queue_.end(); 
    while (it > m_queue_.begin()) { 
    it--; 
    Item* item = *it; 
    if (item == nullptr) { 
     continue; 
    } 

    if (RemoveIf(item, id)) { 
     delete item; 
     item = nullptr; 
     it = m_queue_.erase(it); 
    } 
    } 

Answer 1

EDIT

、実際の原因はかなり明確であるように思われます。

const string&として「id」を渡していますが、コピーではなく元のオブジェクトへの参照が渡されます。最初のオブジェクトから来ているので、私はあなたの上のどこかでconst string& id = *m_queue_.begin()->m_id_;があると推測しており、次にこれをRemoveIfに渡します。比較は最初のアイテムを削除することが保証されているため、ループ内で発生します。今すぐidは、そのアイテムのデータへの参照がありません。あなたの逆の反復バージョンで動作する理由は、最初のアイテムが最後に削除されるということです。削除した後は、idが表示されます。おそらくidを割り当てるコード行をstring id = *m_queue_.begin()->m_id_;のように変更することができます。その時点でstring&ではなくstringにすることによって、強制的にコピーを作成します。そのコピーの有効期間はスコープの終わりまでです。あなたが見てしなければならない

のEND EDIT

ことの一つは、STDライブラリ関数です。特に、vector::erase()とstd::remove_if()が必要です。あなたが必要とする消去のバージョンは、一度に消去するのではなく、イテレータのペアをとるものです。それを呼び出すと、m_queue_.erase(XXXX, m_queue_.end())のようになります。ここでXXXXとは何ですか？戻り値はremove_ifです。 remove_ifへの引数は、イテレータのペアと単項述語です。 （つまり、あなたのベクトルの中には何でもconst T&をとり、削除する必要がある場合はtrueを返す関数）戻り値は、削除されていないアイテムの最後を過ぎたイテレータです。 C++ 11では

、これは次のようになります。

struct Filter { 
    Filter(const string& id) : id_(id) {} 
    string id_; 
    bool operator()(const Item* item) { return item.m_id_ == id_; } 
}; 

してからコール：前のC++ 11では

string id = "the_id_to_filter"; 
m_queue_.erase(std::remove_if(m_queue_.begin(), m_queue_.end(), 
           [&id](const Item* item) { 
            return item_.m_id_ == id; 
           }); 

を、あなたのような何かを持つラムダに代わりますそれが有効なら、あなたのベクトルがnullptrまたはその他の値THAを含むようにするために

string id = "the_id_to_filter"; 
m_queue_.erase(std::remove_if(m_queue_.begin(), m_queue_.end(), Filter(id))); 

：次のようになりますこれらのチェックを述語関数の中に追加します。また、ベクトルがアイテムを所有している場合は、vector<Item*>の代わりにvector<std::unique_ptr<Item>>にしたい場合があります。イレース（remove_if）イディオムを使用しないと漏れてしまう可能性があります。また、物事を削除することを覚えておく必要がなくなります。

ライブラリ関数を使用すると、ループ内に1つのバグが残っていることやその他の痛みを軽減できます。参考

：

• https://en.wikipedia.org/wiki/Erase-remove_idiom
Which kind of pointer do I use when?