Skip to content

Commit 64b5669

Browse files
nehebneheb
committed
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]>
1 parent e8326ba commit 64b5669

23 files changed

+134
-153
lines changed

Diff for: ‎.github/workflows/codeql-analysis.yml

+2-2
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,11 @@ jobs:
3737
steps:
3838
- name: Checkout repository
3939
uses: actions/checkout@v4
40-
40+
4141
- name: Install dependencies
4242
run: |
4343
sudo apt-get update
44-
sudo apt-get install -y libexpat1-dev zlib1g-dev libbrotli-dev libinih-dev
44+
sudo apt-get install -y libfmt-dev libexpat1-dev zlib1g-dev libbrotli-dev libinih-dev
4545
4646
# Initializes the CodeQL tools for scanning.
4747
- name: Initialize CodeQL

Diff for: ‎.github/workflows/on_PR_mac_matrix.yml

+1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ jobs:
2727
run: |
2828
brew install ninja
2929
brew install inih
30+
brew install fmt
3031
brew install googletest
3132
3233
- name: Build

Diff for: ‎.github/workflows/on_PR_mac_special_builds.yml

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ jobs:
2222
run: |
2323
brew install ninja
2424
brew install inih
25+
brew install fmt
2526
brew install googletest
2627
2728
- name: Build

Diff for: ‎.github/workflows/on_PR_meson.yaml

+31-2
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ jobs:
115115
cc:p
116116
cmake:p
117117
curl:p
118+
fmt:p
118119
gtest:p
119120
libinih:p
120121
meson:p
@@ -126,6 +127,34 @@ jobs:
126127
meson setup "${{github.workspace}}/build" -Dauto_features=${{matrix.deps}} -Dwarning_level=3 -Dcpp_std=c++20
127128
meson compile -C "${{github.workspace}}/build" --verbose
128129
meson test -C "${{github.workspace}}/build" --verbose
130+
Cygwin:
131+
runs-on: windows-latest
132+
strategy:
133+
matrix:
134+
deps: ['enabled', 'disabled']
135+
defaults:
136+
run:
137+
shell: msys2 {0}
138+
name: Cygwin-deps=${{matrix.deps}}
139+
steps:
140+
- uses: actions/checkout@v4
141+
- uses: msys2/setup-msys2@v2
142+
with:
143+
msystem: 'MSYS'
144+
install: >-
145+
cmake
146+
gcc
147+
gettext
148+
git
149+
libiconv
150+
meson
151+
ninja
152+
pkgconf
153+
- name: Compile and Test
154+
run: |
155+
meson setup build -Dauto_features=${{matrix.deps}} -Dwarning_level=3 -Dcpp_std=c++20
156+
meson compile -C build --verbose
157+
meson test -C build --verbose
129158
MacOS:
130159
runs-on: macos-latest
131160
name: macOS-deps=${{matrix.deps}}
@@ -137,7 +166,7 @@ jobs:
137166

138167
- name: Install packages
139168
run: |
140-
brew install curl brotli inih expat googletest
169+
brew install fmt curl brotli inih expat googletest
141170
python3 -m pip install meson==0.54.1 ninja
142171
143172
- name: Compile and Test
@@ -154,7 +183,7 @@ jobs:
154183
uses: vmactions/freebsd-vm@v1
155184
with:
156185
prepare: |
157-
pkg install -y cmake curl ninja meson gettext pkgconf googletest expat inih brotli
186+
pkg install -y cmake curl ninja meson gettext pkgconf googletest expat inih brotli libfmt
158187
run: |
159188
meson setup "${{github.workspace}}/build" -Dwarning_level=3 -Dcpp_std=c++20
160189
meson compile -C "${{github.workspace}}/build" --verbose

Diff for: ‎.github/workflows/on_PR_windows_matrix.yml

+1-53
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ jobs:
116116
libiconv:p
117117
libinih:p
118118
zlib:p
119+
fmt:p
119120
120121
- name: Build
121122
run: |
@@ -131,56 +132,3 @@ jobs:
131132
- name: Test
132133
run: |
133134
ctest --test-dir build --output-on-failure
134-
135-
cygwin:
136-
runs-on: windows-latest
137-
strategy:
138-
fail-fast: false
139-
matrix:
140-
build_type: [Release]
141-
shared_libraries: [ON]
142-
platform: [x86_64]
143-
name: Cygwin ${{matrix.platform}} - BuildType:${{matrix.build_type}} - SHARED:${{matrix.shared_libraries}}
144-
env:
145-
SHELLOPTS: igncr
146-
defaults:
147-
run:
148-
shell: C:\cygwin\bin\bash.exe -eo pipefail '{0}'
149-
steps:
150-
# Make sure we don't check out scripts using Windows CRLF line endings
151-
- run: git config --global core.autocrlf input
152-
shell: pwsh
153-
- uses: actions/checkout@v4
154-
155-
- name: Set up Cygwin
156-
uses: cygwin/cygwin-install-action@v4
157-
with:
158-
platform: ${{matrix.platform}}
159-
packages: >-
160-
gcc-g++
161-
cmake
162-
ninja
163-
pkg-config
164-
python3
165-
libbrotli-devel
166-
libcurl-devel
167-
libexpat-devel
168-
libiconv-devel
169-
libinih-devel
170-
zlib-devel
171-
172-
- name: Build
173-
run: |
174-
cmake --preset base_windows \
175-
-DCMAKE_BUILD_TYPE=${{matrix.build_type}} \
176-
-DBUILD_SHARED_LIBS=${{matrix.shared_libraries}} \
177-
-DCONAN_AUTO_INSTALL=OFF \
178-
-DEXIV2_BUILD_SAMPLES=OFF \
179-
-DEXIV2_BUILD_UNIT_TESTS=OFF \
180-
-DEXIV2_TEAM_WARNINGS_AS_ERRORS=OFF \
181-
-S . -B build && \
182-
cmake --build build --parallel
183-
184-
- name: Test
185-
run: |
186-
ctest --test-dir build --output-on-failure

Diff for: ‎.github/workflows/on_push_BasicWinLinMac.yml

+1
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ jobs:
9393
run: |
9494
brew install ninja
9595
brew install inih
96+
brew install fmt
9697
brew install googletest
9798
9899
- name: build and compile

Diff for: ‎CMakeLists.txt

+5
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,11 @@ endif()
9191

9292
include_directories(${CMAKE_BINARY_DIR}) # Make the exv_conf.h file visible for the full project
9393

94+
check_cxx_symbol_exists(__cpp_lib_format "format" HAVE_STD_FORMAT)
95+
if(NOT HAVE_STD_FORMAT)
96+
find_package(fmt REQUIRED)
97+
endif()
98+
9499
if(EXIV2_ENABLE_XMP)
95100
add_subdirectory(xmpsdk)
96101
endif()

Diff for: ‎ci/install_dependencies.sh

+8-8
Original file line numberDiff line numberDiff line change
@@ -41,46 +41,46 @@ distro_id=$(grep '^ID=' /etc/os-release|awk -F = '{print $2}'|sed 's/\"//g')
4141

4242
case "$distro_id" in
4343
'fedora')
44-
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
44+
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
4545
;;
4646

4747
'debian')
4848
apt-get update
49-
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
49+
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
5050
# debian_build_gtest
5151
;;
5252

5353
'arch')
5454
pacman --noconfirm -Syu
55-
pacman --noconfirm --needed -S gcc clang cmake ninja expat zlib brotli libssh curl gtest libinih
55+
pacman --noconfirm --needed -S gcc clang cmake ninja expat zlib brotli libssh curl gtest libinih fmt
5656
;;
5757

5858
'ubuntu')
5959
apt-get update
60-
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
60+
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
6161
# debian_build_gtest
6262
;;
6363

6464
'alpine')
6565
apk update
66-
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
66+
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
6767
;;
6868

6969
'rhel')
7070
dnf clean all
71-
dnf -y install gcc-c++ clang cmake ninja-build expat-devel zlib-devel brotli-devel libssh-devel libcurl-devel inih-devel
71+
dnf -y install gcc-c++ clang cmake ninja-build expat-devel zlib-devel brotli-devel libssh-devel libcurl-devel inih-devel fmt-devel
7272
;;
7373

7474
'centos')
7575
dnf clean all
76-
dnf -y install gcc-c++ clang cmake expat-devel zlib-devel brotli-devel libssh-devel libcurl-devel git
76+
dnf -y install gcc-c++ clang cmake expat-devel zlib-devel brotli-devel libssh-devel libcurl-devel git fmt-devel
7777
dnf -y --enablerepo=crb install ninja-build meson
7878
centos_build_inih
7979
;;
8080

8181
'opensuse-tumbleweed')
8282
zypper --non-interactive refresh
83-
zypper --non-interactive install gcc-c++ clang cmake ninja libexpat-devel zlib-devel libbrotli-devel libssh-devel libcurl-devel gmock libxml2-tools libinih-devel
83+
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
8484
;;
8585
*)
8686
echo "Sorry, no predefined dependencies for your distribution $distro_id exist yet"

Diff for: ‎conanfile.py

+2
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ def requirements(self):
2727

2828
self.requires('inih/55')
2929

30+
self.requires('fmt/10.1.1')
31+
3032
if self.options.webready:
3133
self.requires('libcurl/7.85.0')
3234

Diff for: ‎meson.build

+5-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ project(
66
default_options: ['warning_level=0', 'cpp_std=c++17'],
77
)
88

9-
cargs = []
9+
cargs = ['-D_GNU_SOURCE']
1010
cpp = meson.get_compiler('cpp')
1111
if host_machine.system() == 'windows' and get_option('default_library') != 'static'
1212
cargs += '-DEXIV2API=__declspec(dllexport)'
@@ -54,6 +54,10 @@ deps = []
5454
deps += cpp.find_library('ws2_32', required: host_machine.system() == 'windows')
5555
deps += cpp.find_library('procstat', required: host_machine.system() == 'freebsd')
5656

57+
if not cpp.has_header_symbol('format', '__cpp_lib_format')
58+
deps += dependency('fmt')
59+
endif
60+
5761
if cpp.get_argument_syntax() == 'gcc' and cpp.version().version_compare('<9')
5862
if host_machine.system() == 'linux' and cpp.get_define('_LIBCPP_VERSION', prefix: '#include <new>') == ''
5963
deps += cpp.find_library('stdc++fs')

Diff for: ‎src/CMakeLists.txt

+6
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,12 @@ else()
250250
target_link_libraries(exiv2lib PRIVATE psapi ws2_32 shell32)
251251
endif()
252252

253+
if(NOT HAVE_STD_FORMAT)
254+
target_link_libraries(exiv2lib PRIVATE fmt::fmt)
255+
target_link_libraries(exiv2lib_int PRIVATE fmt::fmt)
256+
list(APPEND requires_private_list "fmt")
257+
endif()
258+
253259
if(EXIV2_ENABLE_PNG)
254260
target_link_libraries(exiv2lib PRIVATE ZLIB::ZLIB)
255261
list(APPEND requires_private_list "zlib")

Diff for: ‎src/bmffimage.cpp

+7-7
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ bool enableBMFF(bool enable) {
8181
}
8282

8383
std::string Iloc::toString() const {
84-
return Internal::stringFormat("ID = %u from,length = %u,%u", ID_, start_, length_);
84+
return stringFormat("ID = {} from,length = {},{}", ID_, start_, length_);
8585
}
8686

8787
BmffImage::BmffImage(BasicIo::UniquePtr io, bool /* create */) :
@@ -269,7 +269,7 @@ uint64_t BmffImage::boxHandler(std::ostream& out /* = std::cout*/, Exiv2::PrintS
269269
if (bTrace) {
270270
bLF = true;
271271
out << Internal::indent(depth) << "Exiv2::BmffImage::boxHandler: " << toAscii(box_type)
272-
<< Internal::stringFormat(" %8zd->%" PRIu64 " ", address, box_length);
272+
<< stringFormat(" {:8}->{} ", address, box_length);
273273
}
274274

275275
if (box_length == 1) {
@@ -366,7 +366,7 @@ uint64_t BmffImage::boxHandler(std::ostream& out /* = std::cout*/, Exiv2::PrintS
366366
id = " *** XMP ***";
367367
}
368368
if (bTrace) {
369-
out << Internal::stringFormat("ID = %3d ", ID) << name << " " << id;
369+
out << stringFormat("ID = {:3} {} {}", ID, name, id);
370370
}
371371
} break;
372372

@@ -444,7 +444,7 @@ uint64_t BmffImage::boxHandler(std::ostream& out /* = std::cout*/, Exiv2::PrintS
444444
uint32_t ldata = data.read_uint32(skip + step - 4, endian_);
445445
if (bTrace) {
446446
out << Internal::indent(depth)
447-
<< Internal::stringFormat("%8zd | %8zd | ID | %4u | %6u,%6u", address + skip, step, ID, offset, ldata)
447+
<< stringFormat("{:8} | {:8} | ID | {:4} | {:6},{:6}", address + skip, step, ID, offset, ldata)
448448
<< std::endl;
449449
}
450450
// save data for post-processing in meta box
@@ -463,7 +463,7 @@ uint64_t BmffImage::boxHandler(std::ostream& out /* = std::cout*/, Exiv2::PrintS
463463
uint32_t height = data.read_uint32(skip, endian_);
464464
skip += 4;
465465
if (bTrace) {
466-
out << "pixelWidth_, pixelHeight_ = " << Internal::stringFormat("%d, %d", width, height);
466+
out << stringFormat("pixelWidth_, pixelHeight_ = {}, {}", width, height);
467467
}
468468
// HEIC files can have multiple ispe records
469469
// Store largest width/height
@@ -678,8 +678,8 @@ void BmffImage::parseCr3Preview(const DataBuf& data, std::ostream& out, bool bTr
678678
return "application/octet-stream";
679679
}();
680680
if (bTrace) {
681-
out << Internal::stringFormat("width,height,size = %zu,%zu,%zu", nativePreview.width_, nativePreview.height_,
682-
nativePreview.size_);
681+
out << stringFormat("width,height,size = {},{},{}", nativePreview.width_, nativePreview.height_,
682+
nativePreview.size_);
683683
}
684684
nativePreviews_.push_back(std::move(nativePreview));
685685
}

Diff for: ‎src/futils.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ std::string getProcessPath() {
373373
return "unknown"; // pathbuf not big enough
374374
auto path = fs::path(pathbuf);
375375
#elif defined(__sun__)
376-
auto path = fs::read_symlink(Internal::stringFormat("/proc/%d/path/a.out", getpid()));
376+
auto path = fs::read_symlink(stringFormat("/proc/%d/path/a.out", getpid()));
377377
#elif defined(__unix__)
378378
auto path = fs::read_symlink("/proc/self/exe");
379379
#endif

Diff for: ‎src/image.cpp

+4-5
Original file line numberDiff line numberDiff line change
@@ -350,8 +350,7 @@ void Image::printIFDStructure(BasicIo& io, std::ostream& out, Exiv2::PrintStruct
350350
throw Error(ErrorCode::kerTiffDirectoryTooLarge);
351351

352352
if (bFirst && bPrint) {
353-
out << Internal::indent(depth) << Internal::stringFormat("STRUCTURE OF TIFF FILE (%c%c): ", c, c) << io.path()
354-
<< std::endl;
353+
out << Internal::indent(depth) << stringFormat("STRUCTURE OF TIFF FILE ({}{}): {}", c, c, io.path()) << std::endl;
355354
}
356355

357356
// Read the dictionary
@@ -435,11 +434,11 @@ void Image::printIFDStructure(BasicIo& io, std::ostream& out, Exiv2::PrintStruct
435434

436435
if (bPrint) {
437436
const size_t address = start + 2 + i * 12;
438-
const std::string offsetString = bOffsetIsPointer ? Internal::stringFormat("%10u", offset) : "";
437+
const std::string offsetString = bOffsetIsPointer ? stringFormat("{:9}", offset) : "";
439438

440439
out << Internal::indent(depth)
441-
<< Internal::stringFormat("%8zu | %#06x %-28s |%10s |%9u |%10s | ", address, tag, tagName(tag).c_str(),
442-
typeName(type), count, offsetString.c_str());
440+
<< stringFormat("{:8} | {:#06x} {:<28} | {:>9} | {:>8} | {:9} | ", address, tag, tagName(tag).c_str(),
441+
typeName(type), count, offsetString);
443442
if (isShortType(type)) {
444443
for (size_t k = 0; k < kount; k++) {
445444
out << sp << byteSwap2(buf, k * size, bSwap);

Diff for: ‎src/image_int.cpp

-26
Original file line numberDiff line numberDiff line change
@@ -9,32 +9,6 @@
99
#include <vector>
1010

1111
namespace Exiv2::Internal {
12-
std::string stringFormat(const char* format, ...) {
13-
std::string result;
14-
std::vector<char> buffer;
15-
size_t need = std::strlen(format) * 8; // initial guess
16-
int rc = -1;
17-
18-
// vsnprintf writes at most size (2nd parameter) bytes (including \0)
19-
// returns the number of bytes required for the formatted string excluding \0
20-
// the following loop goes through:
21-
// one iteration (if 'need' was large enough for the for formatted string)
22-
// or two iterations (after the first call to vsnprintf we know the required length)
23-
do {
24-
buffer.resize(need + 1);
25-
va_list args; // variable arg list
26-
va_start(args, format); // args start after format
27-
rc = vsnprintf(buffer.data(), buffer.size(), format, args);
28-
va_end(args); // free the args
29-
if (rc > 0)
30-
need = static_cast<size_t>(rc);
31-
} while (buffer.size() <= need);
32-
33-
if (rc > 0)
34-
result = std::string(buffer.data(), need);
35-
return result;
36-
}
37-
3812
[[nodiscard]] std::string indent(size_t i) {
3913
return std::string(2 * i, ' ');
4014
}

0 commit comments

Comments
 (0)