From b4548166c329257c97441203ac1e992b9206f9f9 Mon Sep 17 00:00:00 2001 From: Brett Date: Wed, 8 Feb 2023 13:40:28 -0500 Subject: [PATCH] Finally fully fix logging issue -buffer size is now calculated -overflows have been fixed -temporary buffer is allocated on the heap. --- include/blt/std/filesystem.h | 32 +--------------------- include/blt/std/memory.h | 51 ++++++++++++++++++++++++++++++++++++ src/blt/std/logging.cpp | 17 +++++++++--- src/tests/nbt_tests.h | 12 ++++----- 4 files changed, 72 insertions(+), 40 deletions(-) create mode 100644 include/blt/std/memory.h diff --git a/include/blt/std/filesystem.h b/include/blt/std/filesystem.h index 0d38d9b..c0254e4 100644 --- a/include/blt/std/filesystem.h +++ b/include/blt/std/filesystem.h @@ -9,40 +9,10 @@ #include #include +#include "memory.h" namespace blt::fs { - /** - * Creates an encapsulation of a T array which will be automatically deleted when this object goes out of scope. - * This is a simple buffer meant to be used only inside of a function and not moved around, with a few minor exceptions. - * @tparam T type that is stored in buffer eg char - */ - template - struct scoped_buffer { - T* buffer; - unsigned long size; - - explicit scoped_buffer(unsigned long size): size(size) { - buffer = new T[size]; - } - - scoped_buffer(scoped_buffer& copy) = delete; - - scoped_buffer(scoped_buffer&& move) = delete; - - scoped_buffer operator=(scoped_buffer& copyAssignment) = delete; - - scoped_buffer operator=(scoped_buffer&& moveAssignment) = delete; - - inline T& operator[](unsigned long index) const { - return buffer[index]; - } - - ~scoped_buffer() { - delete[] buffer; - } - }; - /** * A simple interface which provides a way of reading the next block of data from a resource. * The interface provides a single function "read" which will read a specified number of bytes into the buffer. diff --git a/include/blt/std/memory.h b/include/blt/std/memory.h new file mode 100644 index 0000000..6dd53e2 --- /dev/null +++ b/include/blt/std/memory.h @@ -0,0 +1,51 @@ +/* + * Created by Brett on 08/02/23. + * Licensed under GNU General Public License V3.0 + * See LICENSE file for license detail + */ + +#ifndef BLT_TESTS_MEMORY_H +#define BLT_TESTS_MEMORY_H + +namespace blt { +/** + * Creates an encapsulation of a T array which will be automatically deleted when this object goes out of scope. + * This is a simple buffer meant to be used only inside of a function and not moved around, with a few minor exceptions. + * The internal buffer is allocated on the heap. + * The operator * has been overloaded to return the internal buffer. (or just use scoped_buffer.buffer if you wish to be explicit) + * The operator & was not used because I think it is stupid to do so. + * "*" on a reference is fine however since it isn't used to dereference in the case. + * @tparam T type that is stored in buffer eg char + */ + template + struct scoped_buffer { + T* buffer; + unsigned long size; + + explicit scoped_buffer(unsigned long size): size(size) { + buffer = new T[size]; + } + + scoped_buffer(scoped_buffer& copy) = delete; + + scoped_buffer(scoped_buffer&& move) = delete; + + scoped_buffer operator=(scoped_buffer& copyAssignment) = delete; + + scoped_buffer operator=(scoped_buffer&& moveAssignment) = delete; + + inline T& operator[](unsigned long index) const { + return buffer[index]; + } + + inline T* operator*(){ + return buffer; + } + + ~scoped_buffer() { + delete[] buffer; + } + }; +} + +#endif //BLT_TESTS_MEMORY_H diff --git a/src/blt/std/logging.cpp b/src/blt/std/logging.cpp index c04a17f..8f7a7d4 100644 --- a/src/blt/std/logging.cpp +++ b/src/blt/std/logging.cpp @@ -14,6 +14,7 @@ #include #include #include +#include // https://en.cppreference.com/w/cpp/utility/variadic // https://medium.com/swlh/variadic-functions-3419c287a0d2 @@ -61,9 +62,19 @@ namespace blt::logging { }; void applyFormatting(const std::string& format, std::string& output, va_list& args){ - std::vector buf(1+std::vsnprintf(nullptr, 0, format.c_str(), args)); - vsprintf(buf.data(), format.c_str(), args); - output = std::string(buf.data()); + // args must be copied because they will be consumed by the first vsnprintf + va_list args_copy; + va_copy(args_copy, args); + + auto buffer_size = std::vsnprintf(nullptr, 0, format.c_str(), args_copy) + 1; + // some compilers don't even allow you to do stack dynamic arrays. So always allocate on the heap. + // originally if the buffer was small enough the buffer was allocated on the stack because it made no sense to make a heap object + // that will be deleted a couple lines later. + scoped_buffer buffer{static_cast(buffer_size)}; + vsnprintf(*buffer, buffer_size, format.c_str(), args); + output = std::string(*buffer); + + va_end(args_copy); } const char* levelColors[6] = { diff --git a/src/tests/nbt_tests.h b/src/tests/nbt_tests.h index e10a9d9..123924c 100644 --- a/src/tests/nbt_tests.h +++ b/src/tests/nbt_tests.h @@ -13,8 +13,8 @@ #include #include -inline bool readLargeBlockUsingNBTBufferedReader(const std::string& file, const blt::fs::scoped_buffer& bufferToCompare, size_t bufferSize) { - blt::fs::scoped_buffer read_buffer{bufferToCompare.size}; +inline bool readLargeBlockUsingNBTBufferedReader(const std::string& file, const blt::scoped_buffer& bufferToCompare, size_t bufferSize) { + blt::scoped_buffer read_buffer{bufferToCompare.size}; std::fstream largeBlockInputLarge(file, std::ios::in | std::ios::binary); blt::fs::fstream_block_reader byteLargeBlockInputLarge(largeBlockInputLarge, bufferSize); byteLargeBlockInputLarge.read(read_buffer.buffer, bufferToCompare.size); @@ -26,7 +26,7 @@ inline bool readLargeBlockUsingNBTBufferedReader(const std::string& file, const return true; } -inline bool readIndividualUsingNBTBufferedReader(const std::string& file, const blt::fs::scoped_buffer& bufferToCompare, size_t bufferSize) { +inline bool readIndividualUsingNBTBufferedReader(const std::string& file, const blt::scoped_buffer& bufferToCompare, size_t bufferSize) { std::fstream largeBlockInput(file, std::ios::in | std::ios::binary); blt::fs::fstream_block_reader byteLargeBlockInput(largeBlockInput, bufferSize); for (int i = 0; i < bufferToCompare.size; i++) { @@ -43,7 +43,7 @@ inline bool readIndividualUsingNBTBufferedReader(const std::string& file, const inline void nbt_read_tests(){ constexpr auto bufferSize = 1024 * 1024; - blt::fs::scoped_buffer buffer{bufferSize}; + blt::scoped_buffer buffer{bufferSize}; char* read_buffer = new char[bufferSize]; char* read_block_buffer = new char[bufferSize]; @@ -125,8 +125,8 @@ inline void nbt_read_tests(){ inline void nbt_write_tests(){ constexpr auto bufferSize = 1024 * 1024; - blt::fs::scoped_buffer buffer{bufferSize}; - blt::fs::scoped_buffer read_buffer{bufferSize}; + blt::scoped_buffer buffer{bufferSize}; + blt::scoped_buffer read_buffer{bufferSize}; for (int i = 0; i < bufferSize; i++) buffer.buffer[i] = i+1;