From 0922b92acec568539b072c9dd6aa1d42596823c2 Mon Sep 17 00:00:00 2001 From: Marc Gilleron Date: Mon, 7 Sep 2020 23:26:04 +0100 Subject: [PATCH] Fix some GCC warnings and Clang error --- streams/voxel_block_serializer.cpp | 22 +++++++++++++++------- util/macros.h | 4 ++++ voxel_buffer.cpp | 9 ++++----- voxel_buffer.h | 8 +++++--- 4 files changed, 28 insertions(+), 15 deletions(-) diff --git a/streams/voxel_block_serializer.cpp b/streams/voxel_block_serializer.cpp index 6d01f254..e96ad215 100644 --- a/streams/voxel_block_serializer.cpp +++ b/streams/voxel_block_serializer.cpp @@ -1,6 +1,7 @@ #include "voxel_block_serializer.h" #include "../math/vector3i.h" #include "../thirdparty/lz4/lz4.h" +#include "../util/macros.h" #include "../voxel_buffer.h" #include "../voxel_memory_pool.h" @@ -24,9 +25,14 @@ size_t get_metadata_size_in_bytes(const VoxelBuffer &buffer) { const Map::Element *elem = buffer.get_voxel_metadata().front(); while (elem != nullptr) { const Vector3i pos = elem->key(); - ERR_FAIL_COND_V_MSG(pos.x < 0 || pos.x >= VoxelBuffer::MAX_SIZE, 0, "Invalid voxel metadata X position"); - ERR_FAIL_COND_V_MSG(pos.y < 0 || pos.y >= VoxelBuffer::MAX_SIZE, 0, "Invalid voxel metadata Y position"); - ERR_FAIL_COND_V_MSG(pos.z < 0 || pos.z >= VoxelBuffer::MAX_SIZE, 0, "Invalid voxel metadata Z position"); + + ERR_FAIL_COND_V_MSG(pos.x < 0 || static_cast(pos.x) >= VoxelBuffer::MAX_SIZE, 0, + "Invalid voxel metadata X position"); + ERR_FAIL_COND_V_MSG(pos.y < 0 || static_cast(pos.y) >= VoxelBuffer::MAX_SIZE, 0, + "Invalid voxel metadata Y position"); + ERR_FAIL_COND_V_MSG(pos.z < 0 || static_cast(pos.z) >= VoxelBuffer::MAX_SIZE, 0, + "Invalid voxel metadata Z position"); + size += 3 * sizeof(uint16_t); // Positions are stored as 3 unsigned shorts int len; @@ -74,7 +80,9 @@ void serialize_metadata(uint8_t *p_dst, const VoxelBuffer &buffer, const size_t encode_variant(buffer.get_block_metadata(), dst, written_length, false); dst += written_length; - CRASH_COND_MSG((dst - p_dst) > metadata_size, "Wrote block metadata out of expected bounds"); + // I chose to cast this way to fix a GCC warning. + // If dst - p_dst is negative (which is wrong), it will wrap and cause a justified assertion failure + CRASH_COND_MSG(static_cast(dst - p_dst) > metadata_size, "Wrote block metadata out of expected bounds"); } const Map::Element *elem = buffer.get_voxel_metadata().front(); @@ -91,14 +99,14 @@ void serialize_metadata(uint8_t *p_dst, const VoxelBuffer &buffer, const size_t CRASH_COND_MSG(err != OK, "Error when trying to encode voxel metadata."); dst += written_length; - CRASH_COND_MSG((dst - p_dst) > metadata_size, "Wrote voxel metadata out of expected bounds"); + CRASH_COND_MSG(static_cast(dst - p_dst) > metadata_size, "Wrote voxel metadata out of expected bounds"); elem = elem->next(); } - CRASH_COND_MSG((dst - p_dst) != metadata_size, + CRASH_COND_MSG(static_cast(dst - p_dst) != metadata_size, String("Written metadata doesn't match expected count (expected {0}, got {1})") - .format(varray(metadata_size, (int)(dst - p_dst)))); + .format(varray(SIZE_T_TO_VARIANT(metadata_size), (int)(dst - p_dst)))); } bool deserialize_metadata(uint8_t *p_src, VoxelBuffer &buffer, const size_t metadata_size) { diff --git a/util/macros.h b/util/macros.h index 388e98f1..dceb9cf9 100644 --- a/util/macros.h +++ b/util/macros.h @@ -10,4 +10,8 @@ print_line(msg); \ } +// TODO Waiting for a fix, Variant() can't be constructed from `size_t` on JavaScript and OSX builds. +// See https://github.com/godotengine/godot/issues/36690 +#define SIZE_T_TO_VARIANT(s) static_cast(s) + #endif // VOXEL_MACROS_H diff --git a/voxel_buffer.cpp b/voxel_buffer.cpp index 1246d2b6..e2cb1e47 100644 --- a/voxel_buffer.cpp +++ b/voxel_buffer.cpp @@ -119,8 +119,7 @@ VoxelBuffer::~VoxelBuffer() { clear(); } -void VoxelBuffer::create(int sx, int sy, int sz) { - ERR_FAIL_COND(sx <= 0 || sy <= 0 || sz <= 0); +void VoxelBuffer::create(unsigned int sx, unsigned int sy, unsigned int sz) { ERR_FAIL_COND(sx > MAX_SIZE || sy > MAX_SIZE || sz > MAX_SIZE); clear_voxel_metadata(); @@ -801,12 +800,12 @@ void VoxelBuffer::clear_voxel_metadata_in_area(Rect3i box) { } } -void VoxelBuffer::copy_voxel_metadata_in_area(Ref src_buffer, Rect3i src_box, Vector3i dst_pos) { +void VoxelBuffer::copy_voxel_metadata_in_area(Ref src_buffer, Rect3i src_box, Vector3i dst_origin) { ERR_FAIL_COND(src_buffer.is_null()); ERR_FAIL_COND(src_buffer->is_box_valid(src_box)); - const Rect3i clipped_src_box = src_box.clipped(Rect3i(src_box.pos - dst_pos, _size)); - const Vector3i clipped_dst_offset = dst_pos + clipped_src_box.pos - src_box.pos; + const Rect3i clipped_src_box = src_box.clipped(Rect3i(src_box.pos - dst_origin, _size)); + const Vector3i clipped_dst_offset = dst_origin + clipped_src_box.pos - src_box.pos; const Map::Element *elem = src_buffer->_voxel_metadata.front(); diff --git a/voxel_buffer.h b/voxel_buffer.h index c7e6962b..eecf82bc 100644 --- a/voxel_buffer.h +++ b/voxel_buffer.h @@ -52,12 +52,14 @@ public: }; static const Depth DEFAULT_CHANNEL_DEPTH = DEPTH_8_BIT; - static const uint32_t MAX_SIZE = 65535; // Limit was made explicit for serialization reasons + + // Limit was made explicit for serialization reasons, and also because there must be a reasonable one + static const uint32_t MAX_SIZE = 65535; VoxelBuffer(); ~VoxelBuffer(); - void create(int sx, int sy, int sz); + void create(unsigned int sx, unsigned int sy, unsigned int sz); void create(Vector3i size); void clear(); void clear_channel(unsigned int channel_index, uint64_t clear_value = 0); @@ -142,7 +144,7 @@ public: void for_each_voxel_metadata_in_area(Ref callback, Rect3i box) const; void clear_voxel_metadata(); void clear_voxel_metadata_in_area(Rect3i box); - void copy_voxel_metadata_in_area(Ref src_buffer, Rect3i src_box, Vector3i dst_pos); + void copy_voxel_metadata_in_area(Ref src_buffer, Rect3i src_box, Vector3i dst_origin); const Map &get_voxel_metadata() const { return _voxel_metadata; }