From 793446cbdcc28709b0bfd858c7077f5d31560162 Mon Sep 17 00:00:00 2001 From: Sergey Sharybin Date: Tue, 23 May 2023 09:21:45 +0200 Subject: [PATCH] BLI: Replace some macros with inlined functions for C++ Covers the macro ARRAY_SIZE() and STRNCPY. The problem this change is aimed to solve it to provide cross-platform compiler-independent safe way pf ensuring that the functions are used correctly. The type safety was only ensured for GCC and only for C. The C++ language and Clang compiler would not have detected issues of passing bare pointer to neither of those macros. Now the STRNCPY() will only accept a bounded array as the destination argument, on any compiler. The ARRAY_SIZE as well, but there are a bit more complications to it in terms of transparency of the change. In one place the ARRAY_SIZE was used on float3 type. This worked in the old code because the type implements subscript operator, and the type consists of 3 floats. One would argue this is somewhat hidden/implicit behavior, which better be avoided. So an in-lined value of 3 is used now there. Another place is the ARRAY_SIZE used to define a bounded array of the size which matches bounded array which is a member of a struct. While the ARRAY_SIZE provides proper size in this case, the compiler does not believe that the value is known at compile time and errors out with a message that construction of variable-size arrays is not supported. Solved by converting the field to std::array<> and adding dedicated utility to get size of std::array at compile time. There might be a better way of achieving the same result, or maybe the approach is fine and just need to find a better place for such utility. Surely, more macro from the BLI_string.h can be covered with the C++ inlined functions, but need to start somewhere. There are also quite some changes to ensure the C linkage is not enforced by code which includes the headers. Pull Request: https://projects.blender.org/blender/blender/pulls/108041 --- intern/ghost/intern/GHOST_EventDragnDrop.hh | 2 - intern/memutil/MEM_CacheLimiterC-Api.h | 4 +- .../blender/blenkernel/intern/customdata.cc | 3 +- source/blender/blenkernel/intern/object.cc | 2 +- source/blender/blenlib/BLI_math_color.h | 8 +-- source/blender/blenlib/BLI_math_geom.h | 8 +-- source/blender/blenlib/BLI_string.h | 18 +++++- source/blender/blenlib/BLI_utildefines.h | 58 +++++++++++++++++-- source/blender/blenlib/CMakeLists.txt | 1 + .../blenlib/intern/math_color_inline.c | 8 +++ .../blenlib/tests/BLI_delaunay_2d_test.cc | 2 - .../blender/blenlib/tests/BLI_string_test.cc | 15 +++++ .../blenlib/tests/BLI_utildefines_test.cc | 43 ++++++++++++++ source/blender/draw/intern/draw_view.cc | 4 +- .../blender/editors/asset/ED_asset_indexer.h | 4 +- .../editors/sculpt_paint/paint_image_proj.cc | 10 +++- .../windowmanager/intern/wm_event_system.cc | 2 +- 17 files changed, 161 insertions(+), 31 deletions(-) create mode 100644 source/blender/blenlib/tests/BLI_utildefines_test.cc diff --git a/intern/ghost/intern/GHOST_EventDragnDrop.hh b/intern/ghost/intern/GHOST_EventDragnDrop.hh index cebbaa6fe4e..8874a6a011a 100644 --- a/intern/ghost/intern/GHOST_EventDragnDrop.hh +++ b/intern/ghost/intern/GHOST_EventDragnDrop.hh @@ -8,10 +8,8 @@ #pragma once #include "GHOST_Event.hh" -extern "C" { #include "IMB_imbuf.h" #include "IMB_imbuf_types.h" -}; /** * Drag & drop event diff --git a/intern/memutil/MEM_CacheLimiterC-Api.h b/intern/memutil/MEM_CacheLimiterC-Api.h index b1c0f3a1eb8..2c0a6c9800e 100644 --- a/intern/memutil/MEM_CacheLimiterC-Api.h +++ b/intern/memutil/MEM_CacheLimiterC-Api.h @@ -7,12 +7,12 @@ #ifndef __MEM_CACHELIMITERC_API_H__ #define __MEM_CACHELIMITERC_API_H__ +#include "BLI_utildefines.h" + #ifdef __cplusplus extern "C" { #endif -#include "BLI_utildefines.h" - struct MEM_CacheLimiter_s; struct MEM_CacheLimiterHandle_s; diff --git a/source/blender/blenkernel/intern/customdata.cc b/source/blender/blenkernel/intern/customdata.cc index ac66a098f29..bedcf4c01fa 100644 --- a/source/blender/blenkernel/intern/customdata.cc +++ b/source/blender/blenkernel/intern/customdata.cc @@ -71,7 +71,8 @@ using blender::Vector; #define CUSTOMDATA_GROW 5 /* ensure typemap size is ok */ -BLI_STATIC_ASSERT(ARRAY_SIZE(((CustomData *)nullptr)->typemap) == CD_NUMTYPES, "size mismatch"); +BLI_STATIC_ASSERT(BOUNDED_ARRAY_TYPE_SIZE() == CD_NUMTYPES, + "size mismatch"); static CLG_LogRef LOG = {"bke.customdata"}; diff --git a/source/blender/blenkernel/intern/object.cc b/source/blender/blenkernel/intern/object.cc index 0f82665a17f..0191939c8e6 100644 --- a/source/blender/blenkernel/intern/object.cc +++ b/source/blender/blenkernel/intern/object.cc @@ -1227,7 +1227,7 @@ static IDProperty *object_asset_dimensions_property(Object *ob) } IDPropertyTemplate idprop{}; - idprop.array.len = ARRAY_SIZE(dimensions); + idprop.array.len = 3; idprop.array.type = IDP_FLOAT; IDProperty *property = IDP_New(IDP_ARRAY, &idprop, "dimensions"); diff --git a/source/blender/blenlib/BLI_math_color.h b/source/blender/blenlib/BLI_math_color.h index 3aa2e35476d..01c119b10fd 100644 --- a/source/blender/blenlib/BLI_math_color.h +++ b/source/blender/blenlib/BLI_math_color.h @@ -201,12 +201,12 @@ void lift_gamma_gain_to_asc_cdl(const float *lift, float *slope, float *power); -#if BLI_MATH_DO_INLINE -# include "intern/math_color_inline.c" -#endif - /** \} */ #ifdef __cplusplus } #endif + +#if BLI_MATH_DO_INLINE +# include "intern/math_color_inline.c" +#endif diff --git a/source/blender/blenlib/BLI_math_geom.h b/source/blender/blenlib/BLI_math_geom.h index 126bd158140..b39e5159a94 100644 --- a/source/blender/blenlib/BLI_math_geom.h +++ b/source/blender/blenlib/BLI_math_geom.h @@ -1402,6 +1402,10 @@ float geodesic_distance_propagate_across_triangle( /** \} */ +#ifdef __cplusplus +} +#endif + /* -------------------------------------------------------------------- */ /** \name Inline Definitions * \{ */ @@ -1415,7 +1419,3 @@ float geodesic_distance_propagate_across_triangle( #endif /** \} */ - -#ifdef __cplusplus -} -#endif diff --git a/source/blender/blenlib/BLI_string.h b/source/blender/blenlib/BLI_string.h index 785904ff87e..0981cfb4318 100644 --- a/source/blender/blenlib/BLI_string.h +++ b/source/blender/blenlib/BLI_string.h @@ -551,7 +551,10 @@ int BLI_string_find_split_words(const char *str, * \note `ARRAY_SIZE` allows pointers on some platforms. * \{ */ -#define STRNCPY(dst, src) BLI_strncpy(dst, src, ARRAY_SIZE(dst)) +#ifndef __cplusplus +# define STRNCPY(dst, src) BLI_strncpy(dst, src, ARRAY_SIZE(dst)) +#endif + #define STRNCPY_RLEN(dst, src) BLI_strncpy_rlen(dst, src, ARRAY_SIZE(dst)) #define SNPRINTF(dst, format, ...) BLI_snprintf(dst, ARRAY_SIZE(dst), format, __VA_ARGS__) #define SNPRINTF_RLEN(dst, format, ...) \ @@ -635,4 +638,17 @@ void BLI_string_debug_size_after_nil(char *str, size_t str_maxncpy); /** \} */ #ifdef __cplusplus } + +/** + * Copy source string str into the destination dst of a size known at a compile time. + * Ensures that the destination is not overflown, and that the destination is always + * null-terminated. + * + * Returns the dst. + */ +template inline char *STRNCPY(char (&dst)[N], const char *src) +{ + return BLI_strncpy(dst, src, N); +} + #endif diff --git a/source/blender/blenlib/BLI_utildefines.h b/source/blender/blenlib/BLI_utildefines.h index bfa5102f6b1..d14752a2c90 100644 --- a/source/blender/blenlib/BLI_utildefines.h +++ b/source/blender/blenlib/BLI_utildefines.h @@ -21,6 +21,9 @@ #include "BLI_compiler_typecheck.h" #ifdef __cplusplus +# include +# include + extern "C" { #endif @@ -493,12 +496,15 @@ extern "C" { ((void)0) /* assuming a static array */ -#if defined(__GNUC__) && !defined(__cplusplus) && !defined(__clang__) && !defined(__INTEL_COMPILER) -# define ARRAY_SIZE(arr) \ - ((sizeof(struct { int isnt_array : ((const void *)&(arr) == &(arr)[0]); }) * 0) + \ - (sizeof(arr) / sizeof(*(arr)))) -#else -# define ARRAY_SIZE(arr) (sizeof(arr) / sizeof(*(arr))) +#ifndef __cplusplus +# if defined(__GNUC__) && !defined(__cplusplus) && !defined(__clang__) && \ + !defined(__INTEL_COMPILER) +# define ARRAY_SIZE(arr) \ + ((sizeof(struct { int isnt_array : ((const void *)&(arr) == &(arr)[0]); }) * 0) + \ + (sizeof(arr) / sizeof(*(arr)))) +# else +# define ARRAY_SIZE(arr) (sizeof(arr) / sizeof(*(arr))) +# endif #endif /* ARRAY_SET_ITEMS#(v, ...): set indices of array 'v' */ @@ -867,6 +873,46 @@ extern bool BLI_memory_is_zero(const void *arr, size_t arr_size); #ifdef __cplusplus } + +namespace blender::blenlib_internal { + +/* A replacement for std::is_bounded_array_v until we go C++20. */ +template struct IsBoundedArray : std::false_type { +}; +template struct IsBoundedArray : std::true_type { +}; + +} // namespace blender::blenlib_internal + +/** + * Size of a bounded array provided as an arg. + * + * The arg must be a bounded array, such as int[7] or MyType[11]. + * Returns the number of elements in the array, known at the compile time. + */ +template constexpr size_t ARRAY_SIZE(const T (&arg)[N]) noexcept +{ + (void)arg; + return N; +} + +/** + * Number of elements in a type which defines a bounded array. + * + * For example, + * struct MyType { + * int array[12]; + * }; + * + * `BOUNDED_ARRAY_TYPE_SIZE` returns 12. + */ +template +constexpr std::enable_if_t::value, size_t> +BOUNDED_ARRAY_TYPE_SIZE() noexcept +{ + return sizeof(std::declval()) / sizeof(std::declval()[0]); +} + #endif #endif /* __BLI_UTILDEFINES_H__ */ diff --git a/source/blender/blenlib/CMakeLists.txt b/source/blender/blenlib/CMakeLists.txt index 1ddadeec864..a25eff0f028 100644 --- a/source/blender/blenlib/CMakeLists.txt +++ b/source/blender/blenlib/CMakeLists.txt @@ -546,6 +546,7 @@ if(WITH_GTESTS) tests/BLI_task_graph_test.cc tests/BLI_task_test.cc tests/BLI_tempfile_test.cc + tests/BLI_utildefines_test.cc tests/BLI_uuid_test.cc tests/BLI_vector_set_test.cc tests/BLI_vector_test.cc diff --git a/source/blender/blenlib/intern/math_color_inline.c b/source/blender/blenlib/intern/math_color_inline.c index fd0c2e3c5f7..0baeb2124e6 100644 --- a/source/blender/blenlib/intern/math_color_inline.c +++ b/source/blender/blenlib/intern/math_color_inline.c @@ -11,6 +11,10 @@ #include +#ifdef __cplusplus +extern "C" { +#endif + #ifndef __MATH_COLOR_INLINE_C__ # define __MATH_COLOR_INLINE_C__ @@ -379,3 +383,7 @@ MINLINE void premul_float_to_straight_uchar(unsigned char *result, const float c } #endif /* __MATH_COLOR_INLINE_C__ */ + +#ifdef __cplusplus +} +#endif diff --git a/source/blender/blenlib/tests/BLI_delaunay_2d_test.cc b/source/blender/blenlib/tests/BLI_delaunay_2d_test.cc index 0717e88a19e..de66fa7072e 100644 --- a/source/blender/blenlib/tests/BLI_delaunay_2d_test.cc +++ b/source/blender/blenlib/tests/BLI_delaunay_2d_test.cc @@ -4,11 +4,9 @@ #include "MEM_guardedalloc.h" -extern "C" { #include "BLI_math.h" #include "BLI_rand.h" #include "PIL_time.h" -} #include #include diff --git a/source/blender/blenlib/tests/BLI_string_test.cc b/source/blender/blenlib/tests/BLI_string_test.cc index f3f03a9f9dc..e39e7e5a406 100644 --- a/source/blender/blenlib/tests/BLI_string_test.cc +++ b/source/blender/blenlib/tests/BLI_string_test.cc @@ -1332,3 +1332,18 @@ TEST_F(StringEscape, Control) } /** \} */ + +TEST(BLI_string, bounded_strcpy) +{ + { + char str[8]; + STRNCPY(str, "Hello"); + EXPECT_STREQ(str, "Hello"); + } + + { + char str[8]; + STRNCPY(str, "Hello, World!"); + EXPECT_STREQ(str, "Hello, "); + } +} diff --git a/source/blender/blenlib/tests/BLI_utildefines_test.cc b/source/blender/blenlib/tests/BLI_utildefines_test.cc new file mode 100644 index 00000000000..f5d2749e459 --- /dev/null +++ b/source/blender/blenlib/tests/BLI_utildefines_test.cc @@ -0,0 +1,43 @@ +/* SPDX-License-Identifier: Apache-2.0 */ + +#include "BLI_utildefines.h" + +#include "testing/testing.h" + +namespace blender::tests { + +TEST(BLI_utildefines, ARRAY_SIZE) +{ + { + int bounded_char[5]; + static_assert(ARRAY_SIZE(bounded_char) == 5); + } + + { + int *bounded_char[5]; + static_assert(ARRAY_SIZE(bounded_char) == 5); + } +} + +TEST(BLI_utildefines, BOUNDED_ARRAY_TYPE_SIZE) +{ + { + int bounded_char[5]; + static_assert(BOUNDED_ARRAY_TYPE_SIZE() == 5); + } + + { + int *bounded_char[5]; + static_assert(BOUNDED_ARRAY_TYPE_SIZE() == 5); + } + + { + struct MyType { + int array[12]; + }; + + static_assert(BOUNDED_ARRAY_TYPE_SIZE() == 12); + } +} + +} // namespace blender::tests diff --git a/source/blender/draw/intern/draw_view.cc b/source/blender/draw/intern/draw_view.cc index c6b7ac11017..92d8c98e335 100644 --- a/source/blender/draw/intern/draw_view.cc +++ b/source/blender/draw/intern/draw_view.cc @@ -51,7 +51,7 @@ void View::frustum_boundbox_calc(int view_id) #endif MutableSpan corners = {culling_[view_id].frustum_corners.corners, - ARRAY_SIZE(culling_[view_id].frustum_corners.corners)}; + int64_t(ARRAY_SIZE(culling_[view_id].frustum_corners.corners))}; float left, right, bottom, top, near, far; bool is_persp = data_[view_id].winmat[3][3] == 0.0f; @@ -107,7 +107,7 @@ void View::frustum_culling_sphere_calc(int view_id) { BoundSphere &bsphere = *reinterpret_cast(&culling_[view_id].bound_sphere); Span corners = {culling_[view_id].frustum_corners.corners, - ARRAY_SIZE(culling_[view_id].frustum_corners.corners)}; + int64_t(ARRAY_SIZE(culling_[view_id].frustum_corners.corners))}; /* Extract Bounding Sphere */ if (data_[view_id].winmat[3][3] != 0.0f) { diff --git a/source/blender/editors/asset/ED_asset_indexer.h b/source/blender/editors/asset/ED_asset_indexer.h index 924a761864b..197c04439a1 100644 --- a/source/blender/editors/asset/ED_asset_indexer.h +++ b/source/blender/editors/asset/ED_asset_indexer.h @@ -6,12 +6,12 @@ #pragma once +#include "ED_file_indexer.h" + #ifdef __cplusplus extern "C" { #endif -#include "ED_file_indexer.h" - /** * File Indexer Service for indexing asset files. * diff --git a/source/blender/editors/sculpt_paint/paint_image_proj.cc b/source/blender/editors/sculpt_paint/paint_image_proj.cc index b9b6f34dcb1..476fbe78e67 100644 --- a/source/blender/editors/sculpt_paint/paint_image_proj.cc +++ b/source/blender/editors/sculpt_paint/paint_image_proj.cc @@ -11,6 +11,7 @@ #include #include #include +#include #include "MEM_guardedalloc.h" @@ -190,6 +191,8 @@ BLI_INLINE uchar f_to_char(const float val) /* to avoid locking in tile initialization */ #define TILE_PENDING POINTER_FROM_INT(-1) +struct ProjPaintState; + /** * This is mainly a convenience struct used so we can keep an array of images we use - * their #ImBuf's, etc, in 1 array, When using threads this array is copied for each thread @@ -217,7 +220,8 @@ struct ProjStrokeHandle { /* Support for painting from multiple views at once, * currently used to implement symmetry painting, * we can assume at least the first is set while painting. */ - struct ProjPaintState *ps_views[8]; + ProjPaintState *ps_views[8]; + int ps_views_tot; int symmetry_flags; @@ -5985,9 +5989,9 @@ void *paint_proj_new_stroke(bContext *C, Object *ob, const float mouse[2], int m ProjStrokeHandle *ps_handle; Scene *scene = CTX_data_scene(C); ToolSettings *settings = scene->toolsettings; - char symmetry_flag_views[ARRAY_SIZE(ps_handle->ps_views)] = {0}; + char symmetry_flag_views[BOUNDED_ARRAY_TYPE_SIZEps_views)>()] = {0}; - ps_handle = MEM_cnew("ProjStrokeHandle"); + ps_handle = MEM_new("ProjStrokeHandle"); ps_handle->scene = scene; ps_handle->brush = BKE_paint_brush(&settings->imapaint.paint); diff --git a/source/blender/windowmanager/intern/wm_event_system.cc b/source/blender/windowmanager/intern/wm_event_system.cc index 22ff5e34dc6..b7436f2e8fd 100644 --- a/source/blender/windowmanager/intern/wm_event_system.cc +++ b/source/blender/windowmanager/intern/wm_event_system.cc @@ -4526,7 +4526,7 @@ static void wm_event_get_keymap_from_toolsystem_ex(wmWindowManager *wm, { memset(km_result, 0x0, sizeof(*km_result)); - const char *keymap_id_list[ARRAY_SIZE(km_result->keymaps)]; + const char *keymap_id_list[BOUNDED_ARRAY_TYPE_SIZEkeymaps)>()]; int keymap_id_list_len = 0; /* NOTE(@ideasman42): If `win` is nullptr, this function may not behave as expected.