switch from custom stringFormat to fmtlib

The latter helps to avoid wrong format errors and is simpler to use.
Will be replaced by std::format once C++20 becomes mandatory.

Signed-off-by: Rosen Penev <[email protected]>

.github/workflows/codeql-analysis.yml

@@ -37,11 +37,11 @@ jobs:
     steps:
     - name: Checkout repository
       uses: actions/checkout@v4
-      
+
     - name: Install dependencies
       run: |
         sudo apt-get update
-        sudo apt-get install -y libexpat1-dev zlib1g-dev libbrotli-dev libinih-dev
+        sudo apt-get install -y libfmt-dev libexpat1-dev zlib1g-dev libbrotli-dev libinih-dev
 
     # Initializes the CodeQL tools for scanning.
     - name: Initialize CodeQL

.github/workflows/on_PR_mac_matrix.yml

@@ -27,6 +27,7 @@ jobs:
         run: |
           brew install ninja
           brew install inih
+          brew install fmt
           brew install googletest
 
       - name: Build

.github/workflows/on_PR_mac_special_builds.yml

@@ -22,6 +22,7 @@ jobs:
         run: |
           brew install ninja
           brew install inih
+          brew install fmt
           brew install googletest
 
       - name: Build

.github/workflows/on_PR_meson.yaml

@@ -18,7 +18,7 @@ jobs:
       - uses: actions/checkout@v4
       - uses: awalsh128/cache-apt-pkgs-action@v1
         with:
-          packages: g++-${{matrix.cxx}} libcurl4-gnutls-dev libbrotli-dev libinih-dev libgmock-dev libgtest-dev gettext ninja-build
+          packages: g++-${{matrix.cxx}} libcurl4-gnutls-dev libbrotli-dev libinih-dev libfmt-dev libgmock-dev libgtest-dev gettext ninja-build
           version: 1.0
       - name: Install meson
         run: python3 -m pip install meson==0.54.1
@@ -119,6 +119,7 @@ jobs:
             cc:p
             cmake:p
             curl:p
+            fmt:p
             gtest:p
             libinih:p
             meson:p
@@ -141,7 +142,7 @@ jobs:
 
       - name: Install packages
         run: |
-          brew install curl brotli inih expat googletest
+          brew install fmt curl brotli inih expat googletest
           python3 -m pip install meson==0.54.1 ninja
 
       - name: Compile and Test
@@ -158,7 +159,7 @@ jobs:
         uses: vmactions/freebsd-vm@v0
         with:
           prepare: |
-            pkg install -y cmake curl ninja meson gettext pkgconf googletest expat inih brotli
+            pkg install -y cmake curl ninja meson gettext pkgconf googletest expat inih brotli libfmt
           run: |
             meson setup "${{github.workspace}}/build" -Dwarning_level=3 -Dcpp_std=c++20
             meson compile -C "${{github.workspace}}/build" --verbose

.github/workflows/on_PR_windows_matrix.yml

@@ -116,6 +116,7 @@ jobs:
             libiconv:p
             libinih:p
             zlib:p
+            fmt:p
 
       - name: Build
         run: |
@@ -167,6 +168,7 @@ jobs:
             libexpat-devel
             libiconv-devel
             libinih-devel
+            libfmt-devel
             zlib-devel
 
       - name: Build

.github/workflows/on_push_BasicWinLinMac.yml

@@ -93,6 +93,7 @@ jobs:
         run: |
           brew install ninja
           brew install inih
+          brew install fmt
           brew install googletest
 
       - name: build and compile

CMakeLists.txt

@@ -90,6 +90,8 @@ endif()
 
 include_directories(${CMAKE_BINARY_DIR})  # Make the exv_conf.h file visible for the full project
 
+find_package(fmt REQUIRED)
+
 if( EXIV2_ENABLE_XMP )
   add_subdirectory( xmpsdk )
 endif()

ci/install_dependencies.sh

@@ -41,46 +41,46 @@ distro_id=$(grep '^ID=' /etc/os-release|awk -F = '{print $2}'|sed 's/\"//g')
 
 case "$distro_id" in
     'fedora')
-        dnf -y --refresh install gcc-c++ clang cmake ninja-build expat-devel zlib-devel brotli-devel libssh-devel libcurl-devel gmock-devel glibc-langpack-en inih-devel
+        dnf -y --refresh install gcc-c++ clang cmake ninja-build expat-devel zlib-devel brotli-devel libssh-devel libcurl-devel gmock-devel glibc-langpack-en inih-devel fmt-devel
         ;;
 
     'debian')
         apt-get update
-        apt-get install -y cmake ninja-build g++ clang libexpat1-dev zlib1g-dev libbrotli-dev libssh-dev libcurl4-openssl-dev libgmock-dev libxml2-utils libinih-dev
+        apt-get install -y cmake ninja-build g++ clang libexpat1-dev zlib1g-dev libbrotli-dev libssh-dev libcurl4-openssl-dev libgmock-dev libxml2-utils libinih-dev libfmt-dev
         # debian_build_gtest
         ;;
 
     'arch')
         pacman --noconfirm -Syu
-        pacman --noconfirm --needed -S gcc clang cmake ninja expat zlib brotli libssh curl gtest libinih
+        pacman --noconfirm --needed -S gcc clang cmake ninja expat zlib brotli libssh curl gtest libinih fmt
         ;;
 
     'ubuntu')
         apt-get update
-        apt-get install -y cmake ninja-build g++ clang libexpat1-dev zlib1g-dev libbrotli-dev libssh-dev libcurl4-openssl-dev libgmock-dev libxml2-utils libinih-dev
+        apt-get install -y cmake ninja-build g++ clang libexpat1-dev zlib1g-dev libbrotli-dev libssh-dev libcurl4-openssl-dev libgmock-dev libxml2-utils libinih-dev libfmt-dev
         # debian_build_gtest
         ;;
 
     'alpine')
         apk update
-        apk add gcc g++ clang cmake samurai expat-dev zlib-dev brotli-dev libssh-dev curl-dev gtest gtest-dev gmock libintl gettext-dev libxml2-utils inih-dev inih-inireader-dev
+        apk add gcc g++ clang cmake samurai expat-dev zlib-dev brotli-dev libssh-dev curl-dev gtest gtest-dev gmock libintl gettext-dev libxml2-utils inih-dev inih-inireader-dev fmt-dev
         ;;
 
     'rhel')
         dnf clean all
-        dnf -y install gcc-c++ clang cmake ninja-build expat-devel zlib-devel brotli-devel libssh-devel libcurl-devel inih-devel
+        dnf -y install gcc-c++ clang cmake ninja-build expat-devel zlib-devel brotli-devel libssh-devel libcurl-devel inih-devel fmt-devel
         ;;
 
     'centos')
         dnf clean all
-        dnf -y install gcc-c++ clang cmake expat-devel zlib-devel brotli-devel libssh-devel libcurl-devel git
+        dnf -y install gcc-c++ clang cmake expat-devel zlib-devel brotli-devel libssh-devel libcurl-devel git fmt-devel
         dnf -y --enablerepo=crb install ninja-build meson
         centos_build_inih
         ;;
 
     'opensuse-tumbleweed')
         zypper --non-interactive refresh
-        zypper --non-interactive install gcc-c++ clang cmake ninja libexpat-devel zlib-devel libbrotli-devel libssh-devel libcurl-devel gmock libxml2-tools libinih-devel
+        zypper --non-interactive install gcc-c++ clang cmake ninja libexpat-devel zlib-devel libbrotli-devel libssh-devel libcurl-devel gmock libxml2-tools libinih-devel libfmt-devel
         ;;
     *)
         echo "Sorry, no predefined dependencies for your distribution $distro_id exist yet"

conanfile.py

@@ -27,6 +27,8 @@ def requirements(self):
 
         self.requires('inih/55')
 
+        self.requires('fmt/10.1.1')
+
         if self.options.webready:
             self.requires('libcurl/7.85.0')
 

meson.build

@@ -53,6 +53,7 @@ cdata.set('EXV_ENABLE_VIDEO', get_option('video'))
 deps = []
 deps += cpp.find_library('ws2_32', required: host_machine.system() == 'windows')
 deps += cpp.find_library('procstat', required: host_machine.system() == 'freebsd')
+deps += dependency('fmt')
 
 if cpp.get_argument_syntax() == 'gcc' and cpp.version().version_compare('<9')
   if host_machine.system() == 'linux' and cpp.get_define('_LIBCPP_VERSION', prefix: '#include <new>') == ''

src/CMakeLists.txt

@@ -172,7 +172,7 @@ set_target_properties( exiv2lib_int PROPERTIES
 
 # NOTE: Cannot use target_link_libraries on OBJECT libraries with old versions of CMake
 target_include_directories(exiv2lib_int PRIVATE ${ZLIB_INCLUDE_DIR})
-target_include_directories(exiv2lib SYSTEM PRIVATE 
+target_include_directories(exiv2lib SYSTEM PRIVATE
     $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/xmpsdk/include>
 )
 
@@ -212,7 +212,7 @@ endif()
 if (WIN32)
     target_compile_definitions(exiv2lib PRIVATE PSAPI_VERSION=1)    # to be compatible with <= WinVista (#905)
     # Since windows.h is included in some headers, we need to propagate this definition
-    target_compile_definitions(exiv2lib PUBLIC WIN32_LEAN_AND_MEAN) 
+    target_compile_definitions(exiv2lib PUBLIC WIN32_LEAN_AND_MEAN)
 endif()
 
 if (NOT MSVC)
@@ -264,6 +264,9 @@ if( EXIV2_ENABLE_INIH )
   list(APPEND requires_private_list "INIReader")
 endif()
 
+target_link_libraries(exiv2lib_int PUBLIC fmt::fmt)
+target_link_libraries(exiv2lib PUBLIC fmt::fmt)
+
 # Convert private lists to delimited strings
 list(SORT libs_private_list)
 string(REPLACE ";" " -l" libs_private_string "${libs_private_list}")
@@ -293,7 +296,7 @@ install(FILES
     ${CMAKE_BINARY_DIR}/exiv2lib_export.h
     DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/exiv2)
 
-install(EXPORT exiv2Export 
+install(EXPORT exiv2Export
     DESTINATION "${CMAKE_INSTALL_DATADIR}/cmake/exiv2"
     NAMESPACE Exiv2::
 )

src/basicio.cpp

@@ -1420,7 +1420,7 @@ int64_t HttpIo::HttpImpl::getFileLength() {
   request["verb"] = "HEAD";
   int serverCode = http(request, response, errors);
   if (serverCode < 0 || serverCode >= 400 || !errors.empty()) {
-    throw Error(ErrorCode::kerFileOpenFailed, "http", Exiv2::Internal::stringFormat("%d", serverCode), hostInfo_.Path);
+    throw Error(ErrorCode::kerFileOpenFailed, "http", std::to_string(serverCode), hostInfo_.Path);
   }
 
   auto lengthIter = response.find("Content-Length");
@@ -1444,7 +1444,7 @@ void HttpIo::HttpImpl::getDataByRange(size_t lowBlock, size_t highBlock, std::st
 
   int serverCode = http(request, responseDic, errors);
   if (serverCode < 0 || serverCode >= 400 || !errors.empty()) {
-    throw Error(ErrorCode::kerFileOpenFailed, "http", Exiv2::Internal::stringFormat("%d", serverCode), hostInfo_.Path);
+    throw Error(ErrorCode::kerFileOpenFailed, "http", std::to_string(serverCode), hostInfo_.Path);
   }
   response = responseDic["body"];
 }
@@ -1497,7 +1497,7 @@ void HttpIo::HttpImpl::writeRemote(const byte* data, size_t size, size_t from, s
 
   int serverCode = http(request, response, errors);
   if (serverCode < 0 || serverCode >= 400 || !errors.empty()) {
-    throw Error(ErrorCode::kerFileOpenFailed, "http", Exiv2::Internal::stringFormat("%d", serverCode), hostInfo_.Path);
+    throw Error(ErrorCode::kerFileOpenFailed, "http", std::to_string(serverCode), hostInfo_.Path);
   }
 }
 
@@ -1594,7 +1594,7 @@ int64_t CurlIo::CurlImpl::getFileLength() {
   int serverCode;
   curl_easy_getinfo(curl_, CURLINFO_RESPONSE_CODE, &serverCode);  // get code
   if (serverCode >= 400 || serverCode < 0) {
-    throw Error(ErrorCode::kerFileOpenFailed, "http", Exiv2::Internal::stringFormat("%d", serverCode), path_);
+    throw Error(ErrorCode::kerFileOpenFailed, "http", std::to_string(serverCode), path_);
   }
   // get length
   curl_off_t temp;
@@ -1628,7 +1628,7 @@ void CurlIo::CurlImpl::getDataByRange(size_t lowBlock, size_t highBlock, std::st
   int serverCode;
   curl_easy_getinfo(curl_, CURLINFO_RESPONSE_CODE, &serverCode);  // get code
   if (serverCode >= 400 || serverCode < 0) {
-    throw Error(ErrorCode::kerFileOpenFailed, "http", Exiv2::Internal::stringFormat("%d", serverCode), path_);
+    throw Error(ErrorCode::kerFileOpenFailed, "http", std::to_string(serverCode), path_);
   }
 }
 
@@ -1676,7 +1676,7 @@ void CurlIo::CurlImpl::writeRemote(const byte* data, size_t size, size_t from, s
   int serverCode;
   curl_easy_getinfo(curl_, CURLINFO_RESPONSE_CODE, &serverCode);
   if (serverCode >= 400 || serverCode < 0) {
-    throw Error(ErrorCode::kerFileOpenFailed, "http", Exiv2::Internal::stringFormat("%d", serverCode), path_);
+    throw Error(ErrorCode::kerFileOpenFailed, "http", std::to_string(serverCode), path_);
   }
 }
 

src/bmffimage.cpp

@@ -81,7 +81,7 @@ bool enableBMFF(bool enable) {
 }
 
 std::string Iloc::toString() const {
-  return Internal::stringFormat("ID = %u from,length = %u,%u", ID_, start_, length_);
+  return fmt::format("ID = {} from,length = {},{}", ID_, start_, length_);
 }
 
 BmffImage::BmffImage(BasicIo::UniquePtr io, bool /* create */) :
@@ -269,7 +269,7 @@ uint64_t BmffImage::boxHandler(std::ostream& out /* = std::cout*/, Exiv2::PrintS
   if (bTrace) {
     bLF = true;
     out << Internal::indent(depth) << "Exiv2::BmffImage::boxHandler: " << toAscii(box_type)
-        << Internal::stringFormat(" %8zd->%" PRIu64 " ", address, box_length);
+        << fmt::format(" {:8}->{} ", address, box_length);
   }
 
   if (box_length == 1) {
@@ -366,7 +366,7 @@ uint64_t BmffImage::boxHandler(std::ostream& out /* = std::cout*/, Exiv2::PrintS
         id = " *** XMP ***";
       }
       if (bTrace) {
-        out << Internal::stringFormat("ID = %3d ", ID) << name << " " << id;
+        out << fmt::format("ID = {:3} ", ID) << name << " " << id;
       }
     } break;
 
@@ -444,7 +444,7 @@ uint64_t BmffImage::boxHandler(std::ostream& out /* = std::cout*/, Exiv2::PrintS
           uint32_t ldata = data.read_uint32(skip + step - 4, endian_);
           if (bTrace) {
             out << Internal::indent(depth)
-                << Internal::stringFormat("%8zd | %8zd |   ID | %4u | %6u,%6u", address + skip, step, ID, offset, ldata)
+                << fmt::format("{:8} | {:8} |   ID | {:4} | {:6},{:6}", address + skip, step, ID, offset, ldata)
                 << std::endl;
           }
           // save data for post-processing in meta box
@@ -463,7 +463,7 @@ uint64_t BmffImage::boxHandler(std::ostream& out /* = std::cout*/, Exiv2::PrintS
       uint32_t height = data.read_uint32(skip, endian_);
       skip += 4;
       if (bTrace) {
-        out << "pixelWidth_, pixelHeight_ = " << Internal::stringFormat("%d, %d", width, height);
+        out << fmt::format("pixelWidth_, pixelHeight_ = {}, {}", width, height);
       }
       // HEIC files can have multiple ispe records
       // Store largest width/height
@@ -678,8 +678,8 @@ void BmffImage::parseCr3Preview(const DataBuf& data, std::ostream& out, bool bTr
     return "application/octet-stream";
   }();
   if (bTrace) {
-    out << Internal::stringFormat("width,height,size = %zu,%zu,%zu", nativePreview.width_, nativePreview.height_,
-                                  nativePreview.size_);
+    out << fmt::format("width,height,size = {},{},{}", nativePreview.width_, nativePreview.height_,
+                       nativePreview.size_);
   }
   nativePreviews_.push_back(std::move(nativePreview));
 }

src/futils.cpp

@@ -373,7 +373,7 @@ std::string getProcessPath() {
       return "unknown";  // pathbuf not big enough
     auto path = fs::path(pathbuf);
 #elif defined(__sun__)
-    auto path = fs::read_symlink(Internal::stringFormat("/proc/%d/path/a.out", getpid()));
+    auto path = fs::read_symlink(fmt::format("/proc/%d/path/a.out", getpid()));
 #elif defined(__unix__)
     auto path = fs::read_symlink("/proc/self/exe");
 #endif

src/image.cpp

@@ -350,8 +350,7 @@ void Image::printIFDStructure(BasicIo& io, std::ostream& out, Exiv2::PrintStruct
       throw Error(ErrorCode::kerTiffDirectoryTooLarge);
 
     if (bFirst && bPrint) {
-      out << Internal::indent(depth) << Internal::stringFormat("STRUCTURE OF TIFF FILE (%c%c): ", c, c) << io.path()
-          << std::endl;
+      out << Internal::indent(depth) << fmt::format("STRUCTURE OF TIFF FILE ({}{}): {}", c, c, io.path()) << std::endl;
     }
 
     // Read the dictionary
@@ -435,11 +434,11 @@ void Image::printIFDStructure(BasicIo& io, std::ostream& out, Exiv2::PrintStruct
 
       if (bPrint) {
         const size_t address = start + 2 + i * 12;
-        const std::string offsetString = bOffsetIsPointer ? Internal::stringFormat("%10u", offset) : "";
+        const std::string offsetString = bOffsetIsPointer ? fmt::format("{:9}", offset) : "";
 
         out << Internal::indent(depth)
-            << Internal::stringFormat("%8zu | %#06x %-28s |%10s |%9u |%10s | ", address, tag, tagName(tag).c_str(),
-                                      typeName(type), count, offsetString.c_str());
+            << fmt::format("{:8} | {:#06x} {:<28} | {:>9} | {:>8} | {:9} | ", address, tag, tagName(tag).c_str(),
+                           typeName(type), count, offsetString.c_str());
         if (isShortType(type)) {
           for (size_t k = 0; k < kount; k++) {
             out << sp << byteSwap2(buf, k * size, bSwap);

src/image_int.cpp

@@ -9,32 +9,6 @@
 #include <vector>
 
 namespace Exiv2::Internal {
-std::string stringFormat(const char* format, ...) {
-  std::string result;
-  std::vector<char> buffer;
-  size_t need = std::strlen(format) * 8;  // initial guess
-  int rc = -1;
-
-  // vsnprintf writes at most size (2nd parameter) bytes (including \0)
-  //           returns the number of bytes required for the formatted string excluding \0
-  // the following loop goes through:
-  // one iteration (if 'need' was large enough for the for formatted string)
-  // or two iterations (after the first call to vsnprintf we know the required length)
-  do {
-    buffer.resize(need + 1);
-    va_list args;            // variable arg list
-    va_start(args, format);  // args start after format
-    rc = vsnprintf(buffer.data(), buffer.size(), format, args);
-    va_end(args);  // free the args
-    if (rc > 0)
-      need = static_cast<size_t>(rc);
-  } while (buffer.size() <= need);
-
-  if (rc > 0)
-    result = std::string(buffer.data(), need);
-  return result;
-}
-
 [[nodiscard]] std::string indent(size_t i) {
   return std::string(2 * i, ' ');
 }