Skip to content

New string concept #39

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Jul 2, 2025
Merged
3 changes: 3 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ SET ( cppcore_src
SET ( cppcore_common_src
include/cppcore/Common/Hash.h
include/cppcore/Common/TStringBase.h
include/cppcore/Common/TStringView.h
include/cppcore/Common/Variant.h
include/cppcore/Common/Sort.h
include/cppcore/Common/TBitField.h
Expand Down Expand Up @@ -137,6 +138,8 @@ IF( CPPCORE_BUILD_UNITTESTS )
test/common/SortTest.cpp
test/common/TBitFieldTest.cpp
test/common/TOptionalTest.cpp
test/common/TStringViewTest.cpp
test/common/TStringBaseTest.cpp
)

SET( cppcore_container_test_src
Expand Down
16 changes: 9 additions & 7 deletions include/cppcore/CPPCoreCommon.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,19 @@ namespace cppcore {
#endif

#ifdef CPPCORE_WINDOWS
# define CPPCORE_TAG_DLL_EXPORT __declspec(dllexport)
# define CPPCORE_TAG_DLL_IMPORT __declspec(dllimport )
# define CPPCORE_TAG_DLL_EXPORT __declspec(dllexport)
# define CPPCORE_TAG_DLL_IMPORT __declspec(dllimport )
# ifdef CPPCORE_BUILD
# define DLL_CPPCORE_EXPORT CPPCORE_TAG_DLL_EXPORT
# define DLL_CPPCORE_EXPORT CPPCORE_TAG_DLL_EXPORT
# else
# define DLL_CPPCORE_EXPORT CPPCORE_TAG_DLL_IMPORT
# define DLL_CPPCORE_EXPORT CPPCORE_TAG_DLL_IMPORT
# endif
// All disabled warnings for windows
# pragma warning( disable : 4251 ) // <class> needs to have dll-interface to be used by clients of class <class>
# define CPPCORE_STACK_ALLOC(size) ::alloca(size)
# define CPPCORE_STACK_ALLOC(size) ::alloca(size)
#else
# define DLL_CPPCORE_EXPORT __attribute__((visibility("default")))
# define CPPCORE_STACK_ALLOC(size) __builtin_alloca(size)
# define DLL_CPPCORE_EXPORT __attribute__((visibility("default")))
# define CPPCORE_STACK_ALLOC(size) __builtin_alloca(size)
#endif

//-------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -107,4 +107,6 @@ public: \
//-------------------------------------------------------------------------------------------------
#define CPPCORE_ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))

using HashId = uint64_t;

} // Namespace cppcore
2 changes: 1 addition & 1 deletion include/cppcore/Common/Sort.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ namespace cppcore {
}

/// @brief Implements a binary search algorithm.
/// @tparam T The type of the value
/// @tparam T The type of the value
/// @param key The key to search for
/// @param array The data to search in
/// @param num The number of elements to search
Expand Down
172 changes: 70 additions & 102 deletions include/cppcore/Common/TStringBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,30 +23,11 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
#pragma once

#include "string.h"
#include <cppcore/Common/Hash.h>
#include <cppcore/CPPCoreCommon.h>
#include <malloc.h>

namespace cppcore {

template <class T>
struct Allocator {
inline T *alloc(size_t size, size_t alignment) {
return (T*) _aligned_malloc(size, alignment);
}

inline void dealloc(T *ptr) {
_aligned_free(ptr);
}

inline static size_t countChars(const T *ptr) {
if (nullptr != ptr) {
return 0;
}

return (::strlen(ptr));
}
};

//-------------------------------------------------------------------------------------------------
/// @class TStringBase
/// @ingroup CPPCore
Expand All @@ -57,135 +38,122 @@ template <class T>
class TStringBase {
public:
/// @brief The default class constructor.
TStringBase() noexcept;
TStringBase() noexcept = default;

/// @brief The class constructor with a pointer showing to the data buffer.
/// @param pPtr [in] The data buffer.
TStringBase(const T *pPtr);
TStringBase(const T *pPtr, size_t size);

/// @brief The class destructor.
~TStringBase();

void set(const T *ptr);
void set(const T *ptr, size_t size);
void clear();
void reset();
size_t size() const;
size_t capacity() const;
const T *c_str() const;

/// @brief Helper method to copy data into the string.
/// @param base [inout] The string data to copy in.
/// @param pPtr [in] The data source.
static void copyFrom(TStringBase<T> &base, const T *pPtr);
static void copyFrom(TStringBase<T> &base, const T *pPtr, size_t size);

bool operator == (const TStringBase<T> &rhs) const;
bool operator != (const TStringBase<T> &rhs) const;

T *m_pStringBuffer;
size_t m_size;
size_t m_capacity;
Allocator<T> mAllocator;
private:
static constexpr size_t InitSize = 256;
T mBuffer[InitSize] = {'\0'};
T *mStringBuffer{nullptr};
size_t mSize{0};
size_t mCapacity{256};
HashId mHashId{0};
};

template <class T>
inline TStringBase<T>::TStringBase() noexcept :
m_pStringBuffer(nullptr),
m_size(0),
m_capacity(0),
mAllocator() {
// empty
inline TStringBase<T>::TStringBase(const T *pPtr, size_t size) {
copyFrom(*this, pPtr, size);
mHashId = THash<HashId>::toHash(pPtr, mSize);
}

template <class T>
inline TStringBase<T>::TStringBase(const T *pPtr) :
m_pStringBuffer(nullptr),
m_size(0),
m_capacity(0),
mAllocator() {
copyFrom(*this, pPtr);
inline TStringBase<T>::~TStringBase() {
clear();
}

template <class T>
inline TStringBase<T>::~TStringBase() {
if (m_pStringBuffer) {
mAllocator.dealloc(m_pStringBuffer);
m_pStringBuffer = nullptr;
inline void TStringBase<T>::set(const T *ptr, size_t size) {
void reset();
if (nullptr != ptr) {
copyFrom(*this, ptr, size);
}
}

template <class T>
inline void TStringBase<T>::set(const T *ptr) {
mAllocator.dealloc(m_pStringBuffer);
if (nullptr != ptr) {
copyFrom(*this, ptr);
}
inline void TStringBase<T>::reset() {
mSize = 0u;
}

template <class T>
inline void TStringBase<T>::copyFrom(TStringBase<T> &base, const T *ptr) {
if (nullptr != ptr) {
base.m_size = Allocator<T>::countChars(ptr);
if (base.m_size) {
base.m_capacity = base.m_size + 1;
base.m_pStringBuffer = base.mAllocator.alloc(base.m_capacity, 16);
#ifdef CPPCORE_WINDOWS
::strncpy_s(base.m_pStringBuffer, base.m_capacity, ptr, base.m_size);
#else
::strncpy(base.m_pStringBuffer, ptr, base.m_size);
#endif
base.m_pStringBuffer[base.m_size] = '\0';
}
}
inline size_t TStringBase<T>::size() const {
return mSize;
}

template <class T>
inline bool TStringBase<T>::operator==( const TStringBase<T> &rhs ) const {
if (rhs.m_size != m_size) {
return false;
}
inline size_t TStringBase<T>::capacity() const {
return mCapacity;
}

for (size_t i = 0; i < m_size; ++i) {
if (rhs.m_pStringBuffer[i] != m_pStringBuffer[i]) {
return false;
}
template <class T>
inline const T *TStringBase<T>::c_str() const {
if (mStringBuffer != nullptr) {
return mStringBuffer;
}

return true;
return mBuffer;
}

template <class T>
inline bool TStringBase<T>::operator!=(const TStringBase<T> &rhs) const {
return !(*this == rhs);
inline void TStringBase<T>::clear() {
if (mStringBuffer != nullptr) {
delete [] mStringBuffer;
mStringBuffer = nullptr;
mCapacity = InitSize;
}
mSize = 0;
}

template <class TCharType>
class TStringView {
public:
TStringView(TCharType *ptr);
~TStringView();
size_t size() const;
TCharType *data() const;

private:
TCharType *mPtr;
size_t mLen;
};

template <class TCharType>
inline TStringView<TCharType>::TStringView(TCharType *ptr) :
mPtr(ptr),
mLen(0) {
if (nullptr != mPtr) {
mLen = strlen(ptr);
template <class T>
inline void TStringBase<T>::copyFrom(TStringBase<T> &base, const T *ptr, size_t size) {
if (ptr != nullptr) {
T *targetPtr = base.mBuffer;

if (size > 0) {
if (size > base.mCapacity) {
base.mStringBuffer = new T[size];
base.mCapacity = size;
targetPtr = base.mStringBuffer;
}
memcpy(targetPtr, ptr, size);
base.mSize = size;
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Address unsafe memory operations and missing null termination.

The copyFrom method has several critical issues:

  1. Using memcpy for potentially non-trivial types is unsafe
  2. No null termination for C-style strings
  3. Missing size validation
 inline void TStringBase<T>::copyFrom(TStringBase<T> &base, const T *ptr, size_t size) {
+    static_assert(std::is_trivially_copyable_v<T>, "T must be trivially copyable");
     if (ptr != nullptr) {
         T *targetPtr = base.mBuffer;
         
         if (size > 0) {
-            if (size > base.mCapacity) {
-                base.mStringBuffer = new T[size];
-                base.mCapacity = size;
+            size_t requiredCapacity = size + 1; // +1 for null terminator
+            if (requiredCapacity > base.mCapacity) {
+                base.mStringBuffer = new T[requiredCapacity];
+                base.mCapacity = requiredCapacity;
                 targetPtr = base.mStringBuffer;
             }
             memcpy(targetPtr, ptr, size);
+            targetPtr[size] = T{}; // Null termination
             base.mSize = size;
         }
     }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
inline void TStringBase<T>::copyFrom(TStringBase<T> &base, const T *ptr, size_t size) {
if (ptr != nullptr) {
T *targetPtr = base.mBuffer;
if (size > 0) {
if (size > base.mCapacity) {
base.mStringBuffer = new T[size];
base.mCapacity = size;
targetPtr = base.mStringBuffer;
}
memcpy(targetPtr, ptr, size);
base.mSize = size;
}
}
}
inline void TStringBase<T>::copyFrom(TStringBase<T> &base, const T *ptr, size_t size) {
static_assert(std::is_trivially_copyable_v<T>, "T must be trivially copyable");
if (ptr != nullptr) {
T *targetPtr = base.mBuffer;
if (size > 0) {
size_t requiredCapacity = size + 1; // +1 for null terminator
if (requiredCapacity > base.mCapacity) {
base.mStringBuffer = new T[requiredCapacity];
base.mCapacity = requiredCapacity;
targetPtr = base.mStringBuffer;
}
memcpy(targetPtr, ptr, size);
targetPtr[size] = T{}; // Null termination
base.mSize = size;
}
}
}
🤖 Prompt for AI Agents
In include/cppcore/Common/TStringBase.h around lines 128 to 142, the copyFrom
method uses memcpy which is unsafe for non-trivial types, lacks null termination
for C-style strings, and does not validate the size properly. Replace memcpy
with element-wise copy or std::copy to safely handle non-trivial types, ensure
the copied string is null-terminated by adding a terminator if applicable, and
add checks to validate that size does not exceed capacity or other logical
constraints before copying.


template <class TCharType>
inline TStringView<TCharType>::~TStringView() {
// empty
}
template <class T>
inline bool TStringBase<T>::operator == (const TStringBase<T> &rhs) const {
if (rhs.mSize != mSize) {
return false;
}

template <class TCharType>
inline size_t TStringView<TCharType>::size() const {
return mLen;

return mHashId == rhs.mHashId;
}

template <class TCharType>
inline TCharType *TStringView<TCharType>::data() const {
template <class T>
inline bool TStringBase<T>::operator != (const TStringBase<T> &rhs) const {
return !(*this == rhs);
}

} // namespace cppcore
88 changes: 88 additions & 0 deletions include/cppcore/Common/TStringView.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*-----------------------------------------------------------------------------------------------
The MIT License (MIT)

Copyright (c) 2014-2025 Kim Kulling

Permission is hereby granted, free of charge, to any person obtaining a copy of
this software and associated documentation files (the "Software"), to deal in
the Software without restriction, including without limitation the rights to
use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of
the Software, and to permit persons to whom the Software is furnished to do so,
subject to the following conditions:

The above copyright notice and this permission notice shall be included in all
copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
-----------------------------------------------------------------------------------------------*/
#pragma once

#include <cppcore/CPPCoreCommon.h>

namespace cppcore {

//-------------------------------------------------------------------------------------------------
/// @class TStringView
/// @ingroup CPPCore
///
/// @brief
//-------------------------------------------------------------------------------------------------
template <class T>
class TStringView {
public:
using const_iterator = const T*;

TStringView(const T *ptr, size_t len);
~TStringView() = default;
size_t size() const;
T *data() const;
bool isEmpty() const;
const_iterator begin() const;
const_iterator end() const;

private:
const T *mPtr;
size_t mLen;
};

template <class T>
inline TStringView<T>::TStringView(const T *ptr, size_t len) :
mPtr(ptr),
mLen(len) {
// empty
}

template <class T>
inline size_t TStringView<T>::size() const {
return mLen;
}

template <class T>
inline T *TStringView<T>::data() const {
return mPtr;
}

template <class T>
inline bool TStringView<T>::isEmpty() const {
return mLen == 0;
}

template <class T>
inline typename TStringView<T>::const_iterator TStringView<T>::begin() const {
if (isEmpty()) {
return end();
}
return mPtr;
}

template <class T>
inline typename TStringView<T>::const_iterator TStringView<T>::end() const {
return mPtr + mLen;
}

} // namespace cppcore
1 change: 0 additions & 1 deletion include/cppcore/Memory/MemUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
#pragma once

#include <cppcore/CPPCoreCommon.h>
#include <string.h>
#include <cinttypes>

namespace cppcore {
Expand Down
Loading