Review fixes.

7 files changed, 81 insertions, 77 deletions

diff --git a/coding/file_container.cpp b/coding/file_container.cpp
index 52d189679e..082ebda4c8 100644
--- a/coding/file_container.cpp
+++ b/coding/file_container.cpp
@@ -1,5 +1,3 @@
-#include "base/SRC_FIRST.hpp"
-
 #include "coding/file_container.hpp"
 #include "coding/read_write_utils.hpp"
 #include "coding/write_to_sink.hpp"
@@ -78,30 +76,25 @@ FilesContainerR::FilesContainerR(ReaderT const & file)
 FilesContainerR::ReaderT FilesContainerR::GetReader(Tag const & tag) const
 {
   Info const * p = GetInfo(tag);
-  if (p)
-    return m_source.SubReader(p->m_offset, p->m_size);
-  else
-    MYTHROW(Reader::OpenException, ("Can't find section:", tag));
+  if (!p)
+    MYTHROW(Reader::OpenException, ("Can't find section:", GetFileName(), tag));
+  return m_source.SubReader(p->m_offset, p->m_size);
 }
 
 pair<uint64_t, uint64_t> FilesContainerR::GetAbsoluteOffsetAndSize(Tag const & tag) const
 {
   Info const * p = GetInfo(tag);
-  if (p)
-  {
-    auto reader = dynamic_cast<FileReader const *>(m_source.GetPtr());
-    uint64_t const offset = reader ? reader->GetOffset() : 0;
-    return make_pair(offset + p->m_offset, p->m_size);
-  }
-  else
-    MYTHROW(Reader::OpenException, ("Can't find section:", tag));
+  if (!p)
+    MYTHROW(Reader::OpenException, ("Can't find section:", GetFileName(), tag));
+
+  auto reader = dynamic_cast<FileReader const *>(m_source.GetPtr());
+  uint64_t const offset = reader ? reader->GetOffset() : 0;
+  return make_pair(offset + p->m_offset, p->m_size);
 }
 
 FilesContainerBase::Info const * FilesContainerBase::GetInfo(Tag const & tag) const
 {
-  InfoContainer::const_iterator i =
-    lower_bound(m_info.begin(), m_info.end(), tag, LessInfo());
-
+  auto i = lower_bound(m_info.begin(), m_info.end(), tag, LessInfo());
   if (i != m_info.end() && i->m_tag == tag)
     return &(*i);
   else
@@ -135,9 +128,15 @@ void MappedFile::Close()
 {
 #ifdef OMIM_OS_WINDOWS
   if (m_hMapping != INVALID_HANDLE_VALUE)
+  {
     CloseHandle(m_hMapping);
+    m_hMapping = INVALID_HANDLE_VALUE;
+  }
   if (m_hFile != INVALID_HANDLE_VALUE)
+  {
     CloseHandle(m_hFile);
+    m_hFile = INVALID_HANDLE_VALUE;
+  }
 #else
   if (m_fd != -1)
   {
@@ -147,37 +146,38 @@ void MappedFile::Close()
 #endif
 }
 
-MappedFile::Handle MappedFile::Map(uint64_t off, uint64_t size, string const & tag) const
+MappedFile::Handle MappedFile::Map(uint64_t offset, uint64_t size, string const & tag) const
 {
 #ifdef OMIM_OS_WINDOWS
   SYSTEM_INFO sysInfo;
   memset(&sysInfo, 0, sizeof(sysInfo));
   GetSystemInfo(&sysInfo);
-  long const offsetAlign = sysInfo.dwAllocationGranularity;
+  long const align = sysInfo.dwAllocationGranularity;
 #else
-  long const offsetAlign = sysconf(_SC_PAGE_SIZE);
+  long const align = sysconf(_SC_PAGE_SIZE);
 #endif
 
-  uint64_t const offset = (off / offsetAlign) * offsetAlign;
-  ASSERT_LESS_OR_EQUAL(offset, off, ());
-  uint64_t const length = size + (off - offset);
+  uint64_t const alignedOffset = (offset / align) * align;
+  ASSERT_LESS_OR_EQUAL(alignedOffset, offset, ());
+  uint64_t const length = size + (offset - alignedOffset);
   ASSERT_GREATER_OR_EQUAL(length, size, ());
 
 #ifdef OMIM_OS_WINDOWS
-  void * pMap = MapViewOfFile(m_hMapping, FILE_MAP_READ, offset >> (sizeof(DWORD) * 8), DWORD(offset), length);
+  void * pMap = MapViewOfFile(m_hMapping, FILE_MAP_READ, alignedOffset >> (sizeof(DWORD) * 8), DWORD(alignedOffset), length);
   if (pMap == NULL)
-    MYTHROW(Reader::OpenException, ("Can't map section:", tag, "with [offset, size]:", off, size, "win last error:", GetLastError()));
+    MYTHROW(Reader::OpenException, ("Can't map section:", tag, "with [offset, size]:", offset, size, "win last error:", GetLastError()));
 #else
-  void * pMap = mmap(0, length, PROT_READ, MAP_SHARED, m_fd, offset);
+  void * pMap = mmap(0, length, PROT_READ, MAP_SHARED, m_fd, alignedOffset);
   if (pMap == MAP_FAILED)
-    MYTHROW(Reader::OpenException, ("Can't map section:", tag, "with [offset, size]:", off, size));
+    MYTHROW(Reader::OpenException, ("Can't map section:", tag, "with [offset, size]:", offset, size));
 #endif
 
   char const * data = reinterpret_cast<char const *>(pMap);
-  char const * d = data + (off - offset);
+  char const * d = data + (offset - alignedOffset);
   return Handle(d, data, size, length);
 }
-}
+
+} // namespace detail
 
 /////////////////////////////////////////////////////////////////////////////
 // FilesMappingContainer
@@ -215,24 +215,19 @@ void FilesMappingContainer::Close()
 FilesMappingContainer::Handle FilesMappingContainer::Map(Tag const & tag) const
 {
   Info const * p = GetInfo(tag);
-  if (p)
-  {
-    ASSERT_EQUAL(tag, p->m_tag, ());
-    return m_file.Map(p->m_offset, p->m_size, tag);
-  }
-  else
-    MYTHROW(Reader::OpenException, ("Can't find section:", tag));
+  if (!p)
+    MYTHROW(Reader::OpenException, ("Can't find section:", m_name, tag));
 
-  return Handle();
+  ASSERT_EQUAL(tag, p->m_tag, ());
+  return m_file.Map(p->m_offset, p->m_size, tag);
 }
 
 FileReader FilesMappingContainer::GetReader(Tag const & tag) const
 {
   Info const * p = GetInfo(tag);
-  if (p)
-    return FileReader(m_name).SubReader(p->m_offset, p->m_size);
-  else
-    MYTHROW(Reader::OpenException, ("Can't find section:", tag));
+  if (!p)
+    MYTHROW(Reader::OpenException, ("Can't find section:", m_name, tag));
+  return FileReader(m_name).SubReader(p->m_offset, p->m_size);
 }
 
 /////////////////////////////////////////////////////////////////////////////
diff --git a/coding/file_container.hpp b/coding/file_container.hpp
index 5c5e9c0e19..ea81e7d970 100644
--- a/coding/file_container.hpp
+++ b/coding/file_container.hpp
@@ -189,7 +189,7 @@ public:
     uint64_t m_origSize;
   };
 
-  Handle Map(uint64_t off, uint64_t size, string const & tag) const;
+  Handle Map(uint64_t offset, uint64_t size, string const & tag) const;
 
 private:
 #ifdef OMIM_OS_WINDOWS
@@ -200,11 +200,13 @@ private:
 #endif
 };
 
-}
+} // namespace detail
 
 class FilesMappingContainer : public FilesContainerBase
 {
 public:
+  typedef detail::MappedFile::Handle Handle;
+
   /// Do nothing by default, call Open to attach to file.
   FilesMappingContainer() = default;
   explicit FilesMappingContainer(string const & fName);
@@ -214,8 +216,6 @@ public:
   void Open(string const & fName);
   void Close();
 
-  typedef detail::MappedFile::Handle Handle;
-
   Handle Map(Tag const & tag) const;
   FileReader GetReader(Tag const & tag) const;
 
diff --git a/generator/generator_tool/generator_tool.cpp b/generator/generator_tool/generator_tool.cpp
index 1d103ab742..1d52e6bea6 100644
--- a/generator/generator_tool/generator_tool.cpp
+++ b/generator/generator_tool/generator_tool.cpp
@@ -166,39 +166,25 @@ int main(int argc, char ** argv)
   for (size_t i = 0; i < count; ++i)
   {
     string const & country = genInfo.m_bucketNames[i];
-    string const datFile = path + country + DATA_FILE_EXTENSION;
+    string const datFile = my::JoinFoldersToPath(path, country + DATA_FILE_EXTENSION);
 
     if (FLAGS_generate_geometry)
     {
-      LOG(LINFO, ("Generating result features for", country));
-
       int mapType = feature::DataHeader::country;
       if (country == WORLD_FILE_NAME)
         mapType = feature::DataHeader::world;
       if (country == WORLD_COASTS_FILE_NAME)
         mapType = feature::DataHeader::worldcoasts;
 
+      // If error - move to next bucket without index generation.
+
+      LOG(LINFO, ("Generating result features for", country));
       if (!feature::GenerateFinalFeatures(genInfo, country, mapType))
-      {
-        // If error - move to next bucket without index generation
         continue;
-      }
 
       LOG(LINFO, ("Generating offsets table for", datFile));
-
-      try
-      {
-        string const destPath = datFile + ".offsets";
-
-        (void)feature::FeaturesOffsetsTable::Build(FilesContainerR(datFile), destPath);
-        FilesContainerW(datFile, FileWriter::OP_WRITE_EXISTING).Write(destPath, FEATURE_OFFSETS_FILE_TAG);
-
-        FileWriter::DeleteFileX(destPath);
-      }
-      catch (RootException const & ex)
-      {
-        LOG(LERROR, ("Generating offsets table failed for", datFile, "Reason", ex.Msg()));
-      }
+      if (!feature::BuildOffsetsTable(datFile))
+        continue;
     }
 
     if (FLAGS_generate_index)
diff --git a/indexer/features_offsets_table.cpp b/indexer/features_offsets_table.cpp
index a16d88f2cb..15a972e783 100644
--- a/indexer/features_offsets_table.cpp
+++ b/indexer/features_offsets_table.cpp
@@ -10,6 +10,7 @@
 
 #include "base/assert.hpp"
 #include "base/logging.hpp"
+#include "base/scope_guard.hpp"
 
 #include "std/string.hpp"
 
@@ -67,14 +68,14 @@ namespace feature
   // static
   unique_ptr<FeaturesOffsetsTable> FeaturesOffsetsTable::Load(FilesContainerR const & cont)
   {
-    unique_ptr<FeaturesOffsetsTable> ptr(new FeaturesOffsetsTable());
+    unique_ptr<FeaturesOffsetsTable> table(new FeaturesOffsetsTable());
 
-    ptr->m_file.Open(cont.GetFileName());
+    table->m_file.Open(cont.GetFileName());
     auto p = cont.GetAbsoluteOffsetAndSize(FEATURE_OFFSETS_FILE_TAG);
-    ptr->m_handle.Assign(ptr->m_file.Map(p.first, p.second, FEATURE_OFFSETS_FILE_TAG));
+    table->m_handle.Assign(table->m_file.Map(p.first, p.second, FEATURE_OFFSETS_FILE_TAG));
 
-    succinct::mapper::map(ptr->m_table, ptr->m_handle.GetData<char>());
-    return ptr;
+    succinct::mapper::map(table->m_table, table->m_handle.GetData<char>());
+    return table;
   }
 
   // static
@@ -166,4 +167,23 @@ namespace feature
     ASSERT_EQUAL(offset, m_table.select(leftBound), ("Can't find offset", offset, "in the table"));
     return leftBound;
   }
+
+  bool BuildOffsetsTable(string const & filePath)
+  {
+    try
+    {
+      string const destPath = filePath + ".offsets";
+      MY_SCOPE_GUARD(fileDeleter, bind(FileWriter::DeleteFileX, destPath));
+
+      (void)feature::FeaturesOffsetsTable::Build(FilesContainerR(filePath), destPath);
+      FilesContainerW(filePath, FileWriter::OP_WRITE_EXISTING).Write(destPath, FEATURE_OFFSETS_FILE_TAG);
+      return true;
+    }
+    catch (RootException const & ex)
+    {
+      LOG(LERROR, ("Generating offsets table failed for", filePath, "reason", ex.Msg()));
+      return false;
+    }
+  }
+
 }  // namespace feature
diff --git a/indexer/features_offsets_table.hpp b/indexer/features_offsets_table.hpp
index d7862657fa..5faba945e4 100644
--- a/indexer/features_offsets_table.hpp
+++ b/indexer/features_offsets_table.hpp
@@ -59,7 +59,7 @@ namespace feature
 
     static unique_ptr<FeaturesOffsetsTable> Load(FilesContainerR const & cont);
     static unique_ptr<FeaturesOffsetsTable> Build(FilesContainerR const & cont,
-                                                     string const & storePath);
+                                                  string const & storePath);
 
     /// Get table for the MWM map, represented by localFile and cont.
     static unique_ptr<FeaturesOffsetsTable> CreateIfNotExistsAndLoad(
@@ -111,4 +111,7 @@ namespace feature
     detail::MappedFile m_file;
     detail::MappedFile::Handle m_handle;
   };
+
+  bool BuildOffsetsTable(string const & filePath);
+
 }  // namespace feature
diff --git a/indexer/features_vector.cpp b/indexer/features_vector.cpp
index 5672088e31..c909579dfd 100644
--- a/indexer/features_vector.cpp
+++ b/indexer/features_vector.cpp
@@ -23,10 +23,10 @@ FeaturesVectorTest::FeaturesVectorTest(string const & filePath)
 FeaturesVectorTest::FeaturesVectorTest(FilesContainerR const & cont)
   : m_cont(cont), m_header(m_cont), m_vector(m_cont, m_header, 0)
 {
-  auto const ver = m_header.GetFormat();
-  if (ver == version::v5)
+  auto const version = m_header.GetFormat();
+  if (version == version::v5)
     m_vector.m_table = feature::FeaturesOffsetsTable::CreateIfNotExistsAndLoad(m_cont).release();
-  else if (ver >= version::v6)
+  else if (version >= version::v6)
     m_vector.m_table = feature::FeaturesOffsetsTable::Load(m_cont).release();
 }
 
diff --git a/indexer/index.cpp b/indexer/index.cpp
index cafe3c4213..ecda41c2c7 100644
--- a/indexer/index.cpp
+++ b/indexer/index.cpp
@@ -24,13 +24,13 @@ MwmValue::MwmValue(LocalCountryFile const & localFile)
 
 void MwmValue::SetTable(MwmInfoEx & info)
 {
-  auto const ver = GetHeader().GetFormat();
-  if (ver < version::v5)
+  auto const version = GetHeader().GetFormat();
+  if (version < version::v5)
     return;
 
   if (!info.m_table)
   {
-    if (ver == version::v5)
+    if (version == version::v5)
       info.m_table = feature::FeaturesOffsetsTable::CreateIfNotExistsAndLoad(m_file, m_cont);
     else
       info.m_table = feature::FeaturesOffsetsTable::Load(m_cont);