From 57dbb22cf7fd01e1950e4011925ae4c24f0e0df7 Mon Sep 17 00:00:00 2001 From: Raul Metsma Date: Tue, 28 Apr 2026 12:37:31 +0300 Subject: [PATCH] Coverity fixes IB-8899 Signed-off-by: Raul Metsma --- src/ASiC_S.cpp | 2 +- src/ASiC_S.h | 2 +- src/ASiContainer.cpp | 10 +++--- src/ASiContainer.h | 2 +- src/DataFile.cpp | 2 +- src/SiVaContainer.cpp | 2 +- src/SignatureTST.cpp | 8 +++-- src/crypto/PKCS11Signer.cpp | 4 +-- src/util/File.cpp | 10 +++--- src/util/File.h | 2 +- src/util/log.cpp | 71 +++++++++++++++++++++++-------------- 11 files changed, 68 insertions(+), 47 deletions(-) diff --git a/src/ASiC_S.cpp b/src/ASiC_S.cpp index 9ba56669a..e6b27d2c1 100644 --- a/src/ASiC_S.cpp +++ b/src/ASiC_S.cpp @@ -96,7 +96,7 @@ ASiC_S::ASiC_S(const string &path) THROW("ASiC-S container does not contain any signatures."); } -void ASiC_S::addDataFileChecks(const string &fileName, const string &mediaType) +void ASiC_S::addDataFileChecks(string_view fileName, const string &mediaType) { ASiContainer::addDataFileChecks(fileName, mediaType); if(!dataFiles().empty()) diff --git a/src/ASiC_S.h b/src/ASiC_S.h index 5397a6802..affc4f7f7 100644 --- a/src/ASiC_S.h +++ b/src/ASiC_S.h @@ -45,7 +45,7 @@ namespace digidoc ASiC_S(const std::string &path); DISABLE_COPY(ASiC_S); - void addDataFileChecks(const std::string &path, const std::string &mediaType) override; + void addDataFileChecks(std::string_view path, const std::string &mediaType) override; void canSave() final; void save(const ZipSerialize &s) final; diff --git a/src/ASiContainer.cpp b/src/ASiContainer.cpp index d143b709d..6cccf81ff 100644 --- a/src/ASiContainer.cpp +++ b/src/ASiContainer.cpp @@ -149,7 +149,7 @@ vector ASiContainer::signatures() const */ void ASiContainer::addDataFile(const string &path, const string &mediaType) { - string fileName = File::fileName(path); + auto fileName = File::fileName(path); addDataFileChecks(fileName, mediaType); auto nativePath = File::encodeName(path); auto size = File::fileSize(nativePath); @@ -165,8 +165,8 @@ void ASiContainer::addDataFile(const string &path, const string &mediaType) *data << is->rdbuf(); is = std::move(data); } - d->properties[fileName] = { appInfo(), File::modifiedTime(path), size }; - d->documents.push_back(new DataFilePrivate(std::move(is), std::move(fileName), mediaType)); + d->properties[string(fileName)] = { appInfo(), File::modifiedTime(path), size }; + d->documents.push_back(new DataFilePrivate(std::move(is), string(fileName), mediaType)); } void ASiContainer::addDataFile(unique_ptr is, const string &fileName, const string &mediaType) @@ -177,14 +177,14 @@ void ASiContainer::addDataFile(unique_ptr is, const string &fileName, c d->documents.push_back(new DataFilePrivate(std::move(is), fileName, mediaType)); } -void ASiContainer::addDataFileChecks(const string &fileName, const string &mediaType) +void ASiContainer::addDataFileChecks(string_view fileName, const string &mediaType) { if(!d->signatures.empty()) THROW("Can not add document to container which has signatures, remove all signatures before adding new document."); if(fileName == "mimetype") THROW("mimetype is reserved file."); if(any_of(d->documents.cbegin(), d->documents.cend(), [&](DataFile *file) { return fileName == file->fileName(); })) - THROW("Document with same file name '%s' already exists.", fileName.c_str()); + THROW("Document with same file name '%.*s' already exists.", STR_VIEW_FMT(fileName)); if(mediaType.find('/') == string::npos) THROW("MediaType does not meet format requirements (RFC2045, section 5.1) '%s'.", mediaType.c_str()); } diff --git a/src/ASiContainer.h b/src/ASiContainer.h index f6c40fe89..84a917d6f 100644 --- a/src/ASiContainer.h +++ b/src/ASiContainer.h @@ -61,7 +61,7 @@ namespace digidoc protected: ASiContainer(std::string_view mimetype); - virtual void addDataFileChecks(const std::string &path, const std::string &mediaType); + virtual void addDataFileChecks(std::string_view path, const std::string &mediaType); void addDataFilePrivate(DataFile *dataFile); Signature* addSignature(std::unique_ptr &&signature); virtual void canSave() = 0; diff --git a/src/DataFile.cpp b/src/DataFile.cpp index 2aafd1a83..17cfd55cd 100644 --- a/src/DataFile.cpp +++ b/src/DataFile.cpp @@ -114,7 +114,7 @@ DataFilePrivate::DataFilePrivate(const ZipSerialize &z, string filename, string (size = r(buf.data(), buf.size())) > 0; currentStreamSize += size) { if(!fs->write(buf.data(), size)) - THROW("Failed to write '%s' data to stream. Stream size: %d", m_filename.c_str(), currentStreamSize); + THROW("Failed to write '%s' data to stream. Stream size: %zu", m_filename.c_str(), currentStreamSize); } m_is = std::move(fs); } diff --git a/src/SiVaContainer.cpp b/src/SiVaContainer.cpp index 759b23a12..ad4e5c52b 100644 --- a/src/SiVaContainer.cpp +++ b/src/SiVaContainer.cpp @@ -157,7 +157,7 @@ SiVaContainer::SiVaContainer(const string &path, ContainerOpenCB *cb, bool useHa else if(File::fileExtension(path, {"pdf"})) { d->mediaType = "application/pdf"; - d->dataFiles.push_back(new DataFilePrivate(std::move(ifs), fileName, "application/pdf")); + d->dataFiles.push_back(new DataFilePrivate(std::move(ifs), string(fileName), "application/pdf")); } else if(File::fileExtension(path, {"asice", "sce", "asics", "scs"})) { diff --git a/src/SignatureTST.cpp b/src/SignatureTST.cpp index 8e19cff39..b6207cb80 100644 --- a/src/SignatureTST.cpp +++ b/src/SignatureTST.cpp @@ -61,7 +61,8 @@ struct SignatureTST::Data { if (auto it = cache->find(method); it != cache->cend()) { return it->second; } - return (*cache)[std::move(method)] = digest(Digest(method)).result(); + auto result = digest(Digest(method)).result(); + return (*cache)[std::move(method)] = std::move(result); } }; @@ -82,7 +83,8 @@ SignatureTST::SignatureTST(bool manifest, const ZipSerialize &z, ASiC_S *asicSDo auto ref = doc/"SigReference"; string uri = util::File::fromUriPath(ref["URI"]); metadata.emplace_back(std::move(file), std::move(mime), xml.str()); - metadata.emplace_back(std::move(uri), string(ref["MimeType"]), z.read(uri)); + auto refData = z.read(uri); + metadata.emplace_back(std::move(uri), string(ref["MimeType"]), std::move(refData)); file.clear(); for(auto ref = doc/"DataObjectReference"; ref; ref++) @@ -172,7 +174,7 @@ void SignatureTST::extendSignatureProfile(Signer *signer) }, true); auto i = metadata.insert(metadata.cbegin(), {"META-INF/ASiCArchiveManifest.xml", "text/xml", std::move(data)}); vector der = TS(i->digest(), signer->userAgent()); - metadata.insert(next(i), {tstName, TST_MIMETYPE, string{der.cbegin(), der.cend()}}); + metadata.insert(next(i), {std::move(tstName), TST_MIMETYPE, string{der.cbegin(), der.cend()}}); } X509Cert SignatureTST::TimeStampCertificate() const diff --git a/src/crypto/PKCS11Signer.cpp b/src/crypto/PKCS11Signer.cpp index 821684169..26b72b525 100644 --- a/src/crypto/PKCS11Signer.cpp +++ b/src/crypto/PKCS11Signer.cpp @@ -99,10 +99,10 @@ static auto PKCS11List(CK_FUNCTION_LIST *f, Args&&... args) using T = std::remove_pointer_t>; std::vector result; CK_ULONG count = 0; - if((f->*Getter)(std::forward(args)..., static_cast(nullptr), &count) != CKR_OK || count == 0) + if((f->*Getter)(args..., static_cast(nullptr), &count) != CKR_OK || count == 0) return result; result.resize(size_t(count)); - if((f->*Getter)(std::forward(args)..., result.data(), &count) != CKR_OK) + if((f->*Getter)(args..., result.data(), &count) != CKR_OK) result.clear(); return result; } diff --git a/src/util/File.cpp b/src/util/File.cpp index c57643f15..d7d803b9c 100644 --- a/src/util/File.cpp +++ b/src/util/File.cpp @@ -64,7 +64,7 @@ using f_utimbuf = struct utimbuf; stack File::tempFiles; -static string decodeName(fs::path path) +static string decodeName(const fs::path &path) { auto name = path.u8string(); return {reinterpret_cast(name.data()), name.size()}; @@ -149,7 +149,7 @@ bool File::fileExtension(string_view path, initializer_list list) /** * Returns file size */ -unsigned long File::fileSize(const std::filesystem::path &path) noexcept +unsigned long File::fileSize(const fs::path &path) noexcept { error_code ec; auto result = fs::file_size(path, ec); @@ -162,7 +162,7 @@ unsigned long File::fileSize(const std::filesystem::path &path) noexcept * @param path full path of the file. * @return returns file name from the file full path in UTF-8. */ -string File::fileName(const string& path) +string_view File::fileName(string_view path) noexcept { size_t pos = path.find_last_of("/\\"); return pos == string::npos ? path : path.substr(pos + 1); @@ -343,8 +343,8 @@ constexpr bool fromHexChar(auto pos, auto end, auto &value) { if(distance(pos, end) < 2) return false; - auto *p = &*pos; - return from_chars(p, p + 2, value, 16).ec == std::errc{}; + auto *p = to_address(pos); + return from_chars(p, p + 2, value, 16).ec == errc{}; } string File::fromUriPath(string_view path) diff --git a/src/util/File.h b/src/util/File.h index ccc24f1e8..ec085507c 100644 --- a/src/util/File.h +++ b/src/util/File.h @@ -42,7 +42,7 @@ namespace digidoc static bool fileExists(const std::string& path); static bool fileExtension(std::string_view path, std::initializer_list list); static unsigned long fileSize(const std::filesystem::path &path) noexcept; - static std::string fileName(const std::string& path); + static std::string_view fileName(std::string_view path) noexcept; static std::string directory(const std::string& path); static std::string path(std::string dir, std::string_view relativePath); static std::filesystem::path tempFileName(); diff --git a/src/util/log.cpp b/src/util/log.cpp index de7b1b0d2..f89898b29 100644 --- a/src/util/log.cpp +++ b/src/util/log.cpp @@ -69,19 +69,35 @@ string Log::formatArgList(const char* fmt, va_list args) return result; } +static ostream &logStream(Conf *conf) noexcept +{ + static fstream f; + static string lastPath; + try { + const auto &path = conf->logFile(); + if(path != lastPath) + { + lastPath = path; + f.close(); + if(!path.empty()) + f.open(File::encodeName(path), fstream::out|fstream::app); + } + } + catch(const exception &e) + { + f.close(); + cerr << "Failed to open log file '" << lastPath << "': " << e.what() << '\n'; + } + return f.is_open() ? static_cast(f) : cout; +} + void Log::out(LogType type, const char *file, unsigned int line, const char *format, ...) { Conf *conf = Conf::instance(); if(!conf || conf->logLevel() < type) return; - ostream *o = &cout; - fstream f; - if(!conf->logFile().empty()) - { - f.open(File::encodeName(conf->logFile()), fstream::out|fstream::app); - o = &f; - } + ostream &o = logStream(conf); time_t t = time(nullptr); tm tm {}; #ifdef _WIN32 @@ -89,19 +105,29 @@ void Log::out(LogType type, const char *file, unsigned int line, const char *for #else gmtime_r(&t, &tm); #endif - *o << put_time(&tm, "%Y-%m-%dT%TZ") << ' '; + o << put_time(&tm, "%Y-%m-%dT%TZ") << ' '; switch(type) { - case ErrorType: *o << 'E'; break; - case WarnType: *o << 'W'; break; - case InfoType: *o << 'I'; break; - case DebugType: *o << 'D'; break; + case ErrorType: o << 'E'; break; + case WarnType: o << 'W'; break; + case InfoType: o << 'I'; break; + case DebugType: o << 'D'; break; } - *o << " [" << File::fileName(file) << ':' << line << "] - "; + o << " [" << File::fileName(file) << ':' << line << "] - "; va_list args{}; va_start(args, format); - *o << formatArgList(format, args) << '\n'; + try { + if(string_view(format).contains('%')) + o << formatArgList(format, args) << '\n'; + else + o << format << '\n'; + } + catch(const exception &e) + { + cerr << "Failed to format log message: " << e.what() << '\n'; + o << format << '\n'; + } va_end(args); } @@ -111,17 +137,10 @@ void Log::dbgPrintfMemImpl(const char *msg, const unsigned char *data, size_t si if(!conf || conf->logLevel() < DebugType) return; - ostream *o = &cout; - fstream f; - if(!conf->logFile().empty()) - { - f.open(File::encodeName(conf->logFile()), fstream::out|fstream::app); - o = &f; - } - - *o << "DEBUG [" << File::fileName(file) << ':' << line << "] - " << msg << " { "; - *o << hex << uppercase << setfill('0'); + ostream &o = logStream(conf); + o << "DEBUG [" << File::fileName(file) << ':' << line << "] - " << msg << " { "; + o << hex << uppercase << setfill('0'); for(size_t i = 0; i < size; ++i) - *o << setw(2) << static_cast(data[i]) << ' '; - *o << dec << nouppercase << setfill(' ') << "}:" << size << '\n'; + o << setw(2) << static_cast(data[i]) << ' '; + o << dec << nouppercase << setfill(' ') << "}:" << size << '\n'; }