Merge pull request ethz-asl#26 from schneith/fix/zero_sized_dynamic_mats

Fix bug where comparing/checking lead to a segfault for dynamic-sized
luhongquan66 · Oct 15, 2014 · 56b0f57 · 56b0f57
2 parents d0797d9 + 68ae64f
commit 56b0f57
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 4 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -41,6 +41,14 @@ catkin_add_gtest(test_glog_equal_double
   test/test_glog-equal-double.cc)
 target_link_libraries(test_glog_equal_double ${PROJECT_NAME} ${CMAKE_THREAD_LIBS_INIT})
 
+catkin_add_gtest(test_glog-zero-sized-types
+  test/test_glog-zero-sized-types.cc)
+target_link_libraries(test_glog-zero-sized-types ${PROJECT_NAME} ${CMAKE_THREAD_LIBS_INIT})
+
+catkin_add_gtest(test_gtest-zero-sized-types
+  test/test_gtest-zero-sized-types.cc)
+target_link_libraries(test_gtest-zero-sized-types ${PROJECT_NAME} ${CMAKE_THREAD_LIBS_INIT})
+
 cs_install()
 
 cs_export(INCLUDE_DIRS include

diff --git a/include/eigen-checks/internal/gtest-equal.h b/include/eigen-checks/internal/gtest-equal.h
@@ -32,6 +32,11 @@ ::testing::AssertionResult MatricesNear(
     return failure_reason;
   }
 
+  // Early exit for dynamic-sized matrices where one dimension is zero. No need to check values...
+  if(rhs.rows() == 0 || rhs.cols() == 0 || lhs.rows() == 0 || lhs.cols() == 0) {
+    return ::testing::AssertionSuccess();
+  }
+
   typedef typename Eigen::MatrixBase<LHSMatrix>::Scalar Scalar;
   const Scalar max_diff = (lhs - rhs).cwiseAbs().maxCoeff();
 
@@ -49,7 +54,7 @@ ::testing::AssertionResult MatricesNear(
         const Scalar& diff = std::abs(lij - rij);
         if (!std::isfinite(lij) ||
             !std::isfinite(rij) ||
-            !diff > tolerance) {
+             diff > tolerance) {
           if (lhs.rows() == 1) {
             failure_reason <<
                 "\nposition " << j << " evaluates to " << lij << " and " << lij;
@@ -60,12 +65,10 @@ ::testing::AssertionResult MatricesNear(
             failure_reason << "\nposition " << i << "," << j << " evaluates to "
                 << lij << " and " << rij;
           }
-          failure_reason << " and " << name_tolerance << " evaluates to "
-              << max_diff << ".\n";
+          failure_reason << " with a tolerance of " << name_tolerance << ".\n";
         }
       }
     }
-    failure_reason << "\n";
     return failure_reason;
   }
 }

diff --git a/test/test_glog-zero-sized-types.cc b/test/test_glog-zero-sized-types.cc
@@ -0,0 +1,22 @@
+#include <eigen-checks/entrypoint.h>
+#include <eigen-checks/glog.h>
+#include <gtest/gtest.h>
+
+#include "test_helper.h"
+
+TEST(EigenChecks, EigenMatrixEqualGlog_Zero_Sized_Dynamic_Matrix) {
+  Eigen::Matrix2Xd A;
+  Eigen::Matrix2Xd B(2, 3);
+  GLOG_TEST_EXPECT_DEATH_DIFFERENT_DATA(CHECK_EIGEN_MATRIX_NEAR(A, B, 0.01));
+  GLOG_TEST_EXPECT_NO_DEATH(CHECK_EIGEN_MATRIX_NEAR(A, A, 0.01));
+}
+
+TEST(EigenChecks, EigenVectorEqualGlog_Zero_Sized_Dynamic_Vector) {
+  Eigen::VectorXd A;
+  Eigen::VectorXd B(3);
+  GLOG_TEST_EXPECT_DEATH_DIFFERENT_DATA(CHECK_EIGEN_MATRIX_NEAR(A, B, 0.01));
+  GLOG_TEST_EXPECT_NO_DEATH(CHECK_EIGEN_MATRIX_NEAR(A, A, 0.01));
+}
+
+UNITTEST_ENTRYPOINT
+
diff --git a/test/test_gtest-zero-sized-types.cc b/test/test_gtest-zero-sized-types.cc
@@ -0,0 +1,20 @@
+#include <eigen-checks/entrypoint.h>
+#include <eigen-checks/gtest.h>
+
+#include "test_helper.h"
+
+TEST(EigenChecks, EigenMatrixEqualGtest_Zero_Sized_Dynamic_Matrix) {
+  Eigen::Matrix2Xd A;
+  Eigen::Matrix2Xd B(2, 3);
+  EXPECT_FALSE(EIGEN_MATRIX_NEAR(A, B, 0.01));
+  EXPECT_TRUE(EIGEN_MATRIX_NEAR(A, A, 0.01));
+}
+
+TEST(EigenChecks, EigenVectorEqualGtest_Zero_Sized_Dynamic_Vector) {
+  Eigen::VectorXd A;
+  Eigen::VectorXd B(3);
+  EXPECT_FALSE(EIGEN_MATRIX_NEAR(A, B, 0.01));
+  EXPECT_TRUE(EIGEN_MATRIX_NEAR(A, A, 0.01));
+}
+
+UNITTEST_ENTRYPOINT