From 6798315327690fdbe93add15159be5b925779bfe Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Sat, 4 Nov 2017 00:08:08 -0700 Subject: otautil: Remove the aborts in RangeSet::Parse(). We used to CHECK and abort on parsing errors. While it works fine for the updater use case (because recovery starts updater in a forked process and collects the process exit code), it's difficult for other clients to use RangeSet as a library (e.g. update_verifier). This CL switches the aborts to returning empty RangeSet instead. Callers need to check the parsing results explicitly. The CL also separates RangeSet::PushBack() into a function, and moves SortedRangeSet::Clear() into RangeSet. Test: recovery_unit_test Test: Sideload an OTA package with the new updater on angler. Test: Sideload an OTA package with injected range string errors. The updater aborts from the explicit checks. Change-Id: If2b7f6f41dc93af917a21c7877a83e98dc3fd016 --- otautil/include/otautil/rangeset.h | 42 ++++++++++++++----- otautil/rangeset.cpp | 85 +++++++++++++++++++++++++------------- 2 files changed, 87 insertions(+), 40 deletions(-) (limited to 'otautil') diff --git a/otautil/include/otautil/rangeset.h b/otautil/include/otautil/rangeset.h index c4234d181..af8ae2dee 100644 --- a/otautil/include/otautil/rangeset.h +++ b/otautil/include/otautil/rangeset.h @@ -30,28 +30,35 @@ class RangeSet { explicit RangeSet(std::vector&& pairs); + // Parses the given string into a RangeSet. Returns the parsed RangeSet, or an empty RangeSet on + // errors. static RangeSet Parse(const std::string& range_text); + // Appends the given Range to the current RangeSet. + bool PushBack(Range range); + + // Clears all the ranges from the RangeSet. + void Clear(); + std::string ToString() const; - // Get the block number for the i-th (starting from 0) block in the RangeSet. + // Gets the block number for the i-th (starting from 0) block in the RangeSet. size_t GetBlockNumber(size_t idx) const; - // RangeSet has half-closed half-open bounds. For example, "3,5" contains blocks 3 and 4. So "3,5" - // and "5,7" are not overlapped. + // Returns whether the current RangeSet overlaps with other. RangeSet has half-closed half-open + // bounds. For example, "3,5" contains blocks 3 and 4. So "3,5" and "5,7" are not overlapped. bool Overlaps(const RangeSet& other) const; - // size() gives the number of Range's in this RangeSet. + // Returns the number of Range's in this RangeSet. size_t size() const { return ranges_.size(); } - // blocks() gives the number of all blocks in this RangeSet. + // Returns the total number of blocks in this RangeSet. size_t blocks() const { return blocks_; } - // We provide const iterators only. std::vector::const_iterator cbegin() const { return ranges_.cbegin(); } @@ -60,13 +67,20 @@ class RangeSet { return ranges_.cend(); } - // Need to provide begin()/end() since range-based loop expects begin()/end(). + std::vector::iterator begin() { + return ranges_.begin(); + } + + std::vector::iterator end() { + return ranges_.end(); + } + std::vector::const_iterator begin() const { - return ranges_.cbegin(); + return ranges_.begin(); } std::vector::const_iterator end() const { - return ranges_.cend(); + return ranges_.end(); } // Reverse const iterators for MoveRange(). @@ -78,6 +92,11 @@ class RangeSet { return ranges_.crend(); } + // Returns whether the RangeSet is valid (i.e. non-empty). + explicit operator bool() const { + return !ranges_.empty(); + } + const Range& operator[](size_t i) const { return ranges_[i]; } @@ -109,6 +128,9 @@ class RangeSet { // every block in the original source. class SortedRangeSet : public RangeSet { public: + // The block size when working with offset and file length. + static constexpr size_t kBlockSize = 4096; + SortedRangeSet() {} // Ranges in the the set should be mutually exclusive; and they're sorted by the start block. @@ -122,8 +144,6 @@ class SortedRangeSet : public RangeSet { // Compute the block range the file occupies, and insert that range. void Insert(size_t start, size_t len); - void Clear(); - using RangeSet::Overlaps; bool Overlaps(size_t start, size_t len) const; diff --git a/otautil/rangeset.cpp b/otautil/rangeset.cpp index a121a4efc..532cba4a8 100644 --- a/otautil/rangeset.cpp +++ b/otautil/rangeset.cpp @@ -16,8 +16,10 @@ #include "otautil/rangeset.h" +#include #include +#include #include #include #include @@ -28,47 +30,79 @@ #include RangeSet::RangeSet(std::vector&& pairs) { - CHECK_NE(pairs.size(), static_cast(0)) << "Invalid number of tokens"; + blocks_ = 0; + if (pairs.empty()) { + LOG(ERROR) << "Invalid number of tokens"; + return; + } - // Sanity check the input. - size_t result = 0; for (const auto& range : pairs) { - CHECK_LT(range.first, range.second) << "Empty or negative range: " << range.first << ", " - << range.second; - size_t sz = range.second - range.first; - CHECK_LE(result, SIZE_MAX - sz) << "RangeSet size overflow"; - result += sz; + if (!PushBack(range)) { + Clear(); + return; + } } - - ranges_ = pairs; - blocks_ = result; } RangeSet RangeSet::Parse(const std::string& range_text) { std::vector pieces = android::base::Split(range_text, ","); - CHECK_GE(pieces.size(), static_cast(3)) << "Invalid range text: " << range_text; + if (pieces.size() < 3) { + LOG(ERROR) << "Invalid range text: " << range_text; + return {}; + } size_t num; - CHECK(android::base::ParseUint(pieces[0], &num, static_cast(INT_MAX))) - << "Failed to parse the number of tokens: " << range_text; - - CHECK_NE(num, static_cast(0)) << "Invalid number of tokens: " << range_text; - CHECK_EQ(num % 2, static_cast(0)) << "Number of tokens must be even: " << range_text; - CHECK_EQ(num, pieces.size() - 1) << "Mismatching number of tokens: " << range_text; + if (!android::base::ParseUint(pieces[0], &num, static_cast(INT_MAX))) { + LOG(ERROR) << "Failed to parse the number of tokens: " << range_text; + return {}; + } + if (num == 0) { + LOG(ERROR) << "Invalid number of tokens: " << range_text; + return {}; + } + if (num % 2 != 0) { + LOG(ERROR) << "Number of tokens must be even: " << range_text; + return {}; + } + if (num != pieces.size() - 1) { + LOG(ERROR) << "Mismatching number of tokens: " << range_text; + return {}; + } std::vector pairs; for (size_t i = 0; i < num; i += 2) { size_t first; - CHECK(android::base::ParseUint(pieces[i + 1], &first, static_cast(INT_MAX))); size_t second; - CHECK(android::base::ParseUint(pieces[i + 2], &second, static_cast(INT_MAX))); - + if (!android::base::ParseUint(pieces[i + 1], &first, static_cast(INT_MAX)) || + !android::base::ParseUint(pieces[i + 2], &second, static_cast(INT_MAX))) { + return {}; + } pairs.emplace_back(first, second); } - return RangeSet(std::move(pairs)); } +bool RangeSet::PushBack(Range range) { + if (range.first >= range.second) { + LOG(ERROR) << "Empty or negative range: " << range.first << ", " << range.second; + return false; + } + size_t sz = range.second - range.first; + if (blocks_ >= SIZE_MAX - sz) { + LOG(ERROR) << "RangeSet size overflow"; + return false; + } + + ranges_.push_back(std::move(range)); + blocks_ += sz; + return true; +} + +void RangeSet::Clear() { + ranges_.clear(); + blocks_ = 0; +} + std::string RangeSet::ToString() const { if (ranges_.empty()) { return ""; @@ -114,8 +148,6 @@ bool RangeSet::Overlaps(const RangeSet& other) const { return false; } -static constexpr size_t kBlockSize = 4096; - // Ranges in the the set should be mutually exclusive; and they're sorted by the start block. SortedRangeSet::SortedRangeSet(std::vector&& pairs) : RangeSet(std::move(pairs)) { std::sort(ranges_.begin(), ranges_.end()); @@ -158,11 +190,6 @@ void SortedRangeSet::Insert(size_t start, size_t len) { Insert(to_insert); } -void SortedRangeSet::Clear() { - blocks_ = 0; - ranges_.clear(); -} - bool SortedRangeSet::Overlaps(size_t start, size_t len) const { RangeSet rs({ { start / kBlockSize, (start + len - 1) / kBlockSize + 1 } }); return Overlaps(rs); -- cgit v1.2.3