From 0b97a13eaad147475fda3f4828c0cc6af24a7fe5 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Fri, 9 May 2025 12:45:52 +0200 Subject: [PATCH] Cleanup: Use libfmt functions in path template code The use of `sprintf()` was causing compiler warnings on Apple's version of Clang. This replaces those uses with `fmt::format_to_n()` from libfmt, which is safer and silences those warnings. Additionally, the format string given to `printf()` in debugging functions for the path template unit tests was also causing warnings due to type mismatch (long long vs long). This replaces those with `fmt::print()`, which infers the correct type on its own, silencing those warnings. Pull Request: https://projects.blender.org/blender/blender/pulls/138660 --- .../blenkernel/intern/path_templates.cc | 64 +++++++++++++------ .../blenkernel/intern/path_templates_test.cc | 14 ++-- 2 files changed, 52 insertions(+), 26 deletions(-) diff --git a/source/blender/blenkernel/intern/path_templates.cc b/source/blender/blenkernel/intern/path_templates.cc index a569a397f66..ce1d8309b43 100644 --- a/source/blender/blenkernel/intern/path_templates.cc +++ b/source/blender/blenkernel/intern/path_templates.cc @@ -2,6 +2,8 @@ * * SPDX-License-Identifier: GPL-2.0-or-later */ +#include + #include "BLT_translation.hh" #include "BKE_path_templates.hh" @@ -239,15 +241,22 @@ static int format_int_to_string(const FormatSpecifier &format, switch (format.type) { case FormatSpecifierType::NONE: { - output_length = sprintf(r_output_string, "%ld", integer_value); + output_length = + fmt::format_to_n(r_output_string, FORMAT_BUFFER_SIZE - 1, "{}", integer_value).size; + r_output_string[output_length] = '\0'; break; } case FormatSpecifierType::INTEGER: { BLI_assert(format.integer_digit_count.has_value()); BLI_assert(*format.integer_digit_count > 0); - output_length = sprintf( - r_output_string, "%0*ld", *format.integer_digit_count, integer_value); + output_length = fmt::format_to_n(r_output_string, + FORMAT_BUFFER_SIZE - 1, + "{:0{}}", + integer_value, + *format.integer_digit_count) + .size; + r_output_string[output_length] = '\0'; break; } @@ -261,17 +270,22 @@ static int format_int_to_string(const FormatSpecifier &format, if (format.integer_digit_count.has_value()) { BLI_assert(*format.integer_digit_count > 0); - output_length = sprintf( - r_output_string, "%0*ld", *format.integer_digit_count, integer_value); + output_length = fmt::format_to_n(r_output_string, + FORMAT_BUFFER_SIZE - 1, + "{:0{}}", + integer_value, + *format.integer_digit_count) + .size; } else { - output_length = sprintf(r_output_string, "%ld", integer_value); + output_length = + fmt::format_to_n(r_output_string, FORMAT_BUFFER_SIZE - 1, "{}", integer_value).size; } r_output_string[output_length] = '.'; output_length++; - for (int i = 0; i < *format.fractional_digit_count; i++) { + for (int i = 0; i < *format.fractional_digit_count && i < (FORMAT_BUFFER_SIZE - 1); i++) { r_output_string[output_length] = '0'; output_length++; } @@ -311,12 +325,13 @@ static int format_float_to_string(const FormatSpecifier &format, switch (format.type) { case FormatSpecifierType::NONE: { - /* When no format specification is given, we attempt to approximate - * Python's behavior in the same situation. We can't exactly match via - * `sprintf()`, but we can get pretty close. The only major thing we can't - * replicate via `sprintf()` is that in Python whole numbers are printed - * with a trailing ".0". So we handle that bit manually. */ - output_length = sprintf(r_output_string, "%.16g", float_value); + /* When no format specification is given, we attempt to replicate Python's + * behavior in the same situation. The only major thing we can't replicate + * via libfmt is that in Python whole numbers are printed with a trailing + * ".0". So we handle that bit manually. */ + output_length = + fmt::format_to_n(r_output_string, FORMAT_BUFFER_SIZE - 1, "{}", float_value).size; + r_output_string[output_length] = '\0'; /* If the string consists only of digits and a possible negative sign, then * we append a ".0" to match Python. */ @@ -343,16 +358,25 @@ static int format_float_to_string(const FormatSpecifier &format, if (format.integer_digit_count.has_value()) { /* Both integer and fractional component lengths are specified. */ BLI_assert(*format.integer_digit_count > 0); - output_length = sprintf(r_output_string, - "%0*.*f", - *format.integer_digit_count + *format.fractional_digit_count + 1, - *format.fractional_digit_count, - float_value); + output_length = fmt::format_to_n(r_output_string, + FORMAT_BUFFER_SIZE - 1, + "{:0{}.{}f}", + float_value, + *format.integer_digit_count + + *format.fractional_digit_count + 1, + *format.fractional_digit_count) + .size; + r_output_string[output_length] = '\0'; } else { /* Only fractional component length is specified. */ - output_length = sprintf( - r_output_string, "%.*f", *format.fractional_digit_count, float_value); + output_length = fmt::format_to_n(r_output_string, + FORMAT_BUFFER_SIZE - 1, + "{:.{}f}", + float_value, + *format.fractional_digit_count) + .size; + r_output_string[output_length] = '\0'; } break; diff --git a/source/blender/blenkernel/intern/path_templates_test.cc b/source/blender/blenkernel/intern/path_templates_test.cc index 2b364e59a74..6ab99bb2072 100644 --- a/source/blender/blenkernel/intern/path_templates_test.cc +++ b/source/blender/blenkernel/intern/path_templates_test.cc @@ -2,6 +2,8 @@ * * SPDX-License-Identifier: GPL-2.0-or-later */ +#include + #include "BKE_path_templates.hh" #include "testing/testing.h" @@ -27,17 +29,17 @@ using namespace blender::bke::path_templates; type = "UNKNOWN_VARIABLE"; break; } - printf("(%s, (%ld, %ld))", type, error.byte_range.start(), error.byte_range.size()); + fmt::print("({}, ({}, {}))", type, error.byte_range.start(), error.byte_range.size()); } [[maybe_unused]] static void debug_print_errors(Span errors) { - printf("["); + fmt::print("["); for (const Error &error : errors) { debug_print_error(error); - printf(", "); + fmt::print(", "); } - printf("]\n"); + fmt::print("]\n"); } TEST(path_templates, VariableMap) @@ -133,8 +135,8 @@ TEST(path_templates, path_apply_variables) EXPECT_TRUE(errors.is_empty()); EXPECT_EQ(blender::StringRef(path), - "hello_goodbye_42_7_-7_3.141592653589793_2.718281828459045_29.97002997002997_2.0_-3." - "141592653589793_2e+32_2e-33"); + "hello_goodbye_42_7_-7_3.141592653589793_2.718281828459045_29.970029970029973_2.0_-" + "3.141592653589793_2e+32_2e-33"); } /* Integer formatting. */