From a058b22b86dc7db468a10d197d2d9d2dac234399 Mon Sep 17 00:00:00 2001 From: "Shaopeng (Chris) Lin" Date: Sun, 3 Mar 2024 02:56:30 -0500 Subject: [PATCH] Utilized Formatter Class to Replace (o)stringstreams Files like tooltesting.cpp, uuid.cpp, archive.cpp, debug.h, and createZimExample.cpp are unchanged due to some special characteristics that need discussion Fix #432 --- include/zim/error.h | 10 ++++------ src/archive.cpp | 5 ++--- src/compression.cpp | 6 +++--- src/entry.cpp | 25 +++++++++++-------------- src/file_compound.cpp | 17 ++++++----------- src/file_reader.cpp | 30 ++++++++++++++---------------- src/fileimpl.cpp | 6 +----- src/fs_unix.cpp | 7 ++----- src/fs_windows.cpp | 28 ++++++++++++---------------- src/narrowdown.h | 9 +++++---- src/version.cpp | 8 ++++---- src/writer/cluster.cpp | 18 +++++++++--------- src/writer/counterHandler.cpp | 4 ++-- src/writer/creator.cpp | 23 +++++++++++------------ src/writer/defaultIndexData.h | 4 ++-- src/writer/xapianWorker.cpp | 5 +---- test/creator.cpp | 4 ++-- test/tools.h | 5 ++--- test/uuid.cpp | 5 ++--- 19 files changed, 95 insertions(+), 124 deletions(-) diff --git a/include/zim/error.h b/include/zim/error.h index 25f2b74b3..a0aedcff8 100644 --- a/include/zim/error.h +++ b/include/zim/error.h @@ -21,9 +21,9 @@ #define ZIM_ERROR_H #include "zim.h" +#include #include -#include #include namespace zim @@ -135,11 +135,9 @@ namespace zim try { std::rethrow_exception(exception); } catch (const std::exception& e) { - std::stringstream ss; - ss << "Asynchronous error: "; - ss << typeid(e).name() << std::endl; - ss << e.what(); - return ss.str(); + return Formatter() + << "Asynchronous error: " << typeid(e).name() << '\n' + << e.what(); } catch (...) { return "Unknown asynchronous exception"; } diff --git a/src/archive.cpp b/src/archive.cpp index 561dc6559..3b47df4e6 100644 --- a/src/archive.cpp +++ b/src/archive.cpp @@ -150,9 +150,8 @@ namespace zim } Item Archive::getIllustrationItem(unsigned int size) const { - std::ostringstream ss; - ss << "Illustration_" << size << "x" << size << "@" << 1; - auto r = m_impl->findx('M', ss.str()); + auto r = m_impl->findx('M', Formatter() << "Illustration_" << size << "x" + << size << "@" << 1); if (r.first) { return getEntryByPath(entry_index_type(r.second)).getItem(); } diff --git a/src/compression.cpp b/src/compression.cpp index f14504052..c738fa649 100644 --- a/src/compression.cpp +++ b/src/compression.cpp @@ -22,6 +22,7 @@ #include "compression.h" #include "envvalue.h" +#include "tools.h" #include @@ -51,9 +52,8 @@ CompStatus LZMA_INFO::stream_run(stream_t* stream, CompStep step) case LZMA_OK: return CompStatus::OK; default: { - std::ostringstream ss; - ss << "Unexpected lzma status : " << errcode; - throw std::runtime_error(ss.str()); + throw std::runtime_error(zim::Formatter() + << "Unexpected lzma status : " << errcode); } } } diff --git a/src/entry.cpp b/src/entry.cpp index 4d3e14a34..57cbf33b4 100644 --- a/src/entry.cpp +++ b/src/entry.cpp @@ -23,8 +23,7 @@ #include #include "fileimpl.h" #include "log.h" - -#include +#include "tools.h" log_define("zim.entry") @@ -57,14 +56,13 @@ bool Entry::isRedirect() const Item Entry::getItem(bool follow) const { - if (isRedirect()) { - if (! follow) { - std::ostringstream sstream; - sstream << "Entry " << getPath() << " is a redirect entry."; - throw InvalidType(sstream.str()); - } + if (isRedirect()) + { + if (!follow) + throw InvalidType(Formatter() + << "Entry " << getPath() << " is a redirect entry."); return getRedirect(); - } + } return Item(*this); } @@ -79,11 +77,10 @@ Item Entry::getRedirect() const { } entry_index_type Entry::getRedirectEntryIndex() const { - if (!isRedirect()) { - std::ostringstream sstream; - sstream << "Entry " << getPath() << " is not a redirect entry."; - throw InvalidType(sstream.str()); - } + if (!isRedirect()) + throw InvalidType(Formatter() + << "Entry " << getPath() << " is not a redirect entry."); + return m_dirent->getRedirectIndex().v; } diff --git a/src/file_compound.cpp b/src/file_compound.cpp index c3fcd8262..6c3ad067a 100644 --- a/src/file_compound.cpp +++ b/src/file_compound.cpp @@ -19,10 +19,10 @@ */ #include "file_compound.h" +#include "tools.h" #include #include -#include #include #ifdef _WIN32 @@ -61,11 +61,8 @@ FileCompound::FileCompound(const std::string& filename): } catch (...) { } if (empty()) - { - std::ostringstream msg; - msg << "error " << errnoSave << " opening file \"" << filename; - throw std::runtime_error(msg.str()); - } + throw std::runtime_error(Formatter() << "error " << errnoSave + << " opening file \"" << filename); } } @@ -115,11 +112,9 @@ time_t FileCompound::getMTime() const { int ret = ::stat(fname, &st); #endif if (ret != 0) - { - std::ostringstream msg; - msg << "stat failed with errno " << errno << " : " << strerror(errno); - throw std::runtime_error(msg.str()); - } + throw std::runtime_error(Formatter() << "stat failed with errno " << errno + << " : " << strerror(errno)); + mtime = st.st_mtime; return mtime; diff --git a/src/file_reader.cpp b/src/file_reader.cpp index 2fcf09661..af92558a8 100644 --- a/src/file_reader.cpp +++ b/src/file_reader.cpp @@ -22,12 +22,12 @@ #include #include "file_reader.h" #include "file_compound.h" +#include "tools.h" #include "buffer.h" #include #include #include #include -#include #include #include @@ -74,7 +74,7 @@ char MultiPartFileReader::read(offset_t offset) const { fhandle.readAt(&ret, zsize_t(1), physical_local_offset); } catch (std::runtime_error& e) { //Error while reading. - std::ostringstream s; + Formatter s; s << "Cannot read a char.\n"; s << " - File part is " << part_pair->second->filename() << "\n"; s << " - File part size is " << part_pair->second->size().v << "\n"; @@ -84,7 +84,7 @@ char MultiPartFileReader::read(offset_t offset) const { s << " - physical local offset is " << physical_local_offset.v << "\n"; s << " - error is " << strerror(errno) << "\n"; std::error_code ec(errno, std::generic_category()); - throw std::system_error(ec, s.str()); + throw std::system_error(ec, s); }; return ret; } @@ -107,7 +107,7 @@ void MultiPartFileReader::read(char* dest, offset_t offset, zsize_t size) const try { part->fhandle().readAt(dest, size_to_get, physical_local_offset); } catch (std::runtime_error& e) { - std::ostringstream s; + Formatter s; s << "Cannot read chars.\n"; s << " - File part is " << part->filename() << "\n"; s << " - File part size is " << part->size().v << "\n"; @@ -119,7 +119,7 @@ void MultiPartFileReader::read(char* dest, offset_t offset, zsize_t size) const s << " - physical local offset is " << physical_local_offset.v << "\n"; s << " - error is " << strerror(errno) << "\n"; std::error_code ec(errno, std::generic_category()); - throw std::system_error(ec, s.str()); + throw std::system_error(ec, s); }; ASSERT(size_to_get, <=, size); dest += size_to_get.v; @@ -147,13 +147,11 @@ mmapReadOnly(int fd, offset_type offset, size_type size) #endif const auto p = (char*)mmap(NULL, size, PROT_READ, MAP_FLAGS, fd, offset); - if (p == MAP_FAILED ) - { - std::ostringstream s; - s << "Cannot mmap size " << size << " at off " << offset - << " : " << strerror(errno); - throw std::runtime_error(s.str()); - } + if (p == MAP_FAILED) + throw std::runtime_error(Formatter() + << "Cannot mmap size " << size << " at off " + << offset << " : " << strerror(errno)); + return p; } @@ -243,12 +241,12 @@ char FileReader::read(offset_t offset) const _fhandle->readAt(&ret, zsize_t(1), offset); } catch (std::runtime_error& e) { //Error while reading. - std::ostringstream s; + Formatter s; s << "Cannot read a char.\n"; s << " - Reading offset at " << offset.v << "\n"; s << " - error is " << strerror(errno) << "\n"; std::error_code ec(errno, std::generic_category()); - throw std::system_error(ec, s.str()); + throw std::system_error(ec, s); }; return ret; } @@ -264,13 +262,13 @@ void FileReader::read(char* dest, offset_t offset, zsize_t size) const try { _fhandle->readAt(dest, size, offset); } catch (std::runtime_error& e) { - std::ostringstream s; + Formatter s; s << "Cannot read chars.\n"; s << " - Reading offset at " << offset.v << "\n"; s << " - size is " << size.v << "\n"; s << " - error is " << strerror(errno) << "\n"; std::error_code ec(errno, std::generic_category()); - throw std::system_error(ec, s.str()); + throw std::system_error(ec, s); }; } diff --git a/src/fileimpl.cpp b/src/fileimpl.cpp index 7882d381b..e9825979f 100644 --- a/src/fileimpl.cpp +++ b/src/fileimpl.cpp @@ -507,11 +507,7 @@ class Grouping const std::string& FileImpl::getMimeType(uint16_t idx) const { if (idx >= mimeTypes.size()) - { - std::ostringstream msg; - msg << "unknown mime type code " << idx; - throw ZimFileFormatError(msg.str()); - } + throw ZimFileFormatError(Formatter() << "unknown mime type code " << idx); return mimeTypes[idx]; } diff --git a/src/fs_unix.cpp b/src/fs_unix.cpp index 872393d3e..47c6133bb 100644 --- a/src/fs_unix.cpp +++ b/src/fs_unix.cpp @@ -18,8 +18,8 @@ */ #include "fs_unix.h" +#include "tools.h" #include -#include #include #include @@ -138,10 +138,7 @@ bool FS::removeFile(path_t path) { std::string getFilePathFromFD(int fd) { - std::ostringstream oss; - oss << "/dev/fd/" << fd; - - return oss.str(); + return Formatter() << "/dev/fd/" << fd; } }; // zim namespace diff --git a/src/fs_windows.cpp b/src/fs_windows.cpp index 4fe568491..1322c5a4a 100644 --- a/src/fs_windows.cpp +++ b/src/fs_windows.cpp @@ -18,6 +18,7 @@ */ #include "fs_windows.h" +#include "tools.h" #include #include @@ -27,7 +28,6 @@ #include #include -#include namespace zim { @@ -135,11 +135,10 @@ std::unique_ptr FS::toWideChar(path_t path) auto wdata = std::unique_ptr(new wchar_t[size]); auto ret = MultiByteToWideChar(CP_UTF8, 0, path.c_str(), -1, wdata.get(), size); - if (0 == ret) { - std::ostringstream oss; - oss << "Cannot convert path to wchar : " << GetLastError(); - throw std::runtime_error(oss.str()); - } + if (0 == ret) + throw std::runtime_error(Formatter() << "Cannot convert path to wchar : " + << GetLastError()); + return wdata; } @@ -154,11 +153,10 @@ FD FS::openFile(path_t filepath) OPEN_EXISTING, FILE_ATTRIBUTE_READONLY|FILE_FLAG_RANDOM_ACCESS, NULL); - if (handle == INVALID_HANDLE_VALUE) { - std::ostringstream oss; - oss << "Cannot open file : " << GetLastError(); - throw std::runtime_error(oss.str()); - } + if (handle == INVALID_HANDLE_VALUE) + throw std::runtime_error(Formatter() oss << "Cannot open file : " + << GetLastError()); + return FD(handle); } @@ -173,11 +171,9 @@ bool FS::makeDirectory(path_t path) void FS::rename(path_t old_path, path_t new_path) { auto ret = MoveFileExW(toWideChar(old_path).get(), toWideChar(new_path).get(), MOVEFILE_REPLACE_EXISTING|MOVEFILE_WRITE_THROUGH); - if (!ret) { - std::ostringstream oss; - oss << "Cannot move file " << old_path << " to " << new_path; - throw std::runtime_error(oss.str()); - } + if (!ret) + throw std::runtime_error(Formatter() << "Cannot move file " << old_path + << " to " << new_path); } std::string FS::join(path_t base, path_t name) diff --git a/src/narrowdown.h b/src/narrowdown.h index 6ce4caf73..e3b633298 100644 --- a/src/narrowdown.h +++ b/src/narrowdown.h @@ -22,6 +22,7 @@ #define ZIM_NARROWDOWN_H #include "debug.h" +#include "tools.h" #include #include @@ -107,11 +108,11 @@ class NarrowDown // It is somehow a bug and have been fixed then, but we still have to be tolerent here and accept that // two concecutive keys can be equal. if (key > nextKey) { - std::stringstream ss; + Formatter ss; ss << "Dirent table is not properly sorted:\n"; ss << " #" << i << ": " << key[0] << "/" << key.substr(1) << "\n"; ss << " #" << i+1 << ": " << nextKey[0] << "/" << nextKey.substr(1); - throw ZimFileFormatError(ss.str()); + throw ZimFileFormatError(ss); } if ( entries.empty() ) { addEntry(key, i); @@ -120,10 +121,10 @@ class NarrowDown { const std::string pseudoKey = shortestStringInBetween(key, nextKey); if (pred(pseudoKey, entries.back())) { - std::stringstream ss; + Formatter ss; ss << "Dirent table is not properly sorted:\n"; ss << "PseudoKey " << pseudoKey << " should be after (or equal) previously generated " << pred.getKeyContent(entries.back()) << "\n"; - throw ZimFileFormatError(ss.str()); + throw ZimFileFormatError(ss); } ASSERT(entries.back().lindex, <, i); addEntry(pseudoKey, i); diff --git a/src/version.cpp b/src/version.cpp index 5f6b077bf..1ba5b31b9 100644 --- a/src/version.cpp +++ b/src/version.cpp @@ -18,7 +18,7 @@ */ #include -#include +#include "tools.h" #include #include @@ -45,9 +45,9 @@ namespace zim versions.push_back({ "libxapian", XAPIAN_VERSION }); // U_ICU_VERSION does not include the patch level if 0 - std::ostringstream libicu_version; - libicu_version << U_ICU_VERSION_MAJOR_NUM << "." << U_ICU_VERSION_MINOR_NUM << "." << U_ICU_VERSION_PATCHLEVEL_NUM; - versions.push_back({ "libicu", libicu_version.str() }); + versions.push_back({"libicu", Formatter() << U_ICU_VERSION_MAJOR_NUM << "." + << U_ICU_VERSION_MINOR_NUM << "." + << U_ICU_VERSION_PATCHLEVEL_NUM}); #endif return versions; diff --git a/src/writer/cluster.cpp b/src/writer/cluster.cpp index a70d5367e..76af1537f 100644 --- a/src/writer/cluster.cpp +++ b/src/writer/cluster.cpp @@ -202,10 +202,10 @@ void Cluster::write(int out_fd) const } default: - std::ostringstream msg; + Formatter msg; msg << "invalid compression flag " << static_cast(getCompression()); - log_error(msg.str()); - throw std::runtime_error(msg.str()); + log_error(msg); + throw std::runtime_error(msg); } } @@ -243,12 +243,12 @@ void Cluster::write_data(writer_t writer) const size += blob.size(); writer(blob); } - if (size != provider->getSize()) { - std::stringstream ss; - ss << "Declared provider's size (" << provider->getSize() << ")"; - ss << " is not equal to total size returned by feed() calls (" << size << ")."; - throw IncoherentImplementationError(ss.str()); - } + if (size != provider->getSize()) + throw IncoherentImplementationError( + Formatter() + << "Declared provider's size (" << provider->getSize() << ")" + << " is not equal to total size returned by feed() calls (" << size + << ")."); } } diff --git a/src/writer/counterHandler.cpp b/src/writer/counterHandler.cpp index b29df05a6..795832cbb 100644 --- a/src/writer/counterHandler.cpp +++ b/src/writer/counterHandler.cpp @@ -45,7 +45,7 @@ DirentHandler::Dirents CounterHandler::createDirents() const { DirentHandler::ContentProviders CounterHandler::getContentProviders() const { ContentProviders ret; - std::stringstream ss; + Formatter ss; bool first = true; for(auto pair: m_mimetypeCounter) { if (! first) { @@ -54,7 +54,7 @@ DirentHandler::ContentProviders CounterHandler::getContentProviders() const { ss << pair.first << "=" << pair.second; first = false; } - ret.push_back(std::unique_ptr(new StringProvider(ss.str()))); + ret.push_back(std::unique_ptr(new StringProvider(ss))); return ret; } diff --git a/src/writer/creator.cpp b/src/writer/creator.cpp index 127661e6d..eee782609 100644 --- a/src/writer/creator.cpp +++ b/src/writer/creator.cpp @@ -190,9 +190,8 @@ namespace zim void Creator::addIllustration(unsigned int size, std::unique_ptr provider) { checkError(); - std::stringstream ss; - ss << "Illustration_" << size << "x" << size << "@1"; - addMetadata(ss.str(), std::move(provider), "image/png"); + addMetadata(Formatter() << "Illustration_" << size << "x" << size << "@1", + std::move(provider), "image/png"); } void Creator::addRedirection(const std::string& path, const std::string& title, const std::string& targetPath, const Hints& hints) @@ -213,10 +212,10 @@ namespace zim auto existing_dirent_it = data->dirents.find(&tmpDirent); if (existing_dirent_it == data->dirents.end()) { - std::ostringstream ss; - ss << "Impossible to alias C/" << targetPath << " as C/" << path << std::endl; - ss << "C/" << targetPath << " doesn't exist." << std::endl; - throw InvalidEntry(ss.str()); + Formatter ss; + ss << "Impossible to alias C/" << targetPath << " as C/" << path << '\n'; + ss << "C/" << targetPath << " doesn't exist." << '\n'; + throw InvalidEntry(ss); } auto dirent = data->createAliasDirent(path, title, **existing_dirent_it); @@ -543,11 +542,11 @@ namespace zim existing->markRemoved(); dirents.insert(dirent); } else { - std::ostringstream ss; - ss << "Impossible to add " << NsAsChar(dirent->getNamespace()) << "/" << dirent->getPath() << std::endl; - ss << " dirent's title to add is : " << dirent->getTitle() << std::endl; - ss << " existing dirent's title is : " << existing->getTitle() << std::endl; - throw InvalidEntry(ss.str()); + Formatter ss; + ss << "Impossible to add " << NsAsChar(dirent->getNamespace()) << "/" << dirent->getPath() << '\n'; + ss << " dirent's title to add is : " << dirent->getTitle() << '\n'; + ss << " existing dirent's title is : " << existing->getTitle() << '\n'; + throw InvalidEntry(ss); } }; diff --git a/src/writer/defaultIndexData.h b/src/writer/defaultIndexData.h index 9924c320a..ef8820177 100644 --- a/src/writer/defaultIndexData.h +++ b/src/writer/defaultIndexData.h @@ -60,7 +60,7 @@ namespace zim return; } #if defined(ENABLE_XAPIAN) - std::ostringstream ss; + Formatter ss; while (true) { auto blob = mp_contentProvider->feed(); if(blob.size() == 0) { @@ -70,7 +70,7 @@ namespace zim } MyHtmlParser htmlParser; try { - htmlParser.parse_html(ss.str(), "UTF-8", true); + htmlParser.parse_html(ss, "UTF-8", true); } catch(...) {} m_hasIndexData = !htmlParser.dump.empty() && htmlParser.indexing_allowed && (htmlParser.dump.find("NOINDEX") == std::string::npos); m_content = zim::removeAccents(htmlParser.dump); diff --git a/src/writer/xapianWorker.cpp b/src/writer/xapianWorker.cpp index eb2b5b5d1..ff4095005 100644 --- a/src/writer/xapianWorker.cpp +++ b/src/writer/xapianWorker.cpp @@ -63,10 +63,7 @@ namespace zim std::string fullPath = "C/" + m_path; document.set_data(fullPath); document.add_value(0, mp_indexData->getTitle()); - - std::stringstream countWordStringStream; - countWordStringStream << mp_indexData->getWordCount(); - document.add_value(1, countWordStringStream.str()); + document.add_value(1, Formatter() << mp_indexData->getWordCount()); auto geoInfo = mp_indexData->getGeoPosition(); if (std::get<0>(geoInfo)) { diff --git a/test/creator.cpp b/test/creator.cpp index b884dce83..3799292a3 100644 --- a/test/creator.cpp +++ b/test/creator.cpp @@ -358,11 +358,11 @@ TEST(ZimCreator, interruptedZimCreation) writer::Creator creator; creator.configClusterSize(16*1024); creator.startZimCreation(tmpFile.path()); - std::ostringstream oss; + Formatter oss; for ( size_t i = 0; i < 12345; ++i ) { oss << i; } - const std::string content(oss.str()); + const std::string content(oss); for ( char c = 'a'; c <= 'z'; ++c ) { const std::string path(1, c); creator.addItem(std::make_shared(path, path, content)); diff --git a/test/tools.h b/test/tools.h index e65a1afed..9ff10d05b 100644 --- a/test/tools.h +++ b/test/tools.h @@ -36,6 +36,7 @@ typedef SSIZE_T ssize_t; #endif #include "../src/buffer.h" +#include "../src/tools.h" #include #define ZIM_PRIVATE @@ -100,9 +101,7 @@ class TempFile template std::string to_string(const T& value) { - std::ostringstream ss; - ss << value; - return ss.str(); + return Formatter() << value; } std::unique_ptr diff --git a/test/uuid.cpp b/test/uuid.cpp index abfdd822d..c59478de8 100644 --- a/test/uuid.cpp +++ b/test/uuid.cpp @@ -22,6 +22,7 @@ #include #include "gtest/gtest.h" +#include "tools.h" #ifdef _WIN32 # include # include @@ -110,9 +111,7 @@ TEST(UuidTest, output) { zim::Uuid uuid( "\x55\x0e\x84\x00\xe2\x9b\x41\xd4\xa7\x16\x44\x66\x55\x44\x00\x00"); - std::ostringstream out; - out << uuid; - std::string s = out.str(); + std::string s = zim::unittests::to_string(uuid); ASSERT_EQ(s, "550e8400-e29b-41d4-a716-446655440000"); ASSERT_EQ((std::string)uuid, "550e8400-e29b-41d4-a716-446655440000"); }