DevShiftTeam / AppShift-MemoryPool

A very fast cross-platform memory pool mechanism for C++ built using a data-oriented approach (3 to 24 times faster than regular new or delete, depending on operating system & compiler)
Apache License 2.0
214 stars 25 forks source link

String.cpp is not copying the null terminator #7

Closed azureskydiver closed 3 years ago

azureskydiver commented 3 years ago

The implementation of the String class doesn't allocate space for the null terminator.

LessComplexity commented 3 years ago

Because we save the size I found no need to save a null terminator, is it something you find problematic? if so then why? so that I could consider if to add it or not :)

azureskydiver commented 3 years ago

Then you'll need to either change your implementation of data() or remove it. See the behavior of the std::basic_string::data() from C++11 onwards which includes the null terminator: https://en.cppreference.com/w/cpp/string/basic_string/data

Also, I have some doubts that the operator << overload works correctly. On a quick glance last night, it looked like it assumes a null terminated string.

LessComplexity commented 3 years ago

I use the size() to determine when to stop inserting character into the input stream, I actually modified the code to work this way in another project of mine and forgot to add it to this repo, though it exists here only for the test, which doesn't use the data() function anyways:

std::ostream& operator<<(std::ostream& os, const String& str)
{
    const char* data = str.data();
    for (int i = 0; i < str.size(); i++) os << data[i];
    os.flush();
    return os;
}

I will add a null terminator in the implementation since it is a much more standard way to manage strings anyways. Thanks for your time :)