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

Safe strcpy implementation with C++

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

Problem

After reading this article, I've decided to take a crack at implementing a "safe_strcpy" function that would remove some of the errors that the author speaks about. Obviously, my safe_strcpy() function is inspired by his. Below, I've created a simple class that houses a std::array of one of the many different kinds of strings. My safe_strcpy() functions rely on always null-terminating the array given.

I have a few worries about my implementation:

-
The lines with out[N - 1] = static_cast(0) seem wrong when the underlying character type is wchar_t. In this sense, I want the array to terminate with something to the effect of L"0". Will this work? If not, how can I fix it?

-
Should I be concerned with any performance issues or any "gotchas" that I didn't think of?

-
Is this a viable solution to the safe_strcpy conundrum? In other words, would you use this class in production code? What kind of improvements or extensions should this class use?

```
#include
#include
#include
#include
#include
#include
using namespace std;

template
struct char_array
{
typedef T value_type;
typedef const T const_value_type;
typedef T& reference;
typedef const T& const_reference;
typedef std::array container;

typedef typename container::size_type size_type;
typedef typename container::iterator iterator;
typedef typename container::const_iterator const_iterator;

typedef typename container::reverse_iterator reverse_iterator;
typedef typename container::const_reverse_iterator const_reverse_iterator;

/ Constructor /
char_array()
{
static_assert((is_same::value || is_same::value ||
is_same::value || is_same::value ||
is_same::value || is_same::value) &&
N > 0,
"char_array initialized with invalid type or size");
}

/ Destructor /
~cha

Solution

Break out your type trait

You have this large amount of is_same checks in a couple places. Break it out in its own trait, and call it something like is_char:

template  is_char : std::false_type { };
template <> is_char : std::true_type { };
template <> is_char : std::true_type { };
template <> is_char : std::true_type { };
...


What way, in your char_array, you can just have:

static_assert(is_char::value, "invalid type, expected a char");
static_assert(N > 0, "invalid size, expected N > 0");


Furthermore, those asserts don't need to be in the constructor, they can be in the body of the class.

Lots of code Repetition

Pretty much all of your char_array code is just duplicating what std::array does. What does char_array actually give you? It costs you aggregate-initialization, and that's not nothing. Ultimateyl, it's really just a std::array<> with some extra conditions on T and N.

But we can just use those same conditions to SFINAE the safe_strcpy:

template ::value &&
                                   (N > 0) &&
                                   is_same::value>>
void safe_strcpy(std::array &out, const S *src)
{
    memcpy(out.data(), src, N * sizeof(T));
    out[N - 1] = 0;
}


And now we have no need for char_array at all.

If you really want to keep the type, I'd just inherit:

template 
struct char_array : std::array
{
    static_assert(is_char::value, "invalid type, expected a char");
    static_assert(N > 0, "invalid size, expected N > 0");
};


memcpy to std::copy

Instead of copying raw bytes, and having to remember to multiply, you can just use std::copy - which will under-the-hood do the same thing anyway, but will be less error-prone. So instead of:

memcpy(out.data(), src, N * sizeof(T));


Write:

std::copy(src, src+N, out.data());

Code Snippets

template <class T> is_char : std::false_type { };
template <> is_char<wchar_t> : std::true_type { };
template <> is_char<char> : std::true_type { };
template <> is_char<signed char> : std::true_type { };
...
static_assert(is_char<T>::value, "invalid type, expected a char");
static_assert(N > 0, "invalid size, expected N > 0");
template <class T, size_t N, class S,
          class = std::enable_if_t<is_char<T>::value &&
                                   (N > 0) &&
                                   is_same<T, S>::value>>
void safe_strcpy(std::array<T,N> &out, const S *src)
{
    memcpy(out.data(), src, N * sizeof(T));
    out[N - 1] = 0;
}
template <size_t N, class T=char>
struct char_array : std::array<T, N>
{
    static_assert(is_char<T>::value, "invalid type, expected a char");
    static_assert(N > 0, "invalid size, expected N > 0");
};
memcpy(out.data(), src, N * sizeof(T));

Context

StackExchange Code Review Q#114695, answer score: 12

Revisions (0)

No revisions yet.