patterncppModerate
Safe strcpy implementation with C++
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
I have a few worries about my implementation:
-
The lines with
-
Should I be concerned with any performance issues or any "gotchas" that I didn't think of?
-
Is this a viable solution to the
```
#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
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
What way, in your
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
But we can just use those same conditions to SFINAE the
And now we have no need for
If you really want to keep the type, I'd just inherit:
memcpy to
Instead of copying raw bytes, and having to remember to multiply, you can just use
Write:
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::copyInstead 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.