patterncppMinor
Packing and unpacking two 32-bit integers into an unsigned 64-bit integer
Viewed 0 times
bitintounsignedpackingtwoandintegersunpackinginteger
Problem
Does this look good for combining/retrieving two 32 bit integers (a type and an index) into/from an unsigned 64 bit integer (a unique ID)?
The point is to hide in the API the composition of the unique ID and allow its calculation to change later without changing the API (e.g. possibility to change later to consecutive 64 bit ids without changing API).
Or would a union approach like below be better? Is this
Here is some code I used to test:
```
#include
#include
#include
template
std::string bits(T num)
{
const int num_bits = sizeof(num) * 8;
T maxPow = T(1) ::max();
int in_high = std::numeric_limits::min();
uint64_t combined = combine(in_low, in_high);
assert( bits(in_high) + bits(in_low) == bits(combined) );
assert( in_low == low(combined) );
assert( in_high == high(combined) );
}
}
void test2() {
Id in; // would "Id in = {-3, 99};" be better? Is this legal C++03? Would a constructor be better?
in.split.type = -3;
in.split.index = 99;
std::cout << in.unique_id << std::endl; // prints 18446744060824649827
std::cout << bits(in.split.type) << bits(in.split.index) << std::endl;
std::cout << bits(in.unique_id) << std::endl; // prints 11111111 11111111 11111111 11111101 00000000 00000000 00000000 01100011
assert(bits(in.split.type) + bits(in.split.index) == bits(in.unique_id));
Id out;
out.unique_id = in.unique_id;
std::cout << int(out.split.type
The point is to hide in the API the composition of the unique ID and allow its calculation to change later without changing the API (e.g. possibility to change later to consecutive 64 bit ids without changing API).
#include
uint64_t combine(uint32_t low, uint32_t high)
{
return (((uint64_t) high) > 32;
}
uint32_t low(uint64_t combined)
{
uint64_t mask = std::numeric_limits::max();
return mask & combined; // should I just do "return combined;" which gives same result?
}Or would a union approach like below be better? Is this
union guaranteed to fit into 64 bits (e.g. guarantee no padding in the struct)?union Id
{
struct
{
uint32_t index; // lower 32 bits
uint32_t type; // upper 32 bits
} split;
uint64_t unique_id;
};Here is some code I used to test:
```
#include
#include
#include
template
std::string bits(T num)
{
const int num_bits = sizeof(num) * 8;
T maxPow = T(1) ::max();
int in_high = std::numeric_limits::min();
uint64_t combined = combine(in_low, in_high);
assert( bits(in_high) + bits(in_low) == bits(combined) );
assert( in_low == low(combined) );
assert( in_high == high(combined) );
}
}
void test2() {
Id in; // would "Id in = {-3, 99};" be better? Is this legal C++03? Would a constructor be better?
in.split.type = -3;
in.split.index = 99;
std::cout << in.unique_id << std::endl; // prints 18446744060824649827
std::cout << bits(in.split.type) << bits(in.split.index) << std::endl;
std::cout << bits(in.unique_id) << std::endl; // prints 11111111 11111111 11111111 11111101 00000000 00000000 00000000 01100011
assert(bits(in.split.type) + bits(in.split.index) == bits(in.unique_id));
Id out;
out.unique_id = in.unique_id;
std::cout << int(out.split.type
Solution
Union provides no guarantee of memory arrangement for this. The C++ standard merely says that it's no bigger than the storage required for the largest element, and that it's designed to only have one active data member.
If you are only targeting one compiler and one architecture it may be okay to use it to translate between types, but it is definitely skiing off-piste.
I'd be tempted to wrap the whole thing up in a class and provide getter methods. This insulates the user from changes in the implementation of the API in the most flexible way. And, use the bitwise operations (>>, <<, | and &) inside the class. These operations will happen on a register, rather than in memory, and therefore do not suffer for endian issues.
If you are only targeting one compiler and one architecture it may be okay to use it to translate between types, but it is definitely skiing off-piste.
I'd be tempted to wrap the whole thing up in a class and provide getter methods. This insulates the user from changes in the implementation of the API in the most flexible way. And, use the bitwise operations (>>, <<, | and &) inside the class. These operations will happen on a register, rather than in memory, and therefore do not suffer for endian issues.
Context
StackExchange Code Review Q#80386, answer score: 2
Revisions (0)
No revisions yet.