From 376ea5b19d9e65b1703ccd5718a31e651925dd3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Barth=C3=A9l=C3=A9my?= Date: Fri, 17 Mar 2017 00:35:40 +0100 Subject: [PATCH 1/2] refactor test_rosbag_storage --- .../src/create_and_iterate_bag.cpp | 96 +++++++++---------- 1 file changed, 48 insertions(+), 48 deletions(-) diff --git a/test/test_rosbag_storage/src/create_and_iterate_bag.cpp b/test/test_rosbag_storage/src/create_and_iterate_bag.cpp index 37d08525c2..bb2f494a6e 100644 --- a/test/test_rosbag_storage/src/create_and_iterate_bag.cpp +++ b/test/test_rosbag_storage/src/create_and_iterate_bag.cpp @@ -10,71 +10,71 @@ #include "boost/foreach.hpp" #include - -TEST(rosbag_storage, create_and_iterate_bag) +void create_test_bag(const std::string &filename) { - const char* bag_filename = "/tmp/rosbag_storage_create_and_iterate_bag.bag"; - { - rosbag::Bag bag; - bag.open(bag_filename, rosbag::bagmode::Write); - - std_msgs::String str; - str.data = std::string("foo"); - - std_msgs::Int32 i; - i.data = 42; - - bag.write("chatter", ros::Time::now(), str); - bag.write("numbers", ros::Time::now(), i); - - bag.close(); - } + rosbag::Bag bag; + bag.open(filename, rosbag::bagmode::Write); - { - rosbag::Bag bag; - bag.open(bag_filename, rosbag::bagmode::Read); + std_msgs::String str; + str.data = std::string("foo"); + + std_msgs::Int32 i; + i.data = 42; + + bag.write("chatter", ros::Time::now(), str); + bag.write("numbers", ros::Time::now(), i); + + bag.close(); +} + +const char* bag_filename = "/tmp/rosbag_storage_create_and_iterate_bag.bag"; - std::vector topics; - topics.push_back(std::string("chatter")); - topics.push_back(std::string("numbers")); +TEST(rosbag_storage, iterate_bag) +{ + rosbag::Bag bag; + bag.open(bag_filename, rosbag::bagmode::Read); + + std::vector topics; + topics.push_back(std::string("chatter")); + topics.push_back(std::string("numbers")); - rosbag::View view(bag, rosbag::TopicQuery(topics)); + rosbag::View view(bag, rosbag::TopicQuery(topics)); - BOOST_FOREACH(rosbag::MessageInstance const m, view) + BOOST_FOREACH(rosbag::MessageInstance const m, view) + { + std_msgs::String::ConstPtr s = m.instantiate(); + if (s != NULL) { - std_msgs::String::ConstPtr s = m.instantiate(); - if (s != NULL) + if(s->data == std::string("foo")) { + printf("Successfully checked string foo\n"); + } + else { - if(s->data == std::string("foo")) { - printf("Successfully checked string foo\n"); - } - else - { - printf("Failed checked string foo\n"); - FAIL(); - } + printf("Failed checked string foo\n"); + FAIL(); } + } - std_msgs::Int32::ConstPtr i = m.instantiate(); - if (i != NULL) + std_msgs::Int32::ConstPtr i = m.instantiate(); + if (i != NULL) + { + if (i->data == 42) { + printf("Successfully checked value 42\n"); + } + else { - if (i->data == 42) { - printf("Successfully checked value 42\n"); - } - else - { - printf("Failed checked value 42.\n"); - FAIL(); - } + printf("Failed checked value 42.\n"); + FAIL(); } } - - bag.close(); } + + bag.close(); } int main(int argc, char **argv) { ros::Time::init(); + create_test_bag(bag_filename); testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); From 400eee0d97a98625df6695251ff2f555d2172b55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Barth=C3=A9l=C3=A9my?= Date: Thu, 16 Mar 2017 13:12:15 +0100 Subject: [PATCH 2/2] fix rosbag::View::iterator copy assignment operator the compiler-generated copy assignment operator did lead to segfault and memory leaks. --- .../src/create_and_iterate_bag.cpp | 33 +++++++++++++++++++ tools/rosbag_storage/include/rosbag/view.h | 1 + tools/rosbag_storage/src/view.cpp | 13 ++++++++ 3 files changed, 47 insertions(+) diff --git a/test/test_rosbag_storage/src/create_and_iterate_bag.cpp b/test/test_rosbag_storage/src/create_and_iterate_bag.cpp index bb2f494a6e..23c91dcd86 100644 --- a/test/test_rosbag_storage/src/create_and_iterate_bag.cpp +++ b/test/test_rosbag_storage/src/create_and_iterate_bag.cpp @@ -29,6 +29,39 @@ void create_test_bag(const std::string &filename) const char* bag_filename = "/tmp/rosbag_storage_create_and_iterate_bag.bag"; +TEST(rosbag_storage, iterator_copy_constructor) +{ + // copy ctor + rosbag::Bag bag; + bag.open(bag_filename, rosbag::bagmode::Read); + rosbag::View view(bag, rosbag::TopicQuery("numbers")); + rosbag::View::const_iterator it0 = view.begin(); + EXPECT_EQ(42, it0->instantiate()->data); + rosbag::View::const_iterator it1(it0); + EXPECT_EQ(it0, it1); + EXPECT_EQ(42, it1->instantiate()->data); + ++it1; + EXPECT_NE(it0, it1); + EXPECT_EQ(42, it0->instantiate()->data); +} + +TEST(rosbag_storage, iterator_copy_assignment) +{ + // copy assignment + rosbag::Bag bag; + bag.open(bag_filename, rosbag::bagmode::Read); + rosbag::View view(bag, rosbag::TopicQuery("numbers")); + rosbag::View::const_iterator it0 = view.begin(); + EXPECT_EQ(42, it0->instantiate()->data); + rosbag::View::const_iterator it1; + it1 = it0; + EXPECT_EQ(it0, it1); + EXPECT_EQ(42, it1->instantiate()->data); + ++it1; + EXPECT_NE(it0, it1); + EXPECT_EQ(42, it0->instantiate()->data); +} + TEST(rosbag_storage, iterate_bag) { rosbag::Bag bag; diff --git a/tools/rosbag_storage/include/rosbag/view.h b/tools/rosbag_storage/include/rosbag/view.h index bce5676fea..ea9048adb9 100644 --- a/tools/rosbag_storage/include/rosbag/view.h +++ b/tools/rosbag_storage/include/rosbag/view.h @@ -63,6 +63,7 @@ class ROSBAG_DECL View { public: iterator(iterator const& i); + iterator &operator=(iterator const& i); iterator(); ~iterator(); diff --git a/tools/rosbag_storage/src/view.cpp b/tools/rosbag_storage/src/view.cpp index 7448c861a3..b388b4c1a4 100644 --- a/tools/rosbag_storage/src/view.cpp +++ b/tools/rosbag_storage/src/view.cpp @@ -59,6 +59,19 @@ View::iterator::iterator(View* view, bool end) : view_(view), view_revision_(0), View::iterator::iterator(const iterator& i) : view_(i.view_), iters_(i.iters_), view_revision_(i.view_revision_), message_instance_(NULL) { } +View::iterator &View::iterator::operator=(iterator const& i) { + if (this != &i) { + view_ = i.view_; + iters_ = i.iters_; + view_revision_ = i.view_revision_; + if (message_instance_ != NULL) { + delete message_instance_; + message_instance_ = NULL; + } + } + return *this; +} + void View::iterator::populate() { assert(view_ != NULL);