I came across a code snippet where the bytes from a buffer were copied into a struct variable using memcpy():
MtaHeader m_hdr;
memcpy(&m_hdr,&p_data[position],sizeof(MtaHeader));
I changed it to:
MtaHeader* m_hdr = reinterpret_cast<MtaHeader*>(&p_data[position]);
My aim was to avoid copying of buffered data from p_data into local variable m_hdr, instead just make an MtaHeader* pointer point to the p_data.
Is what I did correct, and did I make the code faster? Will the elements of the struct get correct values?
On the other hand, if I want to copy the buffer data to a local variable, is there any better way than memcpy() in this case?
CodePudding user response：
The biggest problem here is alignment -- if position is such that the resulting pointer is misaligned, using the reinterpret_cast and dereferencing the resulting pointer gives undefined behavior.
There's also the issue that MtaHeader needs to be trivially-copyable or the memcpy will not work.
CodePudding user response：
My question is, what i did is correct
It depends on p_data is, and what MtaHeader is. But most likely what you did is not correct and the behviour of the program is undefined. That would be bad.
if i want to copy the buffer data to local value, is there any better way than memcpy in this case?
In some cases, std::bit_cast may be better. But it cannot be used in all cases, and using std::memcopy is fine assuming that the preconditions are satisfied.
CodePudding user response：
Assuming that p_data is some random buffer of data then your solution is both technically and practically wrong. It will fail on every architecture where unaligned access is not handled by the CPU.
The code with memcpy is only correct when MtaHeader is trivially-copyable and the data doesn't need any endian conversion.
To make this always correct you have to extract the data from the buffer byte by byte, recombine them into the proper types and call the constructor for MtaHeader. For some types that might mean first shuffeling the bytes to correct for endianess and then memcpy() them into variables of the correct type, e.g. for double.
But nobody writes technically correct code for stuff like this. Everybody takes shortcuts and writes practically working code that depends on architecture and/or compiler defined behavior.
Note: a type like double is technically even more complex since you have to deal with INFINITY and NAN and denormalized numbers that your arch might not even have.
I recommend using an existing framework for stuff like this, like google protobuf if you care about compatibility with other systems.
CodePudding user response：
Your variant, assuming p_data position isn't already pointing to a MtaHeader object or an object pointer-interconvertible with such an object (unlikely if that is supposed to be a byte buffer) and when you actually use the resulting pointer to access its members directly, has always undefined behavior.
Depending on the exact details, it may only be "technically" undefined behavior (because of a lack of std::launder) and work as expected on most compilers, but in other cases, such as for example if you don't assure that p_data position is properly aligned for MtaHeader, it will straight-up result in a compiled program that crashes or misbehaves on common compilers/architectures.
The original version will also be wrong if MtaHeader is not trivially-copyable or too large for the buffer bounds. Both can be checked with static_assert. In many cases since C 20, the memcpy version can be replaced by std::bit_cast, which has some more built-in protections regarding these requirements.
Even then it is of course assumed that the buffer actually holds a valid object representation for MtaHeader. If the contents of the buffer were not obtained by copying the object representation from a MtaHeader in the same program, then there are all sorts of possible representation issues, especially when the data comes from another machine. A common one being for example endianess.
Compilers are often (again depending on the exact circumstances) clever enough to optimize away the copy in the original version if it is not needed.
So in short, don't do that.