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
This commit is contained in:
committed by
Nathan Vegdahl
parent
191f395cac
commit
0b97a13eaa
@@ -2,6 +2,8 @@
|
||||
*
|
||||
* SPDX-License-Identifier: GPL-2.0-or-later */
|
||||
|
||||
#include <fmt/format.h>
|
||||
|
||||
#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;
|
||||
|
||||
@@ -2,6 +2,8 @@
|
||||
*
|
||||
* SPDX-License-Identifier: GPL-2.0-or-later */
|
||||
|
||||
#include <fmt/format.h>
|
||||
|
||||
#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<Error> 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. */
|
||||
|
||||
Reference in New Issue
Block a user