diff --git a/android/jni/com/mapswithme/maps/Framework.cpp b/android/jni/com/mapswithme/maps/Framework.cpp index b7697e9cf2..46f2c453c6 100644 --- a/android/jni/com/mapswithme/maps/Framework.cpp +++ b/android/jni/com/mapswithme/maps/Framework.cpp @@ -471,7 +471,7 @@ namespace android out.push_back(m_work.GetCountryName(v[i])); } } - catch (Reader::Exception const & ex) + catch (RootException const & ex) { // sdcard can contain dummy _*.mwm files. Supress this errors. LOG(LWARNING, ("Bad mwm file:", v[i], "Error:", ex.Msg())); diff --git a/coding/file_reader.cpp b/coding/file_reader.cpp index e652fba58b..a993ccb34a 100644 --- a/coding/file_reader.cpp +++ b/coding/file_reader.cpp @@ -86,25 +86,38 @@ uint64_t FileReader::Size() const void FileReader::Read(uint64_t pos, void * p, size_t size) const { - ASSERT_LESS_OR_EQUAL(pos + size, Size(), (pos, size)); + ASSERT ( AssertPosAndSize(pos, size), () ); m_pFileData->Read(m_Offset + pos, p, size); } FileReader FileReader::SubReader(uint64_t pos, uint64_t size) const { - ASSERT_LESS_OR_EQUAL(pos + size, Size(), (pos, size)); + ASSERT ( AssertPosAndSize(pos, size), () ); return FileReader(*this, m_Offset + pos, size); } FileReader * FileReader::CreateSubReader(uint64_t pos, uint64_t size) const { - ASSERT_LESS_OR_EQUAL(pos + size, Size(), (pos, size)); + ASSERT ( AssertPosAndSize(pos, size), () ); return new FileReader(*this, m_Offset + pos, size); } +bool FileReader::AssertPosAndSize(uint64_t pos, uint64_t size) const +{ + uint64_t const allSize1 = Size(); + bool const ret1 = (pos + size <= allSize1); + ASSERT ( ret1, (pos, size, allSize1) ); + + uint64_t const allSize2 = m_pFileData->Size(); + bool const ret2 = (m_Offset + pos + size <= allSize2); + ASSERT ( ret2, (m_Offset, pos, size, allSize2) ); + + return (ret1 && ret2); +} + void FileReader::SetOffsetAndSize(uint64_t offset, uint64_t size) { - ASSERT_LESS_OR_EQUAL(offset + size, Size(), (offset, size)); + ASSERT ( AssertPosAndSize(offset, size), () ); m_Offset = offset; m_Size = size; } diff --git a/coding/file_reader.hpp b/coding/file_reader.hpp index 76cfa881f8..cb8e080ce1 100644 --- a/coding/file_reader.hpp +++ b/coding/file_reader.hpp @@ -23,7 +23,9 @@ public: FileReader * CreateSubReader(uint64_t pos, uint64_t size) const; protected: - // Used in special derived readers. + /// Make assertion that pos + size in FileReader bounds. + bool AssertPosAndSize(uint64_t pos, uint64_t size) const; + /// Used in special derived readers. void SetOffsetAndSize(uint64_t offset, uint64_t size); private: diff --git a/coding/parse_xml.hpp b/coding/parse_xml.hpp index 60b4921554..b5ca32b904 100644 --- a/coding/parse_xml.hpp +++ b/coding/parse_xml.hpp @@ -29,7 +29,9 @@ bool ParseXML(SourceT & source, XMLDispatcherT & dispatcher, bool useCharData = } catch (SourceOutOfBoundsException & e) { - if (!parser.ParseBuffer(e.BytesRead(), true)) + size_t const toRead = e.BytesRead(); + // 0 - means Reader overflow (see ReaderSource::Read) + if (toRead == 0 || !parser.ParseBuffer(toRead, true)) return false; } diff --git a/coding/reader.cpp b/coding/reader.cpp index 4119bb2e0b..f2ae3b2f68 100644 --- a/coding/reader.cpp +++ b/coding/reader.cpp @@ -19,3 +19,24 @@ bool Reader::IsEqual(string const & name1, string const & name2) return (name1 == name2); #endif } + +namespace +{ + bool AssertPosAndSizeImpl(uint64_t pos, uint64_t size, uint64_t readerSize) + { + bool const ret1 = (pos + size <= readerSize); + bool const ret2 = (size <= static_cast(-1)); + ASSERT ( ret1 && ret2, (pos, size, readerSize) ); + return (ret1 && ret2); + } +} + +bool MemReader::AssertPosAndSize(uint64_t pos, uint64_t size) const +{ + return AssertPosAndSizeImpl(pos, size, Size()); +} + +bool SharedMemReader::AssertPosAndSize(uint64_t pos, uint64_t size) const +{ + return AssertPosAndSizeImpl(pos, size, Size()); +} diff --git a/coding/reader.hpp b/coding/reader.hpp index e0605e541b..df33f77176 100644 --- a/coding/reader.hpp +++ b/coding/reader.hpp @@ -3,9 +3,8 @@ #include "source.hpp" #include "../base/assert.hpp" -#include "../base/base.hpp" +#include "../base/logging.hpp" #include "../base/exception.hpp" -#include "../base/macros.hpp" #include "../std/shared_array.hpp" #include "../std/shared_ptr.hpp" @@ -34,6 +33,8 @@ public: // Reader from memory. class MemReader : public Reader { + bool AssertPosAndSize(uint64_t pos, uint64_t size) const; + public: // Construct from block of memory. MemReader(void const * pData, size_t size) @@ -48,21 +49,19 @@ public: inline void Read(uint64_t pos, void * p, size_t size) const { - ASSERT_LESS_OR_EQUAL(pos + size, Size(), (pos, size)); + ASSERT ( AssertPosAndSize(pos, size), () ); memcpy(p, m_pData + pos, size); } inline MemReader SubReader(uint64_t pos, uint64_t size) const { - ASSERT_LESS_OR_EQUAL(pos + size, Size(), (pos, size)); - ASSERT_LESS_OR_EQUAL(size, static_cast(-1), ()); + ASSERT ( AssertPosAndSize(pos, size), () ); return MemReader(m_pData + pos, static_cast(size)); } inline MemReader * CreateSubReader(uint64_t pos, uint64_t size) const { - ASSERT_LESS_OR_EQUAL(pos + size, Size(), (pos, size)); - ASSERT_LESS_OR_EQUAL(size, static_cast(-1), ()); + ASSERT ( AssertPosAndSize(pos, size), () ); return new MemReader(m_pData + pos, static_cast(size)); } @@ -73,6 +72,8 @@ private: class SharedMemReader { + bool AssertPosAndSize(uint64_t pos, uint64_t size) const; + public: explicit SharedMemReader(size_t size) : m_data(new char[size]), m_offset(0), m_size(size) {} @@ -82,21 +83,19 @@ public: inline void Read(uint64_t pos, void * p, size_t size) const { - ASSERT_LESS_OR_EQUAL(pos + size, Size(), (pos, size)); + ASSERT ( AssertPosAndSize(pos, size), () ); memcpy(p, Data() + pos, size); } inline SharedMemReader SubReader(uint64_t pos, uint64_t size) const { - ASSERT_LESS_OR_EQUAL(pos + size, Size(), (pos, size)); - ASSERT_LESS_OR_EQUAL(size, static_cast(-1), ()); + ASSERT ( AssertPosAndSize(pos, size), () ); return SharedMemReader(m_data, static_cast(pos), static_cast(size)); } inline SharedMemReader * CreateSubReader(uint64_t pos, uint64_t size) const { - ASSERT_LESS_OR_EQUAL(pos + size, Size(), (pos, size)); - ASSERT_LESS_OR_EQUAL(size, static_cast(-1), ()); + ASSERT ( AssertPosAndSize(pos, size), () ); return new SharedMemReader(m_data, static_cast(pos), static_cast(size)); } @@ -180,11 +179,16 @@ public: void Read(void * p, size_t size) { - if (m_pos + size > m_reader.Size()) + uint64_t const readerSize = m_reader.Size(); + if (m_pos + size > readerSize) { - size_t const remainingSize = static_cast(Size()); - m_reader.Read(m_pos, p, remainingSize); - m_pos = m_reader.Size(); + size_t remainingSize = 0; + if (readerSize >= m_pos) + { + remainingSize = static_cast(readerSize - m_pos); + m_reader.Read(m_pos, p, remainingSize); + m_pos = readerSize; + } MYTHROW1(SourceOutOfBoundsException, remainingSize, ()); } else @@ -197,6 +201,7 @@ public: void Skip(uint64_t size) { m_pos += size; + ASSERT ( AssertPosition(), () ); } uint64_t Pos() const @@ -206,6 +211,7 @@ public: uint64_t Size() const { + ASSERT ( AssertPosition(), () ); return (m_reader.Size() - m_pos); } @@ -216,17 +222,24 @@ public: ReaderT SubReader(uint64_t size) { - uint64_t pos = m_pos; - m_pos += size; + uint64_t const pos = m_pos; + Skip(size); return m_reader.SubReader(pos, size); } ReaderT SubReader() { - return SubReader(m_reader.Size() - m_pos); + return SubReader(Size()); } private: + bool AssertPosition() const + { + bool const ret = (m_pos <= m_reader.Size()); + ASSERT ( ret, (m_pos, m_reader.Size()) ); + return ret; + } + ReaderT m_reader; uint64_t m_pos; }; diff --git a/coding/var_serial_vector.hpp b/coding/var_serial_vector.hpp index b2dc66c3f2..366befb120 100644 --- a/coding/var_serial_vector.hpp +++ b/coding/var_serial_vector.hpp @@ -78,6 +78,11 @@ class VarSerialVectorReader public: template explicit VarSerialVectorReader(SourceT & source) : + + /// Reading this code can blow your mind :) + /// Initialization (and declaration below) order of m_offsetsReader and m_dataReader matters!!! + /// @see ReaderSource::SubReader - it's not constant. + /// @todo Do this stuff in a better way. m_size(ReadPrimitiveFromSource(source)), m_offsetsReader(source.SubReader(m_size * sizeof(uint32_t))), m_dataReader(source.SubReader()) @@ -111,6 +116,8 @@ public: } private: + + /// @note Do NOT change declaration order. uint64_t m_size; ReaderT m_offsetsReader; ReaderT m_dataReader; diff --git a/map/feature_vec_model.cpp b/map/feature_vec_model.cpp index 4c6c97bc13..87290df50f 100644 --- a/map/feature_vec_model.cpp +++ b/map/feature_vec_model.cpp @@ -16,17 +16,17 @@ namespace model { +// While reading any files (classificator or mwm), there are 2 types of possible exceptions: +// Reader::Exception; FileAbsentException; SourceOutOfBoundsException. +// Let's process RootException everywhere, to supress such errors. + void FeaturesFetcher::InitClassificator() { try { classificator::Load(); } - catch (FileAbsentException const & e) - { - LOG(LERROR, ("Classificator not found: ", e.what())); - } - catch (Reader::Exception const & e) + catch (RootException const & e) { LOG(LERROR, ("Classificator read error: ", e.what())); } @@ -41,13 +41,9 @@ int FeaturesFetcher::AddMap(string const & file) version = m_multiIndex.Add(file, r); m_rect.Add(r); } - catch (Reader::Exception const & e) - { - LOG(LERROR, ("IO error while adding ", file, " map. ", e.what())); - } catch (RootException const & e) { - LOG(LERROR, ("Can't find map ", file, ". ", e.what())); + LOG(LERROR, ("IO error while adding ", file, " map. ", e.what())); } return version;