safety in std::vector memory manipulation

Question

I have this following code

size_t returnSize(const char* s)
{
       string string(s);
       return string.size();
};

size_t returnSize(const int& i)
{
       return sizeof(i);
};


template<typename T>
vector<char> Serialize(const T& t)
{
    T* pt = new T(t);
    vector<char> CasttoChar;

    for (int i =0 ;i<returnSize(t);i++)
    {
        CasttoChar.push_back(reinterpret_cast<const char*>(pt)[i]);
    }
    delete pt;
    return CasttoChar;
};
template<typename T>
T DeSerialize(const vector<char> cstr)
{
    T* a = (T*)(&cstr[0]);

    return *a;
}

int _tmain(int argc, _TCHAR* argv[])
{
    int x = 97;
    vector<char> c = Serialize(x);
    cout << DeSerialize<int>(c) << endl;

    string k = "blabla";
    vector<char> c3 = Serialize(k.c_str());
    cout << DeSerialize<const char*>(c3) << endl;

    system("PAUSE");
    return EXIT_SUCCESS;
}

//output is 
//97
//blabla

Is this line T* a = (T*)(&cstr[0]); safe?

Also, I tried reinterpret_cast<T*>(&cstr[0]); instead of T* a = (T*)(&cstr[0]); but compiler complained about not being able to convert const char* to int*. so why does the C style cast work?

Answer 1

Refer the standard

Why reinterpret_cast fails?

5.2.10 Reinterpret cast [expr.reinterpret.cast]

The reinterpret_cast operator shall not cast away constness (5.2.11). An expression of integral, enumeration, pointer, or pointer-to-member type can be explicitly converted to its own type; such a cast yields the value of its operand.

Should I use C Cast? No. Using C Cast instead of C++ Cast is always unsafe. You are trying to remove the constness of an Object which is an UB. Using reinterpret_cast, will actually trap this error and advise you of during compile time of the potential pitfall.

You should actually use const_cast in this situation. Its the only legal way to convert a const object to a non const object

But Why does a C Cast works

Quoting from the accepted answer from the Question When should static_cast, dynamic_cast and reinterpret_cast be used?

A C-style cast is defined as the first of the following which succeeds:

const_cast
static_cast
static_cast, then const_cast
reinterpret_cast
reinterpret_cast, then const_cast

So fortunately, it tries the const_cast first.

Answer 2

The C-style cast works because it takes many steps in order to make the cast succeed. It uses the first of the following that succeeds:

const_cast
static_cast
static_cast + const_cast
reinterpret_cast
reinterpret_cast + const_cast

In this case, it's doing the most 'powerful' cast, a reinterpret_cast to const int * followed by const_cast to int*.

The reinterpret_cast alone won't compile, because you're casting away const-ness. The const_cast is required to cast to int*. Doing a reinterpret_cast to const int* would be fine, however.

As an aside, what you're doing is generally unsafe, unless you're using a compiler extension to ensure that any user-defined type you're deserializing to isn't padded.

Answer 3

C style casting in c++ is not a good idea precisily because you go past the checks that prevent you from removing a const or changing the type arbitrary. If you want to make the code work as is you first need to const_cast and then reinterpret_cast, but really try to avoid const casting. To avoid the warning using reinterpret_cast simply declare a as const T*.

Answer 4

Stick to C++ casts. The reason the reinterpret_cast didn't work is you were casting away constness, which isn't cool; you have to use a const_cast for that and you just shouldn't. C casts ignore this.

Having said that, what are you trying to achieve here? You have effectively casting to a char array and memcpying without the efficiency that would bring.

Answer 5

Sorry to chime in here, but your code is broken in several ways, and the casting is just one of them. Concerning the casting, as soon as you use the conversion from/to vector on something that is not just a simple int or so but requires a constructor it will fail. In any case, a two-step conversions from char const* to void const* to T const* is unfortunately necessary.

Now, other problems:

• Try the whole thing with a zero-size string. This should now fully answer your actual question, too: No, it's not safe.
• You are returning a pointer to a char from DeSerialize<char const*>(). This pointer points into memory owned by the given vector, which is passed by value and after returning from that function ceases to exist! It is pure luck that it seems to work.
• Even if you managed to somehow return a char const* from the function, think about who owns that memory now. The point is that this owner must also release the memory. Consider using std::string and making the char const* variant not compile using a specialization of your template.
• In general, if you mean this code earnest, begin adding unit tests. It might slow you down now but avoids errors while you go, thus saving time overall. Search for "test-driven development".
• There is nothing that assures that the string is NUL-terminated.
• Don't use new/delete unless you have to, prefer "plain" stack variables. If you do, take care of properly releasing the memory in case of exceptions (from push_back()). Use auto_ptr (C++98) or unique_ptr (C++11) to make sure the memory is released correctly.