HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Homebrew std::string for use with kernel

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
stdwithhomebrewforkernelusestring

Problem

I've ported some standard library facilities like vector, algorithm, etc. to my kernel so I can code in C++ instead of C. This is one of them. I'm looking for concerns regarding:

  • Performance



  • Safety



  • Friendly interface



Perhaps there are more idiomatic ways to accomplish what I'm going for?

```
#ifndef _LIBCPP_STRING_H_
#define _LIBCPP_STRING_H_

#include
#include
#include
#include

#include "algorithm.h"

namespace std
{
template
class fixed_string
{
public:
static constexpr size_t npos = static_cast(UINT32_MAX);

fixed_string() :
_length(0)
{
_buffer[0] = '\0';
}
fixed_string(const char *string)
{
assign(string, strlen(string));
}
fixed_string(const fixed_string &other)
{
assign(other._buffer, other._length);
}

fixed_string &operator =(const fixed_string &other)
{
assign(other._buffer, other._length);
return *this;
}
fixed_string &operator =(const char *string)
{
assign(string, strlen(string));
return *this;
}

bool operator ==(const fixed_string &other) const
{
if(_length != other._length)
return false;

return (strcmp(other._buffer, _buffer) == 0);
}
bool operator ==(const char *string) const
{
return (strcmp(string, _buffer) == 0);
}

bool operator !=(const fixed_string &other) const
{
return !(*this == other);
}
bool operator !=(const char *string) const
{
return !(*this == string);
}

size_t find(const fixed_string &other) const
{
return find(other._buffer, _length);
}
size_t find(const char *other) const
{
return find(other, strlen(other));
}
size_t find(const ch

Solution

In operator=(), you should always first check to see whether you're doing a self-assignment and do an early exit:

fixed_string &operator =(const fixed_string &other)
{
    if (this == &other) {
        return *this;
    }
    assign(other._buffer, other._length);
    return *this;
}


I would write the found assignment as the more explicit:

if (*buffer == other[found]) {
    found++;
} else {
    found = 0;
}


The compiler will generate the equivalent code to yours.

In fact, your find has a bug. Consider:

fixed_string a = "abcabcd";
assert(a.find("abcd") == 3);


Your assign method calls strlcpy incorrectly. Instead of calling with _length as the third parameter, use the actual size of the buffer:

strlcpy(_buffer, string, sizeof(_buffer));


As a matter of style, I would always surround conditional blocks with braces, even if they only contain one statement:

bool operator ==(const fixed_string &other) const
{
    if(_length != other._length) {
        return false;
    }
    // ...

Code Snippets

fixed_string &operator =(const fixed_string &other)
{
    if (this == &other) {
        return *this;
    }
    assign(other._buffer, other._length);
    return *this;
}
if (*buffer == other[found]) {
    found++;
} else {
    found = 0;
}
fixed_string<10> a = "abcabcd";
assert(a.find("abcd") == 3);
strlcpy(_buffer, string, sizeof(_buffer));
bool operator ==(const fixed_string &other) const
{
    if(_length != other._length) {
        return false;
    }
    // ...

Context

StackExchange Code Review Q#61735, answer score: 3

Revisions (0)

No revisions yet.