diff --git a/include/libtorrent/file_storage.hpp b/include/libtorrent/file_storage.hpp index d3d87e89a..3a4a0e2ab 100644 --- a/include/libtorrent/file_storage.hpp +++ b/include/libtorrent/file_storage.hpp @@ -123,7 +123,7 @@ namespace libtorrent { internal_file_entry& operator=(internal_file_entry&& fe) & noexcept; ~internal_file_entry(); - void set_name(char const* n, bool borrow_string = false, int string_len = 0); + void set_name(string_view n, bool borrow_string = false); string_view filename() const; enum { diff --git a/include/libtorrent/string_util.hpp b/include/libtorrent/string_util.hpp index 2cb38caea..5efd8d7a2 100644 --- a/include/libtorrent/string_util.hpp +++ b/include/libtorrent/string_util.hpp @@ -106,7 +106,7 @@ namespace libtorrent { // strdup is not part of the C standard. Some systems // don't have it and it won't be available when building // in strict ansi mode - char* allocate_string_copy(char const* str); + char* allocate_string_copy(string_view str); // searches for separator ('sep') in the string 'last'. // if found, returns the string_view representing the range from the start of diff --git a/src/disk_io_thread.cpp b/src/disk_io_thread.cpp index 5880b293f..de8fa6a49 100644 --- a/src/disk_io_thread.cpp +++ b/src/disk_io_thread.cpp @@ -33,7 +33,6 @@ POSSIBILITY OF SUCH DAMAGE. #include "libtorrent/config.hpp" #include "libtorrent/storage.hpp" #include "libtorrent/disk_io_thread.hpp" -#include "libtorrent/string_util.hpp" // for allocate_string_copy #include "libtorrent/disk_buffer_holder.hpp" #include "libtorrent/aux_/alloca.hpp" #include "libtorrent/aux_/throw.hpp" diff --git a/src/file_storage.cpp b/src/file_storage.cpp index 35d626c9c..3438b3aed 100644 --- a/src/file_storage.cpp +++ b/src/file_storage.cpp @@ -122,7 +122,7 @@ namespace { if (is_complete(path)) { TORRENT_ASSERT(set_name); - e.set_name(path.c_str()); + e.set_name(path); e.path_index = -2; return; } @@ -137,7 +137,7 @@ namespace { if (branch_path.empty()) { - if (set_name) e.set_name(leaf.data()); + if (set_name) e.set_name(leaf); e.path_index = -1; return; } @@ -162,7 +162,7 @@ namespace { } e.path_index = get_or_add_path(branch_path); - if (set_name) e.set_name(leaf.data()); + if (set_name) e.set_name(leaf); } int file_storage::get_or_add_path(string_view const path) @@ -227,10 +227,8 @@ namespace { , name(nullptr) , path_index(fe.path_index) { - if (fe.name_len == name_is_owned) - name = allocate_string_copy(fe.name); - else - name = fe.name; + bool const borrow = fe.name_len != name_is_owned; + set_name(fe.filename(), borrow); } internal_file_entry& internal_file_entry::operator=(internal_file_entry const& fe) & @@ -245,7 +243,10 @@ namespace { executable_attribute = fe.executable_attribute; symlink_attribute = fe.symlink_attribute; no_root_dir = fe.no_root_dir; - set_name(fe.filename().to_string().c_str()); + // if the name is not owned, don't allocate memory, we can point into the + // same metadata buffer + bool const borrow = fe.name_len != name_is_owned; + set_name(fe.filename(), borrow); return *this; } @@ -262,7 +263,7 @@ namespace { , name(fe.name) , path_index(fe.path_index) { - fe.name_len = name_is_owned; + fe.name_len = 0; fe.name = nullptr; } @@ -281,35 +282,32 @@ namespace { name = fe.name; name_len = fe.name_len; - fe.name_len = name_is_owned; + fe.name_len = 0; fe.name = nullptr; return *this; } - // if borrow_chars >= 0, don't take ownership over n, just - // point to it. It points to borrow_chars number of characters. - // if borrow_chars == -1, n is a 0-terminated string that - // should be copied. - void internal_file_entry::set_name(char const* n, bool const borrow_string - , int string_len) + // if borrow_string is true, don't take ownership over n, just + // point to it. + // if borrow_string is false, n will be copied and owned by the + // internal_file_entry. + void internal_file_entry::set_name(string_view n, bool const borrow_string) { - TORRENT_ASSERT(string_len >= 0); - - // we have limited space in the length field. truncate string - // if it's too long - if (string_len >= name_is_owned) string_len = name_is_owned - 1; - // free the current string, before assigning the new one if (name_len == name_is_owned) delete[] name; - if (n == nullptr) + if (n.empty()) { TORRENT_ASSERT(borrow_string == false); name = nullptr; } else if (borrow_string) { - name = n; - name_len = aux::numeric_cast(string_len); + // we have limited space in the length field. truncate string + // if it's too long + if (n.size() >= name_is_owned) n = n.substr(name_is_owned - 1); + + name = n.data(); + name_len = aux::numeric_cast(n.size()); } else { @@ -615,7 +613,7 @@ namespace { // filename is allowed to be empty, in which case we just use path if (!filename.empty()) - e.set_name(filename.data(), true, int(filename.size())); + e.set_name(filename, true); e.size = aux::numeric_cast(file_size); e.offset = aux::numeric_cast(m_total_size); @@ -680,15 +678,14 @@ namespace { template void process_path_lowercase( std::unordered_set& table - , CRC crc - , char const* str, int len) + , CRC crc, string_view str) { - if (len == 0) return; - for (int i = 0; i < len; ++i, ++str) + if (str.empty()) return; + for (char const c : str) { - if (*str == TORRENT_SEPARATOR) + if (c == TORRENT_SEPARATOR) table.insert(crc.checksum()); - crc.process_byte(to_lower(*str) & 0xff); + crc.process_byte(to_lower(c) & 0xff); } table.insert(crc.checksum()); } @@ -707,9 +704,7 @@ namespace { } for (auto const& p : m_paths) - { - process_path_lowercase(table, crc, p.c_str(), int(p.size())); - } + process_path_lowercase(table, crc, p); } std::uint32_t file_storage::file_path_hash(file_index_t const index diff --git a/src/string_util.cpp b/src/string_util.cpp index b499a3c51..8f4a242a6 100644 --- a/src/string_util.cpp +++ b/src/string_util.cpp @@ -140,13 +140,12 @@ namespace libtorrent { return static_cast(it - target.begin()); } - char* allocate_string_copy(char const* str) + char* allocate_string_copy(string_view str) { - if (str == nullptr) return nullptr; - std::size_t const len = std::strlen(str); - auto* tmp = new char[len + 1]; - std::copy(str, str + len, tmp); - tmp[len] = '\0'; + if (str.empty()) return nullptr; + auto* tmp = new char[str.size() + 1]; + std::copy(str.data(), str.data() + str.size(), tmp); + tmp[str.size()] = '\0'; return tmp; }