From d327d57f3f825fb2ccee97d7223e311c5cf23087 Mon Sep 17 00:00:00 2001 From: Maxime Boucher Date: Fri, 10 May 2013 11:42:20 -0700 Subject: [PATCH] Replace CHECK in Range.h by throw std::out_of_range Summary: Calling CHECK() in folly will force the program to abort in case of a failure. On the other hand, for range checking, the standard library throws std::out_of_range for many functions. Thus it could be a good idea to throw the same exception in folly so that errors can be handled using try {} catch (...) {} blocks. Test Plan: from fbcode, type: fbconfig -r folly; fbmake opt -j32; fbmake runtests_opt -j 32 What other tests should I run? Reviewed By: tudorb@fb.com FB internal diff: D808204 --- folly/Range.cpp | 1 - folly/Range.h | 51 +++++++++++++++++++++++++++++++--------- folly/test/RangeTest.cpp | 18 ++++++++++++++ 3 files changed, 58 insertions(+), 12 deletions(-) diff --git a/folly/Range.cpp b/folly/Range.cpp index c1e58eda..b4359bb1 100644 --- a/folly/Range.cpp +++ b/folly/Range.cpp @@ -21,7 +21,6 @@ #include // __v16qi #include -#include "folly/Likely.h" namespace folly { diff --git a/folly/Range.h b/folly/Range.h index 43bebb2b..04ea64ef 100644 --- a/folly/Range.h +++ b/folly/Range.h @@ -33,6 +33,7 @@ #include #include "folly/CpuId.h" #include "folly/Traits.h" +#include "folly/Likely.h" namespace folly { @@ -143,7 +144,9 @@ public: : b_(str.data()), e_(b_ + str.size()) {} // Works only for Range Range(const std::string& str, std::string::size_type startFrom) { - CHECK_LE(startFrom, str.size()); + if (UNLIKELY(startFrom > str.size())) { + throw std::out_of_range("index out of range"); + } b_ = str.data() + startFrom; e_ = str.data() + str.size(); } @@ -151,32 +154,52 @@ public: Range(const std::string& str, std::string::size_type startFrom, std::string::size_type size) { - CHECK_LE(startFrom + size, str.size()); + if (UNLIKELY(startFrom > str.size())) { + throw std::out_of_range("index out of range"); + } b_ = str.data() + startFrom; - e_ = b_ + size; + if (str.size() - startFrom < size) { + e_ = str.data() + str.size(); + } else { + e_ = b_ + size; + } } Range(const Range& str, size_t startFrom, size_t size) { - CHECK_LE(startFrom + size, str.size()); + if (UNLIKELY(startFrom > str.size())) { + throw std::out_of_range("index out of range"); + } b_ = str.b_ + startFrom; - e_ = b_ + size; + if (str.size() - startFrom < size) { + e_ = str.e_; + } else { + e_ = b_ + size; + } } // Works only for Range /* implicit */ Range(const fbstring& str) : b_(str.data()), e_(b_ + str.size()) { } // Works only for Range Range(const fbstring& str, fbstring::size_type startFrom) { - CHECK_LE(startFrom, str.size()); + if (UNLIKELY(startFrom > str.size())) { + throw std::out_of_range("index out of range"); + } b_ = str.data() + startFrom; e_ = str.data() + str.size(); } // Works only for Range Range(const fbstring& str, fbstring::size_type startFrom, fbstring::size_type size) { - CHECK_LE(startFrom + size, str.size()); + if (UNLIKELY(startFrom > str.size())) { + throw std::out_of_range("index out of range"); + } b_ = str.data() + startFrom; - e_ = b_ + size; + if (str.size() - startFrom < size) { + e_ = str.data() + str.size(); + } else { + e_ = b_ + size; + } } // Allow implicit conversion from Range (aka StringPiece) to @@ -299,12 +322,16 @@ public: } void advance(size_type n) { - CHECK_LE(n, size()); + if (n > size()) { + throw std::out_of_range("index out of range"); + } b_ += n; } void subtract(size_type n) { - CHECK_LE(n, size()); + if (n > size()) { + throw std::out_of_range("index out of range"); + } e_ -= n; } @@ -320,7 +347,9 @@ public: Range subpiece(size_type first, size_type length = std::string::npos) const { - CHECK_LE(first, size()); + if (first > size()) { + throw std::out_of_range("index out of range"); + } return Range(b_ + first, std::min(length, size() - first)); } diff --git a/folly/test/RangeTest.cpp b/folly/test/RangeTest.cpp index 095cb3a5..ebc20a5f 100644 --- a/folly/test/RangeTest.cpp +++ b/folly/test/RangeTest.cpp @@ -221,6 +221,24 @@ TEST(StringPiece, ToByteRange) { EXPECT_EQ(a.end(), c.end()); } +TEST(StringPiece, InvalidRange) { + StringPiece a("hello"); + EXPECT_EQ(a, a.subpiece(0, 10)); + EXPECT_EQ(StringPiece("ello"), a.subpiece(1)); + EXPECT_EQ(StringPiece("ello"), a.subpiece(1, std::string::npos)); + EXPECT_EQ(StringPiece("ell"), a.subpiece(1, 3)); + EXPECT_THROW(a.subpiece(6, 7), std::out_of_range); + EXPECT_THROW(a.subpiece(6), std::out_of_range); + + std::string b("hello"); + EXPECT_EQ(a, StringPiece(b, 0, 10)); + EXPECT_EQ("ello", a.subpiece(1)); + EXPECT_EQ("ello", a.subpiece(1, std::string::npos)); + EXPECT_EQ("ell", a.subpiece(1, 3)); + EXPECT_THROW(a.subpiece(6, 7), std::out_of_range); + EXPECT_THROW(a.subpiece(6), std::out_of_range); +} + template class NeedleFinderTest : public ::testing::Test { public: -- 2.34.1