From 13a3dfd788b014e2f4b2c2faf29222012992272c Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Mon, 15 May 2023 09:09:08 +1000 Subject: [PATCH] BLI_path: remove FILE_MAX limit on BLI_path_frame{_range} functions - Use BLI_str_replace_range to avoid a temporary string copy. - Also add a buffer size argument to BLI_path_frame_range. --- source/blender/blenkernel/intern/writeavi.c | 4 +- .../blender/blenkernel/intern/writeffmpeg.c | 4 +- source/blender/blenlib/BLI_path_util.h | 3 +- source/blender/blenlib/intern/path_util.c | 38 ++++++++++--------- .../blenlib/tests/BLI_path_util_test.cc | 11 +++++- 5 files changed, 36 insertions(+), 24 deletions(-) diff --git a/source/blender/blenkernel/intern/writeavi.c b/source/blender/blenkernel/intern/writeavi.c index 541f9acf9fb..daacda12d1e 100644 --- a/source/blender/blenkernel/intern/writeavi.c +++ b/source/blender/blenkernel/intern/writeavi.c @@ -164,13 +164,13 @@ static void filepath_avi(char *filepath, const RenderData *rd, bool preview, con if (rd->scemode & R_EXTENSION) { if (!BLI_path_extension_check(filepath, ".avi")) { - BLI_path_frame_range(filepath, sfra, efra, 4); + BLI_path_frame_range(filepath, FILE_MAX, sfra, efra, 4); BLI_strncat(filepath, ".avi", FILE_MAX); } } else { if (BLI_path_frame_check_chars(filepath)) { - BLI_path_frame_range(filepath, sfra, efra, 4); + BLI_path_frame_range(filepath, FILE_MAX, sfra, efra, 4); } } diff --git a/source/blender/blenkernel/intern/writeffmpeg.c b/source/blender/blenkernel/intern/writeffmpeg.c index 4b6aa04b18f..d6404a1158c 100644 --- a/source/blender/blenkernel/intern/writeffmpeg.c +++ b/source/blender/blenkernel/intern/writeffmpeg.c @@ -1406,7 +1406,7 @@ static void ffmpeg_filepath_get(FFMpegContext *context, if (*fe == NULL) { BLI_strncat(string, autosplit, FILE_MAX); - BLI_path_frame_range(string, sfra, efra, 4); + BLI_path_frame_range(string, FILE_MAX, sfra, efra, 4); BLI_strncat(string, *exts, FILE_MAX); } else { @@ -1417,7 +1417,7 @@ static void ffmpeg_filepath_get(FFMpegContext *context, } else { if (BLI_path_frame_check_chars(string)) { - BLI_path_frame_range(string, sfra, efra, 4); + BLI_path_frame_range(string, FILE_MAX, sfra, efra, 4); } BLI_strncat(string, autosplit, FILE_MAX); diff --git a/source/blender/blenlib/BLI_path_util.h b/source/blender/blenlib/BLI_path_util.h index eb45642b2e9..50dea1bea80 100644 --- a/source/blender/blenlib/BLI_path_util.h +++ b/source/blender/blenlib/BLI_path_util.h @@ -446,7 +446,8 @@ bool BLI_path_frame(char *path, size_t path_maxncpy, int frame, int digits) ATTR * with sta and end as decimal integers, with leading zeroes as necessary, to make digits * digits each, with a hyphen in-between. */ -bool BLI_path_frame_range(char *path, int sta, int end, int digits) ATTR_NONNULL(1); +bool BLI_path_frame_range(char *path, size_t path_maxncpy, int sta, int end, int digits) + ATTR_NONNULL(1); /** * Get the frame from a filename formatted by blender's frame scheme */ diff --git a/source/blender/blenlib/intern/path_util.c b/source/blender/blenlib/intern/path_util.c index 7f2f1c9dbc1..33ab2696329 100644 --- a/source/blender/blenlib/intern/path_util.c +++ b/source/blender/blenlib/intern/path_util.c @@ -45,6 +45,14 @@ static int BLI_path_unc_prefix_len(const char *path); static bool BLI_path_is_abs_win32(const char *path); #endif /* WIN32 */ +/** + * The maximum number of `#` characters expanded for #BLI_path_frame & #BLI_path_frame_range + * Typically 12 is enough and even 16 is very large. + * Use a much larger value so hitting the upper limit is not an issue. + * Exceeding this limit won't fail either, it will just not insert so many leading zeros. + */ +#define FILENAME_FRAME_CHARS_MAX FILE_MAX + int BLI_path_sequence_decode(const char *path, char *head, const size_t head_maxncpy, @@ -885,18 +893,19 @@ bool BLI_path_frame(char *path, size_t path_maxncpy, int frame, int digits) ensure_digits(path, digits); } - if (stringframe_chars(path, &ch_sta, &ch_end)) { /* Warning: `ch_end` is the last # +1. */ - char tmp[FILE_MAX]; - SNPRINTF(tmp, "%.*s%.*d%s", ch_sta, path, ch_end - ch_sta, frame, path + ch_end); - BLI_strncpy(path, tmp, path_maxncpy); + if (stringframe_chars(path, &ch_sta, &ch_end)) { + char frame_str[FILENAME_FRAME_CHARS_MAX + 1]; /* One for null. */ + const int ch_span = MIN2(ch_end - ch_sta, FILENAME_FRAME_CHARS_MAX); + BLI_snprintf(frame_str, sizeof(frame_str), "%.*d", ch_span, frame); + BLI_str_replace_range(path, path_maxncpy, ch_sta, ch_end, frame_str); return true; } return false; } -bool BLI_path_frame_range(char *path, int sta, int end, int digits) +bool BLI_path_frame_range(char *path, size_t path_maxncpy, int sta, int end, int digits) { - BLI_string_debug_size_after_nil(path, FILE_MAX); + BLI_string_debug_size_after_nil(path, path_maxncpy); int ch_sta, ch_end; @@ -904,18 +913,11 @@ bool BLI_path_frame_range(char *path, int sta, int end, int digits) ensure_digits(path, digits); } - if (stringframe_chars(path, &ch_sta, &ch_end)) { /* Warning: `ch_end` is the last # +1. */ - char tmp[FILE_MAX]; - SNPRINTF(tmp, - "%.*s%.*d-%.*d%s", - ch_sta, - path, - ch_end - ch_sta, - sta, - ch_end - ch_sta, - end, - path + ch_end); - BLI_strncpy(path, tmp, FILE_MAX); + if (stringframe_chars(path, &ch_sta, &ch_end)) { + char frame_str[(FILENAME_FRAME_CHARS_MAX * 2) + 1 + 1]; /* One for null, one for the '-' */ + const int ch_span = MIN2(ch_end - ch_sta, FILENAME_FRAME_CHARS_MAX); + BLI_snprintf(frame_str, sizeof(frame_str), "%.*d-%.*d", ch_span, sta, ch_span, end); + BLI_str_replace_range(path, path_maxncpy, ch_sta, ch_end, frame_str); return true; } return false; diff --git a/source/blender/blenlib/tests/BLI_path_util_test.cc b/source/blender/blenlib/tests/BLI_path_util_test.cc index 24e88af43a1..e7940d69be8 100644 --- a/source/blender/blenlib/tests/BLI_path_util_test.cc +++ b/source/blender/blenlib/tests/BLI_path_util_test.cc @@ -638,6 +638,15 @@ TEST(path_util, Frame) EXPECT_TRUE(ret); EXPECT_STREQ(path, "test_-0100"); } + + /* Ensure very large ranges work. */ + { + char path[FILE_MAX * 2]; + memset(path, '#', sizeof(path)); + path[sizeof(path) - 1] = '\0'; + ret = BLI_path_frame(path, sizeof(path), 123456789, 0); + EXPECT_TRUE(BLI_str_endswith(path, "0123456789")); + } } /** \} */ @@ -987,7 +996,7 @@ TEST(path_util, FrameCheckChars) char path[FILE_MAX]; \ bool ret; \ STRNCPY(path, input_path); \ - ret = BLI_path_frame_range(path, sta, end, digits); \ + ret = BLI_path_frame_range(path, sizeof(path), sta, end, digits); \ if (expect_outpath == nullptr) { \ EXPECT_FALSE(ret); \ } \