From a803dbe7ed80df577b648f9e289aaed2a2cc1700 Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Wed, 19 Oct 2022 12:38:48 -0500 Subject: Geometry Nodes: Use common utility for copying attribute data Attribute copying often uses identical logic for copying selected elements or copying with an index map. Instead of reimplementing this in each file, use the common implementation in the array_utils namespace. This makes the commonality more obvious, gives improved performance (this implementation is multithreaded), reduces binary size (I observed a 173KB reduction), and probably reduces compile time. --- source/blender/blenlib/BLI_array_utils.hh | 5 +++++ source/blender/blenlib/intern/array_utils.cc | 5 +++++ 2 files changed, 10 insertions(+) (limited to 'source/blender/blenlib') diff --git a/source/blender/blenlib/BLI_array_utils.hh b/source/blender/blenlib/BLI_array_utils.hh index 95b3bde10f4..264ac00e034 100644 --- a/source/blender/blenlib/BLI_array_utils.hh +++ b/source/blender/blenlib/BLI_array_utils.hh @@ -39,6 +39,11 @@ inline void copy(const Span src, */ void gather(const GVArray &src, IndexMask indices, GMutableSpan dst, int64_t grain_size = 4096); +/** + * Fill the destination span by gathering indexed values from the `src` array. + */ +void gather(GSpan src, IndexMask indices, GMutableSpan dst, int64_t grain_size = 4096); + /** * Fill the destination span by gathering indexed values from the `src` array. */ diff --git a/source/blender/blenlib/intern/array_utils.cc b/source/blender/blenlib/intern/array_utils.cc index a837d6aceec..2a231228dcb 100644 --- a/source/blender/blenlib/intern/array_utils.cc +++ b/source/blender/blenlib/intern/array_utils.cc @@ -28,4 +28,9 @@ void gather(const GVArray &src, }); } +void gather(const GSpan src, const IndexMask indices, GMutableSpan dst, const int64_t grain_size) +{ + gather(GVArray::ForSpan(src), indices, dst, grain_size); +} + } // namespace blender::array_utils -- cgit v1.2.3 From 84825e4ed2e098954f0c46adee4d65c8c0ba0e99 Mon Sep 17 00:00:00 2001 From: Dalai Felinto Date: Thu, 20 Oct 2022 16:37:07 +0200 Subject: UI: Icon number indicator for data-blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the possibility of having a little number on top of icons. At the moment this is used for: * Outliner * Node Editor bread-crumb * Node Group node header For the outliner there is almost no functional change. It is mostly a refactor to handle the indicators as part of the icon shader instead of the outliner draw code. (note that this was already recently changed in a5d3b648e3e2). The difference is that now we use rounded border rectangle instead of circles, and we can go up to 999 elements. So for the outliner this shows the number of collapsed elements of a certain type (e.g., mesh objects inside a collapsed collection). For the node editors is being used to show the use count for the data-block. This is important for the node editor, so users know whether the node-group they are editing (or are about to edit) is used elsewhere. This is particularly important when the Node Options are hidden, which is the default for node groups appended from the asset libraries. --- Note: This can be easily enabled for ID templates which can then be part of T84669. It just need to call UI_but_icon_indicator_number_set in the function template_add_button_search_menu. --- Special thanks Clément Foucault for the help figuring out the shader, Julian Eisel for the help navigating the UI code, and Pablo Vazquez for the collaboration in this design solution. For images showing the result check the Differential Revision. Differential Revision: https://developer.blender.org/D16284 --- source/blender/blenlib/BLI_string.h | 22 ++++++ source/blender/blenlib/intern/string.c | 30 ++++++++ source/blender/blenlib/tests/BLI_string_test.cc | 97 +++++++++++++++++++++++++ 3 files changed, 149 insertions(+) (limited to 'source/blender/blenlib') diff --git a/source/blender/blenlib/BLI_string.h b/source/blender/blenlib/BLI_string.h index 15926e8f2d2..17abcf52ecc 100644 --- a/source/blender/blenlib/BLI_string.h +++ b/source/blender/blenlib/BLI_string.h @@ -308,6 +308,28 @@ void BLI_str_format_byte_unit(char dst[15], long long int bytes, bool base_10) A * Length of 7 is the maximum of the resulting string, for example, `-15.5K\0`. */ void BLI_str_format_decimal_unit(char dst[7], int number_to_format) ATTR_NONNULL(); +/** + * Format a count to up to 3 places (plus minus sign, plus '\0' terminator) string using long + * number names abbreviations. Used to produce a compact representation of large numbers as + * integers. + * + * It shows a lower bound instead of rounding the number. + * + * 1 -> 1 + * 15 -> 15 + * 155 -> 155 + * 1555 -> 1K + * 15555 -> 15K + * 155555 -> .1M + * 1555555 -> 1M + * 15555555 -> 15M + * 155555555 -> .1B + * 1000000000 -> 1B + * ... + * + * Length of 5 is the maximum of the resulting string, for example, `-15K\0`. + */ +void BLI_str_format_integer_unit(char dst[5], int number_to_format) ATTR_NONNULL(); /** * Compare two strings without regard to case. * diff --git a/source/blender/blenlib/intern/string.c b/source/blender/blenlib/intern/string.c index 89d31c5e93f..755d2dbd55d 100644 --- a/source/blender/blenlib/intern/string.c +++ b/source/blender/blenlib/intern/string.c @@ -1176,4 +1176,34 @@ void BLI_str_format_decimal_unit(char dst[7], int number_to_format) BLI_snprintf(dst, dst_len, "%.*f%s", decimals, number_to_format_converted, units[order]); } +void BLI_str_format_integer_unit(char dst[5], const int number_to_format) +{ + float number_to_format_converted = number_to_format; + int order = 0; + const float base = 1000; + const char *units[] = {"", "K", "M", "B"}; + const int units_num = ARRAY_SIZE(units); + + while ((fabsf(number_to_format_converted) >= base) && ((order + 1) < units_num)) { + number_to_format_converted /= base; + order++; + } + + const bool add_dot = (abs(number_to_format) > 99999) && fabsf(number_to_format_converted) > 99; + + if (add_dot) { + number_to_format_converted /= 100; + order++; + } + + const size_t dst_len = 5; + BLI_snprintf(dst, + dst_len, + "%s%s%d%s", + number_to_format < 0 ? "-" : "", + add_dot ? "." : "", + (int)floorf(fabsf(number_to_format_converted)), + units[order]); +} + /** \} */ diff --git a/source/blender/blenlib/tests/BLI_string_test.cc b/source/blender/blenlib/tests/BLI_string_test.cc index 9bdf6075c70..d726fbccf20 100644 --- a/source/blender/blenlib/tests/BLI_string_test.cc +++ b/source/blender/blenlib/tests/BLI_string_test.cc @@ -515,6 +515,103 @@ TEST(string, StrFormatDecimalUnits) EXPECT_STREQ("-2.1B", size_str); } +/* BLI_str_format_integer_unit */ +TEST(string, StrFormatIntegerUnits) +{ + char size_str[7]; + int size; + + BLI_str_format_integer_unit(size_str, size = 0); + EXPECT_STREQ("0", size_str); + BLI_str_format_integer_unit(size_str, size = 1); + EXPECT_STREQ("1", size_str); + BLI_str_format_integer_unit(size_str, size = 10); + EXPECT_STREQ("10", size_str); + BLI_str_format_integer_unit(size_str, size = 15); + EXPECT_STREQ("15", size_str); + BLI_str_format_integer_unit(size_str, size = 100); + EXPECT_STREQ("100", size_str); + BLI_str_format_integer_unit(size_str, size = 155); + EXPECT_STREQ("155", size_str); + BLI_str_format_integer_unit(size_str, size = 1000); + EXPECT_STREQ("1K", size_str); + BLI_str_format_integer_unit(size_str, size = 1555); + EXPECT_STREQ("1K", size_str); + BLI_str_format_integer_unit(size_str, size = 10000); + EXPECT_STREQ("10K", size_str); + BLI_str_format_integer_unit(size_str, size = 15555); + EXPECT_STREQ("15K", size_str); + BLI_str_format_integer_unit(size_str, size = 100000); + EXPECT_STREQ(".1M", size_str); + BLI_str_format_integer_unit(size_str, size = 155555); + EXPECT_STREQ(".1M", size_str); + BLI_str_format_integer_unit(size_str, size = 1000000); + EXPECT_STREQ("1M", size_str); + BLI_str_format_integer_unit(size_str, size = 1555555); + EXPECT_STREQ("1M", size_str); + BLI_str_format_integer_unit(size_str, size = 2555555); + EXPECT_STREQ("2M", size_str); + BLI_str_format_integer_unit(size_str, size = 10000000); + EXPECT_STREQ("10M", size_str); + BLI_str_format_integer_unit(size_str, size = 15555555); + EXPECT_STREQ("15M", size_str); + BLI_str_format_integer_unit(size_str, size = 100000000); + EXPECT_STREQ(".1B", size_str); + BLI_str_format_integer_unit(size_str, size = 155555555); + EXPECT_STREQ(".1B", size_str); + BLI_str_format_integer_unit(size_str, size = 255555555); + EXPECT_STREQ(".2B", size_str); + BLI_str_format_integer_unit(size_str, size = 1000000000); + EXPECT_STREQ("1B", size_str); + + /* Largest possible value. */ + BLI_str_format_integer_unit(size_str, size = INT32_MAX); + EXPECT_STREQ("2B", size_str); + + BLI_str_format_integer_unit(size_str, size = -0); + EXPECT_STREQ("0", size_str); + BLI_str_format_integer_unit(size_str, size = -1); + EXPECT_STREQ("-1", size_str); + BLI_str_format_integer_unit(size_str, size = -10); + EXPECT_STREQ("-10", size_str); + BLI_str_format_integer_unit(size_str, size = -15); + EXPECT_STREQ("-15", size_str); + BLI_str_format_integer_unit(size_str, size = -100); + EXPECT_STREQ("-100", size_str); + BLI_str_format_integer_unit(size_str, size = -155); + EXPECT_STREQ("-155", size_str); + BLI_str_format_integer_unit(size_str, size = -1000); + EXPECT_STREQ("-1K", size_str); + BLI_str_format_integer_unit(size_str, size = -1555); + EXPECT_STREQ("-1K", size_str); + BLI_str_format_integer_unit(size_str, size = -10000); + EXPECT_STREQ("-10K", size_str); + BLI_str_format_integer_unit(size_str, size = -15555); + EXPECT_STREQ("-15K", size_str); + BLI_str_format_integer_unit(size_str, size = -100000); + EXPECT_STREQ("-.1M", size_str); + BLI_str_format_integer_unit(size_str, size = -155555); + EXPECT_STREQ("-.1M", size_str); + BLI_str_format_integer_unit(size_str, size = -1000000); + EXPECT_STREQ("-1M", size_str); + BLI_str_format_integer_unit(size_str, size = -1555555); + EXPECT_STREQ("-1M", size_str); + BLI_str_format_integer_unit(size_str, size = -10000000); + EXPECT_STREQ("-10M", size_str); + BLI_str_format_integer_unit(size_str, size = -15555555); + EXPECT_STREQ("-15M", size_str); + BLI_str_format_integer_unit(size_str, size = -100000000); + EXPECT_STREQ("-.1B", size_str); + BLI_str_format_integer_unit(size_str, size = -155555555); + EXPECT_STREQ("-.1B", size_str); + BLI_str_format_integer_unit(size_str, size = -1000000000); + EXPECT_STREQ("-1B", size_str); + + /* Smallest possible value. */ + BLI_str_format_integer_unit(size_str, size = -INT32_MAX); + EXPECT_STREQ("-2B", size_str); +} + struct WordInfo { WordInfo() = default; WordInfo(int start, int end) : start(start), end(end) -- cgit v1.2.3 From d66f24cfe30d26e03863a78de9fd58bb3b65ed43 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Sun, 30 Oct 2022 15:34:02 +1100 Subject: Fix potential buffer overflow with BLI_path_slash_ensure use BLI_path_slash_ensure was appending to fixed sized buffers without a size check. --- source/blender/blenlib/BLI_path_util.h | 4 ++-- source/blender/blenlib/intern/fileops.c | 2 +- source/blender/blenlib/intern/path_util.c | 18 +++++++++++------- 3 files changed, 14 insertions(+), 10 deletions(-) (limited to 'source/blender/blenlib') diff --git a/source/blender/blenlib/BLI_path_util.h b/source/blender/blenlib/BLI_path_util.h index d4d2ddead71..9c661178322 100644 --- a/source/blender/blenlib/BLI_path_util.h +++ b/source/blender/blenlib/BLI_path_util.h @@ -218,7 +218,7 @@ const char *BLI_path_slash_rfind(const char *string) ATTR_NONNULL() ATTR_WARN_UN * Appends a slash to string if there isn't one there already. * Returns the new length of the string. */ -int BLI_path_slash_ensure(char *string) ATTR_NONNULL(); +int BLI_path_slash_ensure(char *string, size_t string_maxlen) ATTR_NONNULL(1); /** * Removes the last slash and everything after it to the end of string, if there is one. */ @@ -314,7 +314,7 @@ void BLI_path_normalize(const char *relabase, char *path) ATTR_NONNULL(2); * * \note Same as #BLI_path_normalize but adds a trailing slash. */ -void BLI_path_normalize_dir(const char *relabase, char *dir) ATTR_NONNULL(2); +void BLI_path_normalize_dir(const char *relabase, char *dir, size_t dir_maxlen) ATTR_NONNULL(2); /** * Make given name safe to be used in paths. diff --git a/source/blender/blenlib/intern/fileops.c b/source/blender/blenlib/intern/fileops.c index a157302e51e..005de1f85b4 100644 --- a/source/blender/blenlib/intern/fileops.c +++ b/source/blender/blenlib/intern/fileops.c @@ -401,7 +401,7 @@ static bool delete_recursive(const char *dir) /* dir listing produces dir path without trailing slash... */ BLI_strncpy(path, fl->path, sizeof(path)); - BLI_path_slash_ensure(path); + BLI_path_slash_ensure(path, sizeof(path)); if (delete_recursive(path)) { err = true; diff --git a/source/blender/blenlib/intern/path_util.c b/source/blender/blenlib/intern/path_util.c index afe8c3cc033..759a1bf0dc7 100644 --- a/source/blender/blenlib/intern/path_util.c +++ b/source/blender/blenlib/intern/path_util.c @@ -218,7 +218,7 @@ void BLI_path_normalize(const char *relabase, char *path) #endif } -void BLI_path_normalize_dir(const char *relabase, char *dir) +void BLI_path_normalize_dir(const char *relabase, char *dir, size_t dir_maxlen) { /* Would just create an unexpected "/" path, just early exit entirely. */ if (dir[0] == '\0') { @@ -226,7 +226,7 @@ void BLI_path_normalize_dir(const char *relabase, char *dir) } BLI_path_normalize(relabase, dir); - BLI_path_slash_ensure(dir); + BLI_path_slash_ensure(dir, dir_maxlen); } bool BLI_filename_make_safe_ex(char *fname, bool allow_tokens) @@ -1624,7 +1624,7 @@ bool BLI_path_contains(const char *container_path, const char *containee_path) /* Add a trailing slash to prevent same-prefix directories from matching. * e.g. "/some/path" doesn't contain "/some/path_lib". */ - BLI_path_slash_ensure(container_native); + BLI_path_slash_ensure(container_native, sizeof(container_native)); return BLI_str_startswith(containee_native, container_native); } @@ -1659,13 +1659,17 @@ const char *BLI_path_slash_rfind(const char *string) return (lfslash > lbslash) ? lfslash : lbslash; } -int BLI_path_slash_ensure(char *string) +int BLI_path_slash_ensure(char *string, size_t string_maxlen) { int len = strlen(string); + BLI_assert(len < string_maxlen); if (len == 0 || string[len - 1] != SEP) { - string[len] = SEP; - string[len + 1] = '\0'; - return len + 1; + /* Avoid unlikely buffer overflow. */ + if (len + 1 < string_maxlen) { + string[len] = SEP; + string[len + 1] = '\0'; + return len + 1; + } } return len; } -- cgit v1.2.3 From 5392f220f024b59d199dfc40ef4817401f22ee2b Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Sun, 30 Oct 2022 15:44:17 +1100 Subject: BLI_path: add BLI_path_append_dir (appends and ensures trailing slash) Avoid copying the string then calling BLI_path_slash_ensure afterwards. --- source/blender/blenlib/BLI_path_util.h | 9 ++++++++- source/blender/blenlib/intern/path_util.c | 21 +++++++++++++++++---- 2 files changed, 25 insertions(+), 5 deletions(-) (limited to 'source/blender/blenlib') diff --git a/source/blender/blenlib/BLI_path_util.h b/source/blender/blenlib/BLI_path_util.h index 9c661178322..6d411b51f85 100644 --- a/source/blender/blenlib/BLI_path_util.h +++ b/source/blender/blenlib/BLI_path_util.h @@ -68,8 +68,15 @@ const char *BLI_path_extension(const char *filepath) ATTR_NONNULL(); /** * Append a filename to a dir, ensuring slash separates. + * \return The new length of `dst`. */ -void BLI_path_append(char *__restrict dst, size_t maxlen, const char *__restrict file) +size_t BLI_path_append(char *__restrict dst, size_t maxlen, const char *__restrict file) + ATTR_NONNULL(); +/** + * A version of #BLI_path_append that ensures a trailing slash if there is space in `dst`. + * \return The new length of `dst`. + */ +size_t BLI_path_append_dir(char *__restrict dst, size_t maxlen, const char *__restrict dir) ATTR_NONNULL(); /** diff --git a/source/blender/blenlib/intern/path_util.c b/source/blender/blenlib/intern/path_util.c index 759a1bf0dc7..bb87a26dada 100644 --- a/source/blender/blenlib/intern/path_util.c +++ b/source/blender/blenlib/intern/path_util.c @@ -1431,21 +1431,34 @@ const char *BLI_path_extension(const char *filepath) return extension; } -void BLI_path_append(char *__restrict dst, const size_t maxlen, const char *__restrict file) +size_t BLI_path_append(char *__restrict dst, const size_t maxlen, const char *__restrict file) { size_t dirlen = BLI_strnlen(dst, maxlen); - /* inline BLI_path_slash_ensure */ + /* Inline #BLI_path_slash_ensure. */ if ((dirlen > 0) && (dst[dirlen - 1] != SEP)) { dst[dirlen++] = SEP; dst[dirlen] = '\0'; } if (dirlen >= maxlen) { - return; /* fills the path */ + return dirlen; /* fills the path */ } - BLI_strncpy(dst + dirlen, file, maxlen - dirlen); + return dirlen + BLI_strncpy_rlen(dst + dirlen, file, maxlen - dirlen); +} + +size_t BLI_path_append_dir(char *__restrict dst, const size_t maxlen, const char *__restrict dir) +{ + size_t dirlen = BLI_path_append(dst, maxlen, dir); + if (dirlen + 1 < maxlen) { + /* Inline #BLI_path_slash_ensure. */ + if ((dirlen > 0) && (dst[dirlen - 1] != SEP)) { + dst[dirlen++] = SEP; + dst[dirlen] = '\0'; + } + } + return dirlen; } size_t BLI_path_join_array(char *__restrict dst, -- cgit v1.2.3 From 06abc9509dea9d5cd1cd49121bec9872d47e0ae4 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Mon, 31 Oct 2022 11:15:38 +1100 Subject: Cleanup: quiet warning building with MSVC (_CONCAT redefined) This is defined by a system header (xatomic.h) with MSVC. --- source/blender/blenlib/intern/kdtree_impl.h | 6 ++-- source/blender/blenlib/intern/list_sort_impl.h | 49 ++++++++++++++------------ 2 files changed, 29 insertions(+), 26 deletions(-) (limited to 'source/blender/blenlib') diff --git a/source/blender/blenlib/intern/kdtree_impl.h b/source/blender/blenlib/intern/kdtree_impl.h index 6614f1bf964..f7993eb5adc 100644 --- a/source/blender/blenlib/intern/kdtree_impl.h +++ b/source/blender/blenlib/intern/kdtree_impl.h @@ -11,9 +11,9 @@ #include "BLI_strict_flags.h" #include "BLI_utildefines.h" -#define _CONCAT_AUX(MACRO_ARG1, MACRO_ARG2) MACRO_ARG1##MACRO_ARG2 -#define _CONCAT(MACRO_ARG1, MACRO_ARG2) _CONCAT_AUX(MACRO_ARG1, MACRO_ARG2) -#define BLI_kdtree_nd_(id) _CONCAT(KDTREE_PREFIX_ID, _##id) +#define _BLI_KDTREE_CONCAT_AUX(MACRO_ARG1, MACRO_ARG2) MACRO_ARG1##MACRO_ARG2 +#define _BLI_KDTREE_CONCAT(MACRO_ARG1, MACRO_ARG2) _BLI_KDTREE_CONCAT_AUX(MACRO_ARG1, MACRO_ARG2) +#define BLI_kdtree_nd_(id) _BLI_KDTREE_CONCAT(KDTREE_PREFIX_ID, _##id) typedef struct KDTreeNode_head { uint left, right; diff --git a/source/blender/blenlib/intern/list_sort_impl.h b/source/blender/blenlib/intern/list_sort_impl.h index e1b93986f4a..7c38fc60b29 100644 --- a/source/blender/blenlib/intern/list_sort_impl.h +++ b/source/blender/blenlib/intern/list_sort_impl.h @@ -47,24 +47,25 @@ #endif #ifdef SORT_IMPL_USE_THUNK -# define THUNK_APPEND1(a, thunk) a, thunk -# define THUNK_PREPEND2(thunk, a, b) thunk, a, b +# define BLI_LIST_THUNK_APPEND1(a, thunk) a, thunk +# define BLI_LIST_THUNK_PREPEND2(thunk, a, b) thunk, a, b #else -# define THUNK_APPEND1(a, thunk) a -# define THUNK_PREPEND2(thunk, a, b) a, b +# define BLI_LIST_THUNK_APPEND1(a, thunk) a +# define BLI_LIST_THUNK_PREPEND2(thunk, a, b) a, b #endif -#define _CONCAT_AUX(MACRO_ARG1, MACRO_ARG2) MACRO_ARG1##MACRO_ARG2 -#define _CONCAT(MACRO_ARG1, MACRO_ARG2) _CONCAT_AUX(MACRO_ARG1, MACRO_ARG2) -#define _SORT_PREFIX(id) _CONCAT(SORT_IMPL_FUNC, _##id) +#define _BLI_LIST_SORT_CONCAT_AUX(MACRO_ARG1, MACRO_ARG2) MACRO_ARG1##MACRO_ARG2 +#define _BLI_LIST_SORT_CONCAT(MACRO_ARG1, MACRO_ARG2) \ + _BLI_LIST_SORT_CONCAT_AUX(MACRO_ARG1, MACRO_ARG2) +#define _BLI_LIST_SORT_PREFIX(id) _BLI_LIST_SORT_CONCAT(SORT_IMPL_FUNC, _##id) /* local identifiers */ -#define SortInfo _SORT_PREFIX(SortInfo) -#define CompareFn _SORT_PREFIX(CompareFn) -#define init_sort_info _SORT_PREFIX(init_sort_info) -#define merge_lists _SORT_PREFIX(merge_lists) -#define sweep_up _SORT_PREFIX(sweep_up) -#define insert_list _SORT_PREFIX(insert_list) +#define SortInfo _BLI_LIST_SORT_PREFIX(SortInfo) +#define CompareFn _BLI_LIST_SORT_PREFIX(CompareFn) +#define init_sort_info _BLI_LIST_SORT_PREFIX(init_sort_info) +#define merge_lists _BLI_LIST_SORT_PREFIX(merge_lists) +#define sweep_up _BLI_LIST_SORT_PREFIX(sweep_up) +#define insert_list _BLI_LIST_SORT_PREFIX(insert_list) typedef int (*CompareFn)( #ifdef SORT_IMPL_USE_THUNK @@ -159,7 +160,7 @@ BLI_INLINE list_node *merge_lists(list_node *first, list_node *list = NULL; list_node **pos = &list; while (first && second) { - if (func(THUNK_PREPEND2(thunk, SORT_ARG(first), SORT_ARG(second))) > 0) { + if (func(BLI_LIST_THUNK_PREPEND2(thunk, SORT_ARG(first), SORT_ARG(second))) > 0) { *pos = second; second = second->next; } @@ -181,7 +182,7 @@ BLI_INLINE list_node *sweep_up(struct SortInfo *si, list_node *list, unsigned in { unsigned int i; for (i = si->min_rank; i < upto; i++) { - list = merge_lists(si->ranks[i], list, THUNK_APPEND1(si->func, si->thunk)); + list = merge_lists(si->ranks[i], list, BLI_LIST_THUNK_APPEND1(si->func, si->thunk)); si->ranks[i] = NULL; } return list; @@ -225,17 +226,19 @@ BLI_INLINE void insert_list(struct SortInfo *si, list_node *list, unsigned int r // printf("Rank '%d' should not exceed " STRINGIFY(MAX_RANKS), rank); rank = MAX_RANKS; } - list = merge_lists(sweep_up(si, NULL, si->n_ranks), list, THUNK_APPEND1(si->func, si->thunk)); + list = merge_lists( + sweep_up(si, NULL, si->n_ranks), list, BLI_LIST_THUNK_APPEND1(si->func, si->thunk)); for (i = si->n_ranks; i < rank; i++) { si->ranks[i] = NULL; } } else { if (rank) { - list = merge_lists(sweep_up(si, NULL, rank), list, THUNK_APPEND1(si->func, si->thunk)); + list = merge_lists( + sweep_up(si, NULL, rank), list, BLI_LIST_THUNK_APPEND1(si->func, si->thunk)); } for (i = rank; i < si->n_ranks && si->ranks[i]; i++) { - list = merge_lists(si->ranks[i], list, THUNK_APPEND1(si->func, si->thunk)); + list = merge_lists(si->ranks[i], list, BLI_LIST_THUNK_APPEND1(si->func, si->thunk)); si->ranks[i] = NULL; } } @@ -281,7 +284,7 @@ BLI_INLINE list_node *list_sort_do(list_node *list, list_node *next = list->next; list_node *tail = next->next; - if (func(THUNK_PREPEND2(thunk, SORT_ARG(list), SORT_ARG(next))) > 0) { + if (func(BLI_LIST_THUNK_PREPEND2(thunk, SORT_ARG(list), SORT_ARG(next))) > 0) { next->next = list; next = list; list = list->next; @@ -296,8 +299,8 @@ BLI_INLINE list_node *list_sort_do(list_node *list, return sweep_up(&si, list, si.n_ranks); } -#undef _CONCAT_AUX -#undef _CONCAT +#undef _BLI_LIST_SORT_CONCAT_AUX +#undef _BLI_LIST_SORT_CONCAT #undef _SORT_PREFIX #undef SortInfo @@ -310,6 +313,6 @@ BLI_INLINE list_node *list_sort_do(list_node *list, #undef list_node #undef list_sort_do -#undef THUNK_APPEND1 -#undef THUNK_PREPEND2 +#undef BLI_LIST_THUNK_APPEND1 +#undef BLI_LIST_THUNK_PREPEND2 #undef SORT_ARG -- cgit v1.2.3 From 8f7ab1bf46d5e8610b167180b7631ff62e718a08 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Mon, 31 Oct 2022 13:04:30 +1100 Subject: BLI_path: only operate on native path slashes for BLI_path_join Previously both slashes were considered when joining paths, meaning slashes that were part of the path name could be stripped before joining the path. Prefer using the native path separator for low level path functions, callers can always convert slashes into the expected direction if they need. This also matches BLI_path_append behavior. --- source/blender/blenlib/intern/path_util.c | 11 ++--- source/blender/blenlib/tests/BLI_path_util_test.cc | 47 +++++++++++++++++++--- 2 files changed, 47 insertions(+), 11 deletions(-) (limited to 'source/blender/blenlib') diff --git a/source/blender/blenlib/intern/path_util.c b/source/blender/blenlib/intern/path_util.c index bb87a26dada..4e3d9be6186 100644 --- a/source/blender/blenlib/intern/path_util.c +++ b/source/blender/blenlib/intern/path_util.c @@ -1487,9 +1487,10 @@ size_t BLI_path_join_array(char *__restrict dst, bool has_trailing_slash = false; if (ofs != 0) { size_t len = ofs; - while ((len != 0) && ELEM(path[len - 1], SEP, ALTSEP)) { + while ((len != 0) && (path[len - 1] == SEP)) { len -= 1; } + if (len != 0) { ofs = len; } @@ -1500,18 +1501,18 @@ size_t BLI_path_join_array(char *__restrict dst, path = path_array[path_index]; has_trailing_slash = false; const char *path_init = path; - while (ELEM(path[0], SEP, ALTSEP)) { + while (path[0] == SEP) { path++; } size_t len = strlen(path); if (len != 0) { - while ((len != 0) && ELEM(path[len - 1], SEP, ALTSEP)) { + while ((len != 0) && (path[len - 1] == SEP)) { len -= 1; } if (len != 0) { /* the very first path may have a slash at the end */ - if (ofs && !ELEM(dst[ofs - 1], SEP, ALTSEP)) { + if (ofs && (dst[ofs - 1] != SEP)) { dst[ofs++] = SEP; if (ofs == dst_last) { break; @@ -1534,7 +1535,7 @@ size_t BLI_path_join_array(char *__restrict dst, } if (has_trailing_slash) { - if ((ofs != dst_last) && (ofs != 0) && (ELEM(dst[ofs - 1], SEP, ALTSEP) == 0)) { + if ((ofs != dst_last) && (ofs != 0) && (dst[ofs - 1] != SEP)) { dst[ofs++] = SEP; } } diff --git a/source/blender/blenlib/tests/BLI_path_util_test.cc b/source/blender/blenlib/tests/BLI_path_util_test.cc index 89e537235db..1120e85c959 100644 --- a/source/blender/blenlib/tests/BLI_path_util_test.cc +++ b/source/blender/blenlib/tests/BLI_path_util_test.cc @@ -201,20 +201,53 @@ TEST(path_util, NameAtIndex_NoneComplexNeg) #undef AT_INDEX -#define JOIN(str_expect, out_size, ...) \ +/* For systems with `/` path separator (non WIN32). */ +#define JOIN_FORWARD_SLASH(str_expect, out_size, ...) \ { \ const char *expect = str_expect; \ char result[(out_size) + 1024]; \ - /* check we don't write past the last byte */ \ + /* Check we don't write past the last byte. */ \ result[out_size] = '\0'; \ BLI_path_join(result, out_size, __VA_ARGS__); \ - /* simplify expected string */ \ + EXPECT_STREQ(result, expect); \ + EXPECT_EQ(result[out_size], '\0'); \ + } \ + ((void)0) + +/* For systems with `\` path separator (WIN32). + * Perform additional manipulation to behave as if input arguments used `\` separators. + * Needed since #BLI_path_join uses native slashes. */ +#define JOIN_BACK_SLASH(str_expect, out_size, ...) \ + { \ + const char *expect = str_expect; \ + char result[(out_size) + 1024]; \ + const char *input_forward_slash[] = {__VA_ARGS__}; \ + char *input_back_slash[ARRAY_SIZE(input_forward_slash)] = {nullptr}; \ + for (int i = 0; i < ARRAY_SIZE(input_forward_slash); i++) { \ + input_back_slash[i] = strdup(input_forward_slash[i]); \ + BLI_str_replace_char(input_back_slash[i], '/', '\\'); \ + } \ + /* Check we don't write past the last byte. */ \ + result[out_size] = '\0'; \ + BLI_path_join_array(result, \ + out_size, \ + const_cast(input_back_slash), \ + ARRAY_SIZE(input_back_slash)); \ BLI_str_replace_char(result, '\\', '/'); \ EXPECT_STREQ(result, expect); \ EXPECT_EQ(result[out_size], '\0'); \ + for (int i = 0; i < ARRAY_SIZE(input_forward_slash); i++) { \ + free(input_back_slash[i]); \ + } \ } \ ((void)0) +#ifdef WIN32 +# define JOIN JOIN_BACK_SLASH +#else +# define JOIN JOIN_FORWARD_SLASH +#endif + /* BLI_path_join */ TEST(path_util, JoinNop) { @@ -293,9 +326,9 @@ TEST(path_util, JoinTruncateLong) TEST(path_util, JoinComplex) { - JOIN("/a/b/c/d/e/f/g/", 100, "/", "\\a/b", "//////c/d", "", "e\\\\", "f", "g//"); - JOIN("/aa/bb/cc/dd/ee/ff/gg/", 100, "/", "\\aa/bb", "//////cc/dd", "", "ee\\\\", "ff", "gg//"); - JOIN("1/2/3/", 100, "1", "////////", "", "2", "3\\"); + JOIN("/a/b/c/d/e/f/g/", 100, "/", "a/b", "//////c/d", "", "e", "f", "g//"); + JOIN("/aa/bb/cc/dd/ee/ff/gg/", 100, "/", "aa/bb", "//////cc/dd", "", "ee", "ff", "gg//"); + JOIN("1/2/3/", 100, "1", "////////", "", "2", "3///"); } TEST(path_util, JoinRelativePrefix) @@ -306,6 +339,8 @@ TEST(path_util, JoinRelativePrefix) } #undef JOIN +#undef JOIN_BACK_SLASH +#undef JOIN_FORWARD_SLASH /* BLI_path_frame */ TEST(path_util, Frame) -- cgit v1.2.3 From 511ae2226473df57e47b439392da387cd355abef Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Mon, 31 Oct 2022 13:21:24 +1100 Subject: BLI_path: only operate on native path slashes for BLI_path_name_at_index Prefer using the native path separator for low level path functions. --- source/blender/blenlib/intern/path_util.c | 4 ++-- source/blender/blenlib/tests/BLI_path_util_test.cc | 24 +++++++++++++--------- 2 files changed, 16 insertions(+), 12 deletions(-) (limited to 'source/blender/blenlib') diff --git a/source/blender/blenlib/intern/path_util.c b/source/blender/blenlib/intern/path_util.c index 4e3d9be6186..b16cf8ea161 100644 --- a/source/blender/blenlib/intern/path_util.c +++ b/source/blender/blenlib/intern/path_util.c @@ -1563,7 +1563,7 @@ bool BLI_path_name_at_index(const char *__restrict path, int i = 0; while (true) { const char c = path[i]; - if (ELEM(c, SEP, ALTSEP, '\0')) { + if (ELEM(c, SEP, '\0')) { if (prev + 1 != i) { prev += 1; if (index_step == index) { @@ -1590,7 +1590,7 @@ bool BLI_path_name_at_index(const char *__restrict path, int i = prev - 1; while (true) { const char c = i >= 0 ? path[i] : '\0'; - if (ELEM(c, SEP, ALTSEP, '\0')) { + if (ELEM(c, SEP, '\0')) { if (prev - 1 != i) { i += 1; if (index_step == index) { diff --git a/source/blender/blenlib/tests/BLI_path_util_test.cc b/source/blender/blenlib/tests/BLI_path_util_test.cc index 1120e85c959..1241c71cf94 100644 --- a/source/blender/blenlib/tests/BLI_path_util_test.cc +++ b/source/blender/blenlib/tests/BLI_path_util_test.cc @@ -72,6 +72,10 @@ TEST(path_util, Clean) #define AT_INDEX(str_input, index_input, str_expect) \ { \ char path[] = str_input; \ + /* Test input assumes forward slash, support back-slash on WIN32. */ \ + if (SEP == '\\') { \ + BLI_str_replace_char(path, '/', '\\'); \ + } \ const char *expect = str_expect; \ int index_output, len_output; \ const bool ret = BLI_path_name_at_index(path, index_input, &index_output, &len_output); \ @@ -166,21 +170,21 @@ TEST(path_util, NameAtIndex_MiscNeg) TEST(path_util, NameAtIndex_MiscComplex) { AT_INDEX("how//now/brown/cow", 0, "how"); - AT_INDEX("//how///now\\/brown/cow", 1, "now"); - AT_INDEX("/how/now\\//brown\\/cow", 2, "brown"); - AT_INDEX("/how/now/brown/cow//\\", 3, "cow"); - AT_INDEX("/how/now/brown/\\cow", 4, nullptr); - AT_INDEX("how/now/brown/\\cow\\", 4, nullptr); + AT_INDEX("//how///now//brown/cow", 1, "now"); + AT_INDEX("/how/now///brown//cow", 2, "brown"); + AT_INDEX("/how/now/brown/cow///", 3, "cow"); + AT_INDEX("/how/now/brown//cow", 4, nullptr); + AT_INDEX("how/now/brown//cow/", 4, nullptr); } TEST(path_util, NameAtIndex_MiscComplexNeg) { AT_INDEX("how//now/brown/cow", -4, "how"); - AT_INDEX("//how///now\\/brown/cow", -3, "now"); - AT_INDEX("/how/now\\//brown\\/cow", -2, "brown"); - AT_INDEX("/how/now/brown/cow//\\", -1, "cow"); - AT_INDEX("/how/now/brown/\\cow", -5, nullptr); - AT_INDEX("how/now/brown/\\cow\\", -5, nullptr); + AT_INDEX("//how///now//brown/cow", -3, "now"); + AT_INDEX("/how/now///brown//cow", -2, "brown"); + AT_INDEX("/how/now/brown/cow///", -1, "cow"); + AT_INDEX("/how/now/brown//cow", -5, nullptr); + AT_INDEX("how/now/brown//cow/", -5, nullptr); } TEST(path_util, NameAtIndex_NoneComplex) -- cgit v1.2.3 From f17fbf80653dc0e1561b30fe03f46e354deb12bf Mon Sep 17 00:00:00 2001 From: Sergey Sharybin Date: Mon, 24 Oct 2022 14:16:37 +0200 Subject: Refactor: Rename Object->obmat to Object->object_to_world Motivation is to disambiguate on the naming level what the matrix actually means. It is very easy to understand the meaning backwards, especially since in Python the name goes the opposite way (it is called `world_matrix` in the Python API). It is important to disambiguate the naming without making developers to look into the comment in the header file (which is also not super clear either). Additionally, more clear naming facilitates the unit verification (or, in this case, space validation) when reading an expression. This patch calls the matrix `object_to_world` which makes it clear from the local code what is it exactly going on. This is only done on DNA level, and a lot of local variables still follow the old naming. A DNA rename is setup in a way that there is no change on the file level, so there should be no regressions at all. The possibility is to add `_matrix` or `_mat` suffix to the name to make it explicit that it is a matrix. Although, not sure if it really helps the readability, or is it something redundant. Differential Revision: https://developer.blender.org/D16328 --- source/blender/blenlib/BLI_math_matrix.h | 2 +- source/blender/blenlib/BLI_uvproject.h | 2 +- source/blender/blenlib/intern/uvproject.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) (limited to 'source/blender/blenlib') diff --git a/source/blender/blenlib/BLI_math_matrix.h b/source/blender/blenlib/BLI_math_matrix.h index 7e1b7c2ba56..538474f58b6 100644 --- a/source/blender/blenlib/BLI_math_matrix.h +++ b/source/blender/blenlib/BLI_math_matrix.h @@ -624,7 +624,7 @@ void BLI_space_transform_apply_normal(const struct SpaceTransform *data, float n void BLI_space_transform_invert_normal(const struct SpaceTransform *data, float no[3]); #define BLI_SPACE_TRANSFORM_SETUP(data, local, target) \ - BLI_space_transform_from_matrices((data), (local)->obmat, (target)->obmat) + BLI_space_transform_from_matrices((data), (local)->object_to_world, (target)->object_to_world) /** \} */ diff --git a/source/blender/blenlib/BLI_uvproject.h b/source/blender/blenlib/BLI_uvproject.h index 75f39de6b7f..a94fd796121 100644 --- a/source/blender/blenlib/BLI_uvproject.h +++ b/source/blender/blenlib/BLI_uvproject.h @@ -15,7 +15,7 @@ struct ProjCameraInfo; /** * Create UV info from the camera, needs to be freed. * - * \param rotmat: can be `obedit->obmat` when uv project is used. + * \param rotmat: can be `obedit->object_to_world` when uv project is used. * \param winx, winy: can be from `scene->r.xsch / ysch`. */ struct ProjCameraInfo *BLI_uvproject_camera_info(struct Object *ob, diff --git a/source/blender/blenlib/intern/uvproject.c b/source/blender/blenlib/intern/uvproject.c index 347166bd57d..0398bf0b3fe 100644 --- a/source/blender/blenlib/intern/uvproject.c +++ b/source/blender/blenlib/intern/uvproject.c @@ -129,7 +129,7 @@ ProjCameraInfo *BLI_uvproject_camera_info(Object *ob, float rotmat[4][4], float uci.camsize = uci.do_persp ? tanf(uci.camangle) : camera->ortho_scale; /* account for scaled cameras */ - copy_m4_m4(uci.caminv, ob->obmat); + copy_m4_m4(uci.caminv, ob->object_to_world); normalize_m4(uci.caminv); if (invert_m4(uci.caminv)) { -- cgit v1.2.3 From 513dfa179f3d7becf2e88da1084741e0c70a8da7 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Tue, 1 Nov 2022 21:30:02 +1100 Subject: Tests: add BLI_path_parent_dir tests, split BLI_path_normalize tests Also enable a test that was disabled with a fix FIXME comment but works. --- source/blender/blenlib/tests/BLI_path_util_test.cc | 97 ++++++++++++---------- 1 file changed, 51 insertions(+), 46 deletions(-) (limited to 'source/blender/blenlib') diff --git a/source/blender/blenlib/tests/BLI_path_util_test.cc b/source/blender/blenlib/tests/BLI_path_util_test.cc index 1241c71cf94..2f0e730129c 100644 --- a/source/blender/blenlib/tests/BLI_path_util_test.cc +++ b/source/blender/blenlib/tests/BLI_path_util_test.cc @@ -13,59 +13,64 @@ /* BLI_path_normalize */ #ifndef _WIN32 -TEST(path_util, Clean) -{ - /* "/./" -> "/" */ - { - char path[FILE_MAX] = "/a/./b/./c/./"; - BLI_path_normalize(nullptr, path); - EXPECT_STREQ("/a/b/c/", path); - } - { - char path[FILE_MAX] = "/./././"; - BLI_path_normalize(nullptr, path); - EXPECT_STREQ("/", path); - } +# define NORMALIZE_WITH_BASEDIR(input, input_base, output) \ + { \ + char path[FILE_MAX] = input; \ + BLI_path_normalize(input_base, path); \ + EXPECT_STREQ(output, path); \ + } \ + ((void)0) - { - char path[FILE_MAX] = "/a/./././b/"; - BLI_path_normalize(nullptr, path); - EXPECT_STREQ("/a/b/", path); - } +# define NORMALIZE(input, output) NORMALIZE_WITH_BASEDIR(input, nullptr, output) - /* "//" -> "/" */ - { - char path[FILE_MAX] = "a////"; - BLI_path_normalize(nullptr, path); - EXPECT_STREQ("a/", path); - } +/* #BLI_path_normalize: "/./" -> "/" */ +TEST(path_util, Clean_Dot) +{ + NORMALIZE("/./", "/"); + NORMALIZE("/a/./b/./c/./", "/a/b/c/"); + NORMALIZE("/./././", "/"); + NORMALIZE("/a/./././b/", "/a/b/"); +} +/* #BLI_path_normalize: "//" -> "/" */ +TEST(path_util, Clean_DoubleSlash) +{ + NORMALIZE("//", "//"); /* Exception, double forward slash. */ + NORMALIZE(".//", "./"); + NORMALIZE("a////", "a/"); + NORMALIZE("./a////", "./a/"); +} +/* #BLI_path_normalize: "foo/bar/../" -> "foo/" */ +TEST(path_util, Clean_Parent) +{ + NORMALIZE("/a/b/c/../../../", "/"); + NORMALIZE("/a/../a/b/../b/c/../c/", "/a/b/c/"); + NORMALIZE_WITH_BASEDIR("//../", "/a/b/c/", "/a/b/"); +} - if (false) /* FIXME */ - { - char path[FILE_MAX] = "./a////"; - BLI_path_normalize(nullptr, path); - EXPECT_STREQ("./a/", path); - } +# undef NORMALIZE_WITH_BASEDIR +# undef NORMALIZE - /* "foo/bar/../" -> "foo/" */ - { - char path[FILE_MAX] = "/a/b/c/../../../"; - BLI_path_normalize(nullptr, path); - EXPECT_STREQ("/", path); - } +#endif /* _WIN32 */ - { - char path[FILE_MAX] = "/a/../a/b/../b/c/../c/"; - BLI_path_normalize(nullptr, path); - EXPECT_STREQ("/a/b/c/", path); - } +/* #BLI_path_parent_dir */ +#ifndef _WIN32 +TEST(path_util, ParentDir) +{ +# define PARENT_DIR(input, output) \ + { \ + char path[FILE_MAX] = input; \ + BLI_path_parent_dir(path); \ + EXPECT_STREQ(output, path); \ + } \ + ((void)0) - { - char path[FILE_MAX] = "//../"; - BLI_path_normalize("/a/b/c/", path); - EXPECT_STREQ("/a/b/", path); - } + PARENT_DIR("/a/b/", "/a/"); + PARENT_DIR("/a/b", "/a/"); + PARENT_DIR("/a", "/"); + PARENT_DIR("/", "/"); + +# undef PARENT_DIR } #endif -- cgit v1.2.3 From 12f4ac170658f82ce85d96873cfb32ec786136ec Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Tue, 1 Nov 2022 21:32:46 +1100 Subject: Fix BLI_path_normalize failing with "." and ".." in the path The logic to go up a directory (using "..") ran before stripping "/./" from the path. This caused "/a/b/./../" to result in "/a/b/" instead of "/a/". Now redundant characters are removed before before checking for ".." in paths. Include test to ensure this works as expected. --- source/blender/blenlib/intern/path_util.c | 36 ++++++++++++---------- source/blender/blenlib/tests/BLI_path_util_test.cc | 6 ++++ 2 files changed, 26 insertions(+), 16 deletions(-) (limited to 'source/blender/blenlib') diff --git a/source/blender/blenlib/intern/path_util.c b/source/blender/blenlib/intern/path_util.c index b16cf8ea161..df18689c74c 100644 --- a/source/blender/blenlib/intern/path_util.c +++ b/source/blender/blenlib/intern/path_util.c @@ -153,6 +153,19 @@ void BLI_path_normalize(const char *relabase, char *path) */ #ifdef WIN32 + + while ((start = strstr(path, "\\.\\"))) { + eind = start + strlen("\\.\\") - 1; + memmove(start, eind, strlen(eind) + 1); + } + + /* remove two consecutive backslashes, but skip the UNC prefix, + * which needs to be preserved */ + while ((start = strstr(path + BLI_path_unc_prefix_len(path), "\\\\"))) { + eind = start + strlen("\\\\") - 1; + memmove(start, eind, strlen(eind) + 1); + } + while ((start = strstr(path, "\\..\\"))) { eind = start + strlen("\\..\\") - 1; a = start - path - 1; @@ -170,18 +183,18 @@ void BLI_path_normalize(const char *relabase, char *path) } } - while ((start = strstr(path, "\\.\\"))) { - eind = start + strlen("\\.\\") - 1; +#else + + while ((start = strstr(path, "/./"))) { + eind = start + (3 - 1) /* strlen("/./") - 1 */; memmove(start, eind, strlen(eind) + 1); } - /* remove two consecutive backslashes, but skip the UNC prefix, - * which needs to be preserved */ - while ((start = strstr(path + BLI_path_unc_prefix_len(path), "\\\\"))) { - eind = start + strlen("\\\\") - 1; + while ((start = strstr(path, "//"))) { + eind = start + (2 - 1) /* strlen("//") - 1 */; memmove(start, eind, strlen(eind) + 1); } -#else + while ((start = strstr(path, "/../"))) { a = start - path - 1; if (a > 0) { @@ -206,15 +219,6 @@ void BLI_path_normalize(const char *relabase, char *path) } } - while ((start = strstr(path, "/./"))) { - eind = start + (3 - 1) /* strlen("/./") - 1 */; - memmove(start, eind, strlen(eind) + 1); - } - - while ((start = strstr(path, "//"))) { - eind = start + (2 - 1) /* strlen("//") - 1 */; - memmove(start, eind, strlen(eind) + 1); - } #endif } diff --git a/source/blender/blenlib/tests/BLI_path_util_test.cc b/source/blender/blenlib/tests/BLI_path_util_test.cc index 2f0e730129c..51db376e03f 100644 --- a/source/blender/blenlib/tests/BLI_path_util_test.cc +++ b/source/blender/blenlib/tests/BLI_path_util_test.cc @@ -32,6 +32,12 @@ TEST(path_util, Clean_Dot) NORMALIZE("/./././", "/"); NORMALIZE("/a/./././b/", "/a/b/"); } +/* #BLI_path_normalize: complex "/./" -> "/", "//" -> "/", "./path/../" -> "./". */ +TEST(path_util, Clean_Complex) +{ + NORMALIZE("/a/./b/./c/./.././.././", "/a/"); + NORMALIZE("/a//.//b//.//c//.//..//.//..//.//", "/a/"); +} /* #BLI_path_normalize: "//" -> "/" */ TEST(path_util, Clean_DoubleSlash) { -- cgit v1.2.3 From cc6f41f8a53f7f0392afa59e6da8ebc3849fc03c Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Tue, 1 Nov 2022 22:02:08 +1100 Subject: Fix BLI_path_parent_dir failing on paths ending with ".." The check for BLI_path_normalize having succeeded only checked for a trailing "../" which isn't correct. This caused going up a directory in the file selector to do nothing on directories ending with "..". This also caused an empty path to expand into "../" because BLI_path_extension_check didn't account for this case. Resolve using BLI_path_name_at_index which extracts the last component of the path without having to match the the surrounding slashes. --- source/blender/blenlib/BLI_path_util.h | 2 ++ source/blender/blenlib/intern/path_util.c | 26 +++++++++++++++++----- source/blender/blenlib/tests/BLI_path_util_test.cc | 11 +++++++++ 3 files changed, 33 insertions(+), 6 deletions(-) (limited to 'source/blender/blenlib') diff --git a/source/blender/blenlib/BLI_path_util.h b/source/blender/blenlib/BLI_path_util.h index 6d411b51f85..1b723ab038d 100644 --- a/source/blender/blenlib/BLI_path_util.h +++ b/source/blender/blenlib/BLI_path_util.h @@ -364,6 +364,8 @@ bool BLI_path_make_safe(char *path) ATTR_NONNULL(1); * * Replaces path with the path of its parent directory, returning true if * it was able to find a parent directory within the path. + * + * On success, the resulting path will always have a trailing slash. */ bool BLI_path_parent_dir(char *path) ATTR_NONNULL(); /** diff --git a/source/blender/blenlib/intern/path_util.c b/source/blender/blenlib/intern/path_util.c index df18689c74c..f46c2e65395 100644 --- a/source/blender/blenlib/intern/path_util.c +++ b/source/blender/blenlib/intern/path_util.c @@ -630,14 +630,28 @@ bool BLI_path_parent_dir(char *path) char tmp[FILE_MAX + 4]; BLI_path_join(tmp, sizeof(tmp), path, parent_dir); - BLI_path_normalize(NULL, tmp); /* does all the work of normalizing the path for us */ - - if (!BLI_path_extension_check(tmp, parent_dir)) { - strcpy(path, tmp); /* We assume the parent directory is always shorter. */ - return true; + /* Does all the work of normalizing the path for us. + * + * NOTE(@campbellbarton): While it's possible strip text after the second last slash, + * this would have to be clever and skip cases like "/./" & multiple slashes. + * Since this ends up solving some of the same problems as #BLI_path_normalize, + * call this function instead of attempting to handle them separately. */ + BLI_path_normalize(NULL, tmp); + + /* Use #BLI_path_name_at_index instead of checking if the strings ends with `parent_dir` + * to ensure the logic isn't confused by: + * - Directory names that happen to end with `..`. + * - When `path` is empty, the contents will be `../` + * which would cause checking for a tailing `/../` fail. + * Extracting the span of the final directory avoids both these issues. */ + int tail_ofs = 0, tail_len = 0; + if (BLI_path_name_at_index(tmp, -1, &tail_ofs, &tail_len) && (tail_len == 2) && + (memcmp(&tmp[tail_ofs], "..", 2) == 0)) { + return false; } - return false; + strcpy(path, tmp); /* We assume the parent directory is always shorter. */ + return true; } bool BLI_path_parent_dir_until_exists(char *dir) diff --git a/source/blender/blenlib/tests/BLI_path_util_test.cc b/source/blender/blenlib/tests/BLI_path_util_test.cc index 51db376e03f..93922b41b23 100644 --- a/source/blender/blenlib/tests/BLI_path_util_test.cc +++ b/source/blender/blenlib/tests/BLI_path_util_test.cc @@ -75,6 +75,17 @@ TEST(path_util, ParentDir) PARENT_DIR("/a/b", "/a/"); PARENT_DIR("/a", "/"); PARENT_DIR("/", "/"); + PARENT_DIR("", ""); + + /* Ensure trailing dots aren't confused with parent path. */ + PARENT_DIR("/.../.../.../", "/.../.../"); + PARENT_DIR("/.../.../...", "/.../.../"); + + PARENT_DIR("/a../b../c../", "/a../b../"); + PARENT_DIR("/a../b../c..", "/a../b../"); + + PARENT_DIR("/a./b./c./", "/a./b./"); + PARENT_DIR("/a./b./c.", "/a./b./"); # undef PARENT_DIR } -- cgit v1.2.3 From db43aa772925d4f6bd311858cd1ee4fe9aebc4e8 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Wed, 2 Nov 2022 14:07:54 +1100 Subject: Fix T102201: File selector shows "\" before folder names on WIN32 Regression in [0] on WIN32 caused joining paths {"//", "path"} to result in "//\path". This made the file selector show paths with a "\" prefix. Add an exception for WIN32 where an initial path of forward slashes is joined without a back-slash. [0]: 8f7ab1bf46d5e8610b167180b7631ff62e718a08 --- source/blender/blenlib/intern/path_util.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'source/blender/blenlib') diff --git a/source/blender/blenlib/intern/path_util.c b/source/blender/blenlib/intern/path_util.c index f46c2e65395..7c08eeedaa2 100644 --- a/source/blender/blenlib/intern/path_util.c +++ b/source/blender/blenlib/intern/path_util.c @@ -1500,6 +1500,27 @@ size_t BLI_path_join_array(char *__restrict dst, return ofs; } +#ifdef WIN32 + /* Special case "//" for relative paths, don't use separator #SEP + * as this has a special meaning on both WIN32 & UNIX. + * Without this check joining `"//", "path"`. results in `"//\path"`. */ + if (ofs != 0) { + size_t i; + for (i = 0; i < ofs; i++) { + if (dst[i] != '/') { + break; + } + } + if (i == ofs) { + /* All slashes, keep them as-is, and join the remaining path array. */ + return path_array_num > 1 ? + BLI_path_join_array( + dst + ofs, dst_len - ofs, &path_array[1], path_array_num - 1) : + ofs; + } + } +#endif + /* Remove trailing slashes, unless there are *only* trailing slashes * (allow `//` or `//some_path` as the first argument). */ bool has_trailing_slash = false; -- cgit v1.2.3 From 52664437fbf8f0c948a30c0cc385e292aa03f5c4 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Wed, 2 Nov 2022 14:26:26 +1100 Subject: Cleanup: use doxy sections for path tests --- source/blender/blenlib/tests/BLI_path_util_test.cc | 105 +++++++++++++++++---- 1 file changed, 86 insertions(+), 19 deletions(-) (limited to 'source/blender/blenlib') diff --git a/source/blender/blenlib/tests/BLI_path_util_test.cc b/source/blender/blenlib/tests/BLI_path_util_test.cc index 93922b41b23..5d64de99ff6 100644 --- a/source/blender/blenlib/tests/BLI_path_util_test.cc +++ b/source/blender/blenlib/tests/BLI_path_util_test.cc @@ -9,9 +9,9 @@ #include "BLI_string.h" /* -------------------------------------------------------------------- */ -/* tests */ +/** \name Tests for: #BLI_path_normalize + * \{ */ -/* BLI_path_normalize */ #ifndef _WIN32 # define NORMALIZE_WITH_BASEDIR(input, input_base, output) \ @@ -57,9 +57,14 @@ TEST(path_util, Clean_Parent) # undef NORMALIZE_WITH_BASEDIR # undef NORMALIZE -#endif /* _WIN32 */ +#endif /* !_WIN32 */ + +/** \} */ + +/* -------------------------------------------------------------------- */ +/** \name Tests for: #BLI_path_parent_dir + * \{ */ -/* #BLI_path_parent_dir */ #ifndef _WIN32 TEST(path_util, ParentDir) { @@ -89,7 +94,13 @@ TEST(path_util, ParentDir) # undef PARENT_DIR } -#endif +#endif /* !_WIN32 */ + +/** \} */ + +/* -------------------------------------------------------------------- */ +/** \name Tests for: #BLI_path_name_at_index + * \{ */ #define AT_INDEX(str_input, index_input, str_expect) \ { \ @@ -113,7 +124,6 @@ TEST(path_util, ParentDir) } \ ((void)0) -/* BLI_path_name_at_index */ TEST(path_util, NameAtIndex_Single) { AT_INDEX("/a", 0, "a"); @@ -227,6 +237,12 @@ TEST(path_util, NameAtIndex_NoneComplexNeg) #undef AT_INDEX +/** \} */ + +/* -------------------------------------------------------------------- */ +/** \name Tests for: #BLI_path_join + * \{ */ + /* For systems with `/` path separator (non WIN32). */ #define JOIN_FORWARD_SLASH(str_expect, out_size, ...) \ { \ @@ -274,7 +290,6 @@ TEST(path_util, NameAtIndex_NoneComplexNeg) # define JOIN JOIN_FORWARD_SLASH #endif -/* BLI_path_join */ TEST(path_util, JoinNop) { JOIN("", 100, ""); @@ -368,7 +383,12 @@ TEST(path_util, JoinRelativePrefix) #undef JOIN_BACK_SLASH #undef JOIN_FORWARD_SLASH -/* BLI_path_frame */ +/** \} */ + +/* -------------------------------------------------------------------- */ +/** \name Tests for: #BLI_path_frame + * \{ */ + TEST(path_util, Frame) { bool ret; @@ -445,7 +465,12 @@ TEST(path_util, Frame) } } -/* BLI_split_dirfile */ +/** \} */ + +/* -------------------------------------------------------------------- */ +/** \name Tests for: #BLI_split_dirfile + * \{ */ + TEST(path_util, SplitDirfile) { { @@ -501,6 +526,12 @@ TEST(path_util, SplitDirfile) } } +/** \} */ + +/* -------------------------------------------------------------------- */ +/** \name Tests for: #BLI_path_frame_strip + * \{ */ + #define PATH_FRAME_STRIP(input_path, expect_path, expect_ext) \ { \ char path[FILE_MAX]; \ @@ -512,7 +543,6 @@ TEST(path_util, SplitDirfile) } \ ((void)0) -/* BLI_path_frame_strip */ TEST(path_util, PathFrameStrip) { PATH_FRAME_STRIP("", "", ""); @@ -524,6 +554,12 @@ TEST(path_util, PathFrameStrip) } #undef PATH_FRAME_STRIP +/** \} */ + +/* -------------------------------------------------------------------- */ +/** \name Tests for: #BLI_path_extension_check + * \{ */ + #define PATH_EXTENSION_CHECK(input_path, input_ext, expect_ext) \ { \ const bool ret = BLI_path_extension_check(input_path, input_ext); \ @@ -536,7 +572,6 @@ TEST(path_util, PathFrameStrip) } \ ((void)0) -/* BLI_path_extension_check */ TEST(path_util, PathExtensionCheck) { PATH_EXTENSION_CHECK("a/b/c.exe", ".exe", ".exe"); @@ -562,6 +597,12 @@ TEST(path_util, PathExtensionCheck) } #undef PATH_EXTENSION_CHECK +/** \} */ + +/* -------------------------------------------------------------------- */ +/** \name Tests for: #BLI_path_frame_check_chars + * \{ */ + #define PATH_FRAME_CHECK_CHARS(input_path, expect_hasChars) \ { \ const bool ret = BLI_path_frame_check_chars(input_path); \ @@ -574,7 +615,6 @@ TEST(path_util, PathExtensionCheck) } \ ((void)0) -/* BLI_path_frame_check_chars */ TEST(path_util, PathFrameCheckChars) { PATH_FRAME_CHECK_CHARS("a#", true); @@ -594,6 +634,12 @@ TEST(path_util, PathFrameCheckChars) } #undef PATH_FRAME_CHECK_CHARS +/** \} */ + +/* -------------------------------------------------------------------- */ +/** \name Tests for: #BLI_path_frame_range + * \{ */ + #define PATH_FRAME_RANGE(input_path, sta, end, digits, expect_outpath) \ { \ char path[FILE_MAX]; \ @@ -610,7 +656,6 @@ TEST(path_util, PathFrameCheckChars) } \ ((void)0) -/* BLI_path_frame_range */ TEST(path_util, PathFrameRange) { int dummy = -1; @@ -626,6 +671,12 @@ TEST(path_util, PathFrameRange) } #undef PATH_FRAME_RANGE +/** \} */ + +/* -------------------------------------------------------------------- */ +/** \name Tests for: #BLI_path_frame_get + * \{ */ + #define PATH_FRAME_GET(input_path, expect_frame, expect_numdigits, expect_pathisvalid) \ { \ char path[FILE_MAX]; \ @@ -643,7 +694,6 @@ TEST(path_util, PathFrameRange) } \ ((void)0) -/* BLI_path_frame_get */ TEST(path_util, PathFrameGet) { PATH_FRAME_GET("001.avi", 1, 3, true); @@ -655,7 +705,12 @@ TEST(path_util, PathFrameGet) } #undef PATH_FRAME_GET -/* BLI_path_extension */ +/** \} */ + +/* -------------------------------------------------------------------- */ +/** \name Tests for: #BLI_path_extension + * \{ */ + TEST(path_util, PathExtension) { EXPECT_EQ(nullptr, BLI_path_extension("some.def/file")); @@ -669,7 +724,12 @@ TEST(path_util, PathExtension) EXPECT_STREQ(".001", BLI_path_extension("Text.001")); } -/* BLI_path_rel. */ +/** \} */ + +/* -------------------------------------------------------------------- */ +/** \name Tests for: #BLI_path_rel + * \{ */ + #ifndef _WIN32 # define PATH_REL(abs_path, ref_path, rel_path) \ @@ -722,9 +782,14 @@ TEST(path_util, PathRelPath) # undef PATH_REL -#endif +#endif /* !_WIN32 */ + +/** \} */ + +/* -------------------------------------------------------------------- */ +/** \name Tests for: #BLI_path_contains + * \{ */ -/* BLI_path_contains */ TEST(path_util, PathContains) { EXPECT_TRUE(BLI_path_contains("/some/path", "/some/path")) << "A path contains itself"; @@ -753,4 +818,6 @@ TEST(path_util, PathContains_Windows_case_insensitive) EXPECT_TRUE(BLI_path_contains("C:\\some\\path", "c:\\SOME\\path\\inside")) << "On Windows path comparison should ignore case"; } -#endif +#endif /* WIN32 */ + +/** \} */ -- cgit v1.2.3 From 7f1e98839a79ef8c5d08b53e01b3d01302a2e419 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Wed, 2 Nov 2022 16:15:48 +1100 Subject: Tests: enable path tests for WIN32 Tests for BLI_path_normalize, BLI_path_parent_dir & BLI_path_rel were disabled on WIN32 because of complications with slash direction. Enable these tests using character replacement to manipulate test data. --- source/blender/blenlib/tests/BLI_path_util_test.cc | 191 +++++++++++++-------- 1 file changed, 124 insertions(+), 67 deletions(-) (limited to 'source/blender/blenlib') diff --git a/source/blender/blenlib/tests/BLI_path_util_test.cc b/source/blender/blenlib/tests/BLI_path_util_test.cc index 5d64de99ff6..379ff432988 100644 --- a/source/blender/blenlib/tests/BLI_path_util_test.cc +++ b/source/blender/blenlib/tests/BLI_path_util_test.cc @@ -9,20 +9,59 @@ #include "BLI_string.h" /* -------------------------------------------------------------------- */ -/** \name Tests for: #BLI_path_normalize +/** \name Local Utilities * \{ */ -#ifndef _WIN32 +static void str_replace_char_with_relative_exception(char *str, char src, char dst) +{ + /* Always keep "//" or more leading slashes (special meaning). */ + if (src == '/') { + if (str[0] == '/' && str[1] == '/') { + str += 2; + while (*str == '/') { + str++; + } + } + } + BLI_str_replace_char(str, src, dst); +} + +static char *str_replace_char_strdup(const char *str, char src, char dst) +{ + if (str == nullptr) { + return nullptr; + } + char *str_dupe = strdup(str); + BLI_str_replace_char(str_dupe, src, dst); + return str_dupe; +} + +/** \} */ + +/* -------------------------------------------------------------------- */ +/** \name Tests for: #BLI_path_normalize + * \{ */ -# define NORMALIZE_WITH_BASEDIR(input, input_base, output) \ - { \ - char path[FILE_MAX] = input; \ - BLI_path_normalize(input_base, path); \ - EXPECT_STREQ(output, path); \ +#define NORMALIZE_WITH_BASEDIR(input, input_base, output) \ + { \ + char path[FILE_MAX] = input; \ + const char *input_base_test = input_base; \ + if (SEP == '\\') { \ + str_replace_char_with_relative_exception(path, '/', '\\'); \ + input_base_test = str_replace_char_strdup(input_base_test, '/', '\\'); \ } \ - ((void)0) + BLI_path_normalize(input_base_test, path); \ + if (SEP == '\\') { \ + BLI_str_replace_char(path, '\\', '/'); \ + if (input_base_test) { \ + free((void *)input_base_test); \ + } \ + } \ + EXPECT_STREQ(output, path); \ + } \ + ((void)0) -# define NORMALIZE(input, output) NORMALIZE_WITH_BASEDIR(input, nullptr, output) +#define NORMALIZE(input, output) NORMALIZE_WITH_BASEDIR(input, nullptr, output) /* #BLI_path_normalize: "/./" -> "/" */ TEST(path_util, Clean_Dot) @@ -54,10 +93,8 @@ TEST(path_util, Clean_Parent) NORMALIZE_WITH_BASEDIR("//../", "/a/b/c/", "/a/b/"); } -# undef NORMALIZE_WITH_BASEDIR -# undef NORMALIZE - -#endif /* !_WIN32 */ +#undef NORMALIZE_WITH_BASEDIR +#undef NORMALIZE /** \} */ @@ -65,16 +102,21 @@ TEST(path_util, Clean_Parent) /** \name Tests for: #BLI_path_parent_dir * \{ */ -#ifndef _WIN32 TEST(path_util, ParentDir) { -# define PARENT_DIR(input, output) \ - { \ - char path[FILE_MAX] = input; \ - BLI_path_parent_dir(path); \ - EXPECT_STREQ(output, path); \ +#define PARENT_DIR(input, output) \ + { \ + char path[FILE_MAX] = input; \ + if (SEP == '\\') { \ + BLI_str_replace_char(path, '/', '\\'); \ } \ - ((void)0) + BLI_path_parent_dir(path); \ + if (SEP == '\\') { \ + BLI_str_replace_char(path, '\\', '/'); \ + } \ + EXPECT_STREQ(output, path); \ + } \ + ((void)0) PARENT_DIR("/a/b/", "/a/"); PARENT_DIR("/a/b", "/a/"); @@ -92,9 +134,8 @@ TEST(path_util, ParentDir) PARENT_DIR("/a./b./c./", "/a./b./"); PARENT_DIR("/a./b./c.", "/a./b./"); -# undef PARENT_DIR +#undef PARENT_DIR } -#endif /* !_WIN32 */ /** \} */ @@ -730,59 +771,75 @@ TEST(path_util, PathExtension) /** \name Tests for: #BLI_path_rel * \{ */ -#ifndef _WIN32 - -# define PATH_REL(abs_path, ref_path, rel_path) \ - { \ - char path[FILE_MAX]; \ - BLI_strncpy(path, abs_path, sizeof(path)); \ - BLI_path_rel(path, ref_path); \ - EXPECT_STREQ(rel_path, path); \ +#define PATH_REL(abs_path, ref_path, rel_path) \ + { \ + char path[FILE_MAX]; \ + const char *ref_path_test = ref_path; \ + BLI_strncpy(path, abs_path, sizeof(path)); \ + if (SEP == '\\') { \ + BLI_str_replace_char(path, '/', '\\'); \ + ref_path_test = str_replace_char_strdup(ref_path_test, '/', '\\'); \ + } \ + BLI_path_rel(path, ref_path_test); \ + if (SEP == '\\') { \ + BLI_str_replace_char(path, '\\', '/'); \ + free((void *)ref_path_test); \ } \ - void(0) + EXPECT_STREQ(rel_path, path); \ + } \ + void(0) + +#ifdef WIN32 +# define ABS_PREFIX "C:" +#else +# define ABS_PREFIX "" +#endif + +TEST(path_util, PathRelPath_Simple) +{ + PATH_REL(ABS_PREFIX "/foo/bar/blender.blend", ABS_PREFIX "/foo/bar/", "//blender.blend"); +} -TEST(path_util, PathRelPath) +TEST(path_util, PathRelPath_SimpleSubdir) { - PATH_REL("/foo/bar/blender.blend", "/foo/bar/", "//blender.blend"); - PATH_REL("/foo/bar/blender.blend", "/foo/bar", "//bar/blender.blend"); + PATH_REL(ABS_PREFIX "/foo/bar/blender.blend", ABS_PREFIX "/foo/bar", "//bar/blender.blend"); +} - /* Check for potential buffer overflows. */ - { - char abs_path_in[FILE_MAX]; - abs_path_in[0] = '/'; - for (int i = 1; i < FILE_MAX - 1; i++) { - abs_path_in[i] = 'A'; - } - abs_path_in[FILE_MAX - 1] = '\0'; - char abs_path_out[FILE_MAX]; - abs_path_out[0] = '/'; - abs_path_out[1] = '/'; - for (int i = 2; i < FILE_MAX - 1; i++) { - abs_path_out[i] = 'A'; - } - abs_path_out[FILE_MAX - 1] = '\0'; - PATH_REL(abs_path_in, "/", abs_path_out); - - const char *ref_path_in = "/foo/bar/"; - const size_t ref_path_in_len = strlen(ref_path_in); - strcpy(abs_path_in, ref_path_in); - for (int i = ref_path_in_len; i < FILE_MAX - 1; i++) { - abs_path_in[i] = 'A'; - } - abs_path_in[FILE_MAX - 1] = '\0'; - abs_path_out[0] = '/'; - abs_path_out[1] = '/'; - for (int i = 2; i < FILE_MAX - (int(ref_path_in_len) - 1); i++) { - abs_path_out[i] = 'A'; - } - abs_path_out[FILE_MAX - (ref_path_in_len - 1)] = '\0'; - PATH_REL(abs_path_in, ref_path_in, abs_path_out); +TEST(path_util, PathRelPath_BufferOverflowRoot) +{ + char abs_path_in[FILE_MAX]; + const char *abs_prefix = ABS_PREFIX "/"; + for (int i = STRNCPY_RLEN(abs_path_in, abs_prefix); i < FILE_MAX - 1; i++) { + abs_path_in[i] = 'A'; } + abs_path_in[FILE_MAX - 1] = '\0'; + char abs_path_out[FILE_MAX]; + for (int i = STRNCPY_RLEN(abs_path_out, "//"); i < FILE_MAX - 1; i++) { + abs_path_out[i] = 'A'; + } + abs_path_out[FILE_MAX - std::max((strlen(abs_prefix) - 1), size_t(1))] = '\0'; + PATH_REL(abs_path_in, abs_prefix, abs_path_out); } -# undef PATH_REL +TEST(path_util, PathRelPath_BufferOverflowSubdir) +{ + char abs_path_in[FILE_MAX]; + const char *ref_path_in = ABS_PREFIX "/foo/bar/"; + const size_t ref_path_in_len = strlen(ref_path_in); + for (int i = STRNCPY_RLEN(abs_path_in, ref_path_in); i < FILE_MAX - 1; i++) { + abs_path_in[i] = 'A'; + } + abs_path_in[FILE_MAX - 1] = '\0'; + char abs_path_out[FILE_MAX]; + for (int i = STRNCPY_RLEN(abs_path_out, "//"); i < FILE_MAX - (int(ref_path_in_len) - 1); i++) { + abs_path_out[i] = 'A'; + } + abs_path_out[FILE_MAX - std::max((ref_path_in_len - 1), size_t(1))] = '\0'; + PATH_REL(abs_path_in, ref_path_in, abs_path_out); +} -#endif /* !_WIN32 */ +#undef PATH_REL +#undef ABS_PREFIX /** \} */ -- cgit v1.2.3 From 638bf05a23e1ef7dddd3b5d42d9521d8849a4375 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Wed, 2 Nov 2022 16:56:37 +1100 Subject: Fix BLI_path_parent_dir returning success with a single period as input While relatively harmless, BLI_path_parent_dir wasn't returning failure when passed in "./" which wouldn't succeed. Instead of adding a ".." and normalizing, normalize the path and remove the last directly. This is simpler than inspecting the resulting path to see if normalize removed it or not. Add additional tests which failed previously. --- source/blender/blenlib/intern/path_util.c | 18 ++++++++++------ source/blender/blenlib/tests/BLI_path_util_test.cc | 25 +++++++++++++++++++--- 2 files changed, 34 insertions(+), 9 deletions(-) (limited to 'source/blender/blenlib') diff --git a/source/blender/blenlib/intern/path_util.c b/source/blender/blenlib/intern/path_util.c index 7c08eeedaa2..3a87b39a446 100644 --- a/source/blender/blenlib/intern/path_util.c +++ b/source/blender/blenlib/intern/path_util.c @@ -626,10 +626,9 @@ bool BLI_path_suffix(char *string, size_t maxlen, const char *suffix, const char bool BLI_path_parent_dir(char *path) { - const char parent_dir[] = {'.', '.', SEP, '\0'}; /* "../" or "..\\" */ - char tmp[FILE_MAX + 4]; + char tmp[FILE_MAX]; - BLI_path_join(tmp, sizeof(tmp), path, parent_dir); + STRNCPY(tmp, path); /* Does all the work of normalizing the path for us. * * NOTE(@campbellbarton): While it's possible strip text after the second last slash, @@ -645,12 +644,19 @@ bool BLI_path_parent_dir(char *path) * which would cause checking for a tailing `/../` fail. * Extracting the span of the final directory avoids both these issues. */ int tail_ofs = 0, tail_len = 0; - if (BLI_path_name_at_index(tmp, -1, &tail_ofs, &tail_len) && (tail_len == 2) && - (memcmp(&tmp[tail_ofs], "..", 2) == 0)) { + if (!BLI_path_name_at_index(tmp, -1, &tail_ofs, &tail_len)) { return false; } + if (tail_len == 1) { + /* Last path is ".", as normalize should remove this, it's safe to assume failure. + * This happens when the input a single period (possibly with slashes before or after). */ + if (tmp[tail_ofs] == '.') { + return false; + } + } - strcpy(path, tmp); /* We assume the parent directory is always shorter. */ + memcpy(path, tmp, tail_ofs); + path[tail_ofs] = '\0'; return true; } diff --git a/source/blender/blenlib/tests/BLI_path_util_test.cc b/source/blender/blenlib/tests/BLI_path_util_test.cc index 379ff432988..293d353efcc 100644 --- a/source/blender/blenlib/tests/BLI_path_util_test.cc +++ b/source/blender/blenlib/tests/BLI_path_util_test.cc @@ -102,8 +102,6 @@ TEST(path_util, Clean_Parent) /** \name Tests for: #BLI_path_parent_dir * \{ */ -TEST(path_util, ParentDir) -{ #define PARENT_DIR(input, output) \ { \ char path[FILE_MAX] = input; \ @@ -118,12 +116,25 @@ TEST(path_util, ParentDir) } \ ((void)0) +TEST(path_util, ParentDir_Simple) +{ PARENT_DIR("/a/b/", "/a/"); PARENT_DIR("/a/b", "/a/"); PARENT_DIR("/a", "/"); +} + +TEST(path_util, ParentDir_NOP) +{ PARENT_DIR("/", "/"); PARENT_DIR("", ""); + PARENT_DIR(".", "."); + PARENT_DIR("./", "./"); + PARENT_DIR(".//", ".//"); + PARENT_DIR("./.", "./."); +} +TEST(path_util, ParentDir_TrailingPeriod) +{ /* Ensure trailing dots aren't confused with parent path. */ PARENT_DIR("/.../.../.../", "/.../.../"); PARENT_DIR("/.../.../...", "/.../.../"); @@ -133,10 +144,18 @@ TEST(path_util, ParentDir) PARENT_DIR("/a./b./c./", "/a./b./"); PARENT_DIR("/a./b./c.", "/a./b./"); +} -#undef PARENT_DIR +TEST(path_util, ParentDir_Complex) +{ + PARENT_DIR("./a/", "./"); + PARENT_DIR("./a", "./"); + PARENT_DIR("../a/", "../"); + PARENT_DIR("../a", "../"); } +#undef PARENT_DIR + /** \} */ /* -------------------------------------------------------------------- */ -- cgit v1.2.3