From 8c03ba15bc4bd53287064ae697e9491b9777f16d Mon Sep 17 00:00:00 2001 From: King_DuckZ Date: Tue, 17 May 2016 20:21:44 +0200 Subject: [PATCH] Crash fix - make path on the fly Newer versions of gcc have a small string optimizations that makes moving FileRecordData objects behave incorrectly. When a small string is moved, string_refs into it become invalid. Making a path on the fly using the path() method also has the side effect of making FileRecordData smaller in size. --- include/dindexer-machinery/recorddata.hpp | 11 +++++++---- src/machinery/globbing.cpp | 6 +++--- src/machinery/scantask/hashing.cpp | 2 +- src/machinery/set_listing.cpp | 20 ++++++++++---------- src/scan/dbbackend.cpp | 6 +++--- 5 files changed, 24 insertions(+), 21 deletions(-) diff --git a/include/dindexer-machinery/recorddata.hpp b/include/dindexer-machinery/recorddata.hpp index 59b1845..5032b6b 100644 --- a/include/dindexer-machinery/recorddata.hpp +++ b/include/dindexer-machinery/recorddata.hpp @@ -39,11 +39,11 @@ namespace mchlib { mime_full(), atime(parATime), mtime(parMTime), - path(abs_path), mime_type(), mime_charset(), size(0), level(parLevel), + path_offset(0), is_directory(parIsDir), is_symlink(parIsSymLink), unreadable(false), @@ -51,22 +51,23 @@ namespace mchlib { { } - FileRecordData ( std::string&& parPath, std::size_t parRelPathOffs, std::time_t parATime, std::time_t parMTime, uint16_t parLevel, bool parIsDir, bool parIsSymLink ) : + FileRecordData ( std::string&& parPath, uint16_t parRelPathOffs, std::time_t parATime, std::time_t parMTime, uint16_t parLevel, bool parIsDir, bool parIsSymLink ) : hash {}, abs_path(std::move(parPath)), mime_full(), atime(parATime), mtime(parMTime), - path(boost::string_ref(abs_path).substr(parRelPathOffs)), mime_type(), mime_charset(), size(0), level(parLevel), + path_offset(parRelPathOffs), is_directory(parIsDir), is_symlink(parIsSymLink), unreadable(false), hash_valid(false) { + assert(path_offset <= abs_path.size()); } #if defined(NDEBUG) @@ -81,16 +82,18 @@ namespace mchlib { bool operator== ( const FileRecordData& parOther ) const; #endif + boost::string_ref path ( void ) const { return boost::string_ref(abs_path).substr(path_offset); } + TigerHash hash; std::string abs_path; mime_string mime_full; std::time_t atime; std::time_t mtime; - boost::string_ref path; boost::string_ref mime_type; boost::string_ref mime_charset; uint64_t size; uint16_t level; + uint16_t path_offset; //Relative path starting character into abs_path bool is_directory; bool is_symlink; bool unreadable; diff --git a/src/machinery/globbing.cpp b/src/machinery/globbing.cpp index a60f23b..f93fa4d 100644 --- a/src/machinery/globbing.cpp +++ b/src/machinery/globbing.cpp @@ -24,14 +24,14 @@ namespace mchlib { namespace implem { bool glob_matches (const FileRecordData& parData, const char* parGlob) { - assert(parData.path.data()); + assert(parData.path().data()); //assert that the substring in path terminates at the same place //where the one in abs_path terminates (ie: it's null-terminated). - assert(parData.path == std::string(parData.path.data())); + assert(parData.path() == std::string(parData.path().data())); //See https://github.com/FlibbleMr/neolib/blob/master/include/neolib/string_utils.hpp //for an alternative to fnmatch() (grep wildcard_match) - const int match = fnmatch(parGlob, parData.path.data(), FNM_PATHNAME); + const int match = fnmatch(parGlob, parData.path().data(), FNM_PATHNAME); return not match; } } //namespace implem diff --git a/src/machinery/scantask/hashing.cpp b/src/machinery/scantask/hashing.cpp index 4e42a75..42ce09c 100644 --- a/src/machinery/scantask/hashing.cpp +++ b/src/machinery/scantask/hashing.cpp @@ -69,7 +69,7 @@ namespace mchlib { for (auto it = parList.begin(); it != parList.end(); ++it) { assert(PathName(parEntry.abs_path) == PathName(it->abs_path).pop_right()); - PathName curr_path(it->path); + PathName curr_path(it->path()); const auto basename = mchlib::basename(curr_path); if (it->is_directory) { auto cd_list = MutableSetListingView(it); diff --git a/src/machinery/set_listing.cpp b/src/machinery/set_listing.cpp index cda981b..0639695 100644 --- a/src/machinery/set_listing.cpp +++ b/src/machinery/set_listing.cpp @@ -99,14 +99,14 @@ namespace mchlib { { assert(parBasePath); assert(m_base_path or m_current == m_end); - assert(m_current == m_end or m_base_path->atom_count() == PathName(m_current->path).atom_count() + parLevelOffset); + assert(m_current == m_end or m_base_path->atom_count() == PathName(m_current->path()).atom_count() + parLevelOffset); assert(m_current == m_end or m_base_path->atom_count() == m_current->level + m_level_offset); //Look for the point where the children of this entry start while ( m_current != m_end and ( m_current->level + m_level_offset == m_base_path->atom_count() or - *m_base_path != PathName(m_current->path).pop_right() + *m_base_path != PathName(m_current->path()).pop_right() )) { assert(m_base_path); ++m_current; @@ -157,13 +157,13 @@ namespace mchlib { template void DirIterator::increment() { - assert(PathName(m_current->path).pop_right() == *m_base_path); + assert(PathName(m_current->path()).pop_right() == *m_base_path); do { ++m_current; } while( m_current != m_end and m_current->level + m_level_offset == m_base_path->atom_count() + 1 and - *m_base_path != PathName(m_current->path).pop_right() + *m_base_path != PathName(m_current->path()).pop_right() ); } @@ -222,7 +222,7 @@ namespace mchlib { assert(std::equal(m_list.begin(), m_list.end(), SetListing(ListType(m_list), true).sorted_list().begin())); } if (not m_list.empty()) { - m_base_path.reset(new PathName(m_list.front().path)); + m_base_path.reset(new PathName(m_list.front().path())); } } @@ -289,17 +289,17 @@ namespace mchlib { } SetListingView SetListing::make_view() { - const auto offs = (m_list.empty() ? 0 : PathName(m_list.front().path).atom_count()); + const auto offs = (m_list.empty() ? 0 : PathName(m_list.front().path()).atom_count()); return SetListingView(m_list.begin(), m_list.end(), offs, m_base_path); } SetListingView SetListing::make_view() const { - const auto offs = (m_list.empty() ? 0 : PathName(m_list.front().path).atom_count()); + const auto offs = (m_list.empty() ? 0 : PathName(m_list.front().path()).atom_count()); return SetListingView(m_list.begin(), m_list.end(), offs, m_base_path); } SetListingView SetListing::make_cview() const { - const auto offs = (m_list.empty() ? 0 : PathName(m_list.front().path).atom_count()); + const auto offs = (m_list.empty() ? 0 : PathName(m_list.front().path()).atom_count()); return SetListingView(m_list.begin(), m_list.end(), offs, m_base_path); } @@ -311,7 +311,7 @@ namespace mchlib { m_level_offset(parIter.m_level_offset) { if (m_begin != m_end) { - m_base_path.reset(new PathName(m_begin->path)); + m_base_path.reset(new PathName(m_begin->path())); } } @@ -323,7 +323,7 @@ namespace mchlib { m_level_offset(parLevelOffset) { if (m_begin != m_end) { - m_base_path.reset(new PathName(m_begin->path)); + m_base_path.reset(new PathName(m_begin->path())); } } diff --git a/src/scan/dbbackend.cpp b/src/scan/dbbackend.cpp index 77ff972..c7c290e 100644 --- a/src/scan/dbbackend.cpp +++ b/src/scan/dbbackend.cpp @@ -60,7 +60,7 @@ namespace din { if (parItem.abs_path.size() != 1 or parItem.abs_path != "/") { parItem.abs_path = std::string("/") + parItem.abs_path; } - parItem.path = boost::string_ref(parItem.abs_path).substr(1); + parItem.path_offset = 1; } { @@ -125,9 +125,9 @@ namespace din { "($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13);"; const auto& itm = parData[z]; - assert(itm.path.data()); + assert(itm.path().data()); conn.query(query, - (itm.path.empty() ? empty_path_string : itm.path), + (itm.path().empty() ? empty_path_string : itm.path()), tiger_to_string(itm.hash), itm.level, new_group_id,