[ox/mc] Fix VLI encoding to handle negatives

This commit is contained in:
Gary Talent 2019-03-16 14:41:43 -05:00
parent a9b7b00011
commit e4aab88831
3 changed files with 39 additions and 23 deletions

View File

@ -13,20 +13,23 @@
namespace ox::mc { namespace ox::mc {
template<typename T>
static constexpr auto Bits = sizeof(T) << 3;
/** /**
* Returns highest bit other than possible signed bit. * Returns highest bit other than possible signed bit.
* Bit numbering starts at 0. * Bit numbering starts at 0.
*/ */
template<typename I> template<typename I>
[[nodiscard]] constexpr auto highestBit(I val) noexcept { [[nodiscard]] constexpr std::size_t highestBit(I val) noexcept {
constexpr auto Bits = sizeof(I) * 8; auto shiftStart = sizeof(I) * 8 - 1;
// find most significant non-sign indicator bit // find most significant non-sign indicator bit
std::size_t highestBit = Bits; std::size_t highestBit = 0;
// start at one bit lower if signed // start at one bit lower if signed
if constexpr(ox::is_signed<I>) { if constexpr(ox::is_signed<I>) {
--highestBit; --shiftStart;
} }
for (int i = Bits - 1; i > -1; --i) { for (int i = shiftStart; i > -1; --i) {
const auto bitValue = (val >> i) & 1; const auto bitValue = (val >> i) & 1;
if (bitValue) { if (bitValue) {
highestBit = i; highestBit = i;
@ -36,11 +39,12 @@ template<typename I>
return highestBit; return highestBit;
} }
static_assert(highestBit(int8_t(0b10000000)) == 0);
static_assert(highestBit(1) == 0); static_assert(highestBit(1) == 0);
static_assert(highestBit(2) == 1); static_assert(highestBit(2) == 1);
static_assert(highestBit(4) == 2); static_assert(highestBit(4) == 2);
static_assert(highestBit(8) == 3); static_assert(highestBit(8) == 3);
static_assert(highestBit(1 << 31) == 31); static_assert(highestBit(uint64_t(1) << 31) == 31);
static_assert(highestBit(uint64_t(1) << 63) == 63); static_assert(highestBit(uint64_t(1) << 63) == 63);
struct McInt { struct McInt {
@ -50,28 +54,31 @@ struct McInt {
}; };
template<typename I> template<typename I>
McInt encodeInteger(I val) noexcept { [[nodiscard]] McInt encodeInteger(I input) noexcept {
McInt out; McInt out;
// move input to uint64_t to allow consistent bit manipulation, and to avoid
// overflow concerns
uint64_t val = *reinterpret_cast<Uint<Bits<I>>*>(&input);
if (val) { if (val) {
// bits needed to represent number factoring in space possibly // bits needed to represent number factoring in space possibly
// needed for signed bit // needed for signed bit
const auto bits = highestBit(val) + 1 + (ox::is_signed<I> ? 1 : 0); const auto bits = highestBit(val) + 1 + (ox::is_signed<I> ? 1 : 0);
// bytes needed to store value // bytes needed to store value
std::size_t bytes = bits / 8 + (bits % 8 != 0); std::size_t bytes = bits / 8 + (bits % 8 != 0);
auto bitsAvailable = bytes * 8; const auto bitsAvailable = bytes * 8; // bits available to integer value
const auto bitsNeeded = bits + bytes;
// factor in bits needed for bytesIndicator (does not affect bytesIndicator) // factor in bits needed for bytesIndicator (does not affect bytesIndicator)
// bits for integer + bits neded to represent bytes > bits available // bits for integer + bits neded to represent bytes > bits available
if (bits + bytes > bitsAvailable && bytes != 9) { if (bitsNeeded > bitsAvailable && bytes != 9) {
++bytes; ++bytes;
bitsAvailable = bytes * 8;
} }
const auto bytesIndicator = onMask<uint8_t>(bytes - 1); // << (7 - bytes); const auto bytesIndicator = onMask<uint8_t>(bytes - 1);
// move sign bit // move sign bit
if constexpr(ox::is_signed<I>) { if constexpr(ox::is_signed<I>) {
if (val < 0) { if (val < 0) {
val ^= int64_t(1) << (sizeof(I) * 8 - 1); *reinterpret_cast<uint64_t*>(&val) ^= uint64_t(1) << (sizeof(I) * 8 - 1);
val |= int64_t(1) << (bitsAvailable - 1); *reinterpret_cast<uint64_t*>(&val) |= uint64_t(1) << (bitsAvailable - 1);
} }
} }
// ensure we are copying from little endian represenstation // ensure we are copying from little endian represenstation

View File

@ -142,11 +142,12 @@ std::map<std::string, ox::Error(*)()> tests = {
constexpr auto check = [](McInt val, std::vector<uint8_t> &&expected) { constexpr auto check = [](McInt val, std::vector<uint8_t> &&expected) {
if (val.length != expected.size()) { if (val.length != expected.size()) {
std::cout << "val.length: " << val.length << ", expected: " << expected.size() << '\n';
return OxError(1); return OxError(1);
} }
for (std::size_t i = 0; i < expected.size(); i++) { for (std::size_t i = 0; i < expected.size(); i++) {
if (expected[i] != val.data[i]) { if (expected[i] != val.data[i]) {
std::cout << static_cast<uint32_t>(val.data[i]) << '\n'; std::cout << i << ": " << static_cast<uint32_t>(val.data[i]) << '\n';
return OxError(1); return OxError(1);
} }
} }
@ -165,14 +166,23 @@ std::map<std::string, ox::Error(*)()> tests = {
return OxError(0); return OxError(0);
}; };
oxAssert(check(encodeInteger(int64_t(1)), {0b0010}), "Encode 1 fail"); oxAssert(check(encodeInteger(int64_t(1)), {0b00000010}), "Encode 1 fail");
oxAssert(check(encodeInteger(int64_t(2)), {0b0100}), "Encode 2 fail"); oxAssert(check(encodeInteger(int64_t(2)), {0b00000100}), "Encode 2 fail");
oxAssert(check(encodeInteger(int64_t(3)), {0b0110}), "Encode 3 fail"); oxAssert(check(encodeInteger(int64_t(3)), {0b00000110}), "Encode 3 fail");
oxAssert(check(encodeInteger(int64_t(4)), {0b1000}), "Encode 4 fail"); oxAssert(check(encodeInteger(int64_t(4)), {0b00001000}), "Encode 4 fail");
oxAssert(check(encodeInteger(int64_t(128)), {0b0001, 0b10}), "Encode 128 fail"); oxAssert(check(encodeInteger(int64_t(128)), {0b00000001, 0b10}), "Encode 128 fail");
oxAssert(check(encodeInteger(int64_t(129)), {0b0101, 0b10}), "Encode 129 fail"); oxAssert(check(encodeInteger(int64_t(129)), {0b00000101, 0b10}), "Encode 129 fail");
oxAssert(check(encodeInteger(int64_t(130)), {0b1001, 0b10}), "Encode 130 fail"); oxAssert(check(encodeInteger(int64_t(130)), {0b00001001, 0b10}), "Encode 130 fail");
oxAssert(check(encodeInteger(int64_t(131)), {0b1101, 0b10}), "Encode 131 fail"); oxAssert(check(encodeInteger(int64_t(131)), {0b00001101, 0b10}), "Encode 131 fail");
oxAssert(check(encodeInteger(int64_t(-1)), {255, 255, 255, 255, 255, 255, 255, 255, 255}), "Encode -1 fail");
oxAssert(check(encodeInteger(int64_t(-2)), {255, 254, 255, 255, 255, 255, 255, 255, 255}), "Encode -2 fail");
oxAssert(check(encodeInteger(int64_t(-3)), {255, 253, 255, 255, 255, 255, 255, 255, 255}), "Encode -3 fail");
oxAssert(check(encodeInteger(int64_t(-4)), {255, 252, 255, 255, 255, 255, 255, 255, 255}), "Encode -4 fail");
oxAssert(check(encodeInteger(int64_t(-128)), {255, 128, 255, 255, 255, 255, 255, 255, 255}), "Encode -128 fail");
oxAssert(check(encodeInteger(int64_t(-129)), {255, 127, 255, 255, 255, 255, 255, 255, 255}), "Encode -129 fail");
oxAssert(check(encodeInteger(int64_t(-130)), {255, 126, 255, 255, 255, 255, 255, 255, 255}), "Encode -130 fail");
oxAssert(check(encodeInteger(int64_t(-131)), {255, 125, 255, 255, 255, 255, 255, 255, 255}), "Encode -131 fail");
oxAssert(check(encodeInteger(uint64_t(1)), {0b0010}), "Encode 1 fail"); oxAssert(check(encodeInteger(uint64_t(1)), {0b0010}), "Encode 1 fail");
oxAssert(check(encodeInteger(uint64_t(2)), {0b0100}), "Encode 2 fail"); oxAssert(check(encodeInteger(uint64_t(2)), {0b0100}), "Encode 2 fail");

View File

@ -142,7 +142,6 @@ Error MetalClawWriter::appendInteger(I val) {
if (val) { if (val) {
if (m_buffIt + sizeof(I) < m_buffLen) { if (m_buffIt + sizeof(I) < m_buffLen) {
LittleEndian<I> leVal = val; LittleEndian<I> leVal = val;
mc::encodeInteger(val);
// bits needed to represent number factoring in space possibly needed // bits needed to represent number factoring in space possibly needed
// for signed bit // for signed bit
const auto bits = mc::highestBit(val) + (ox::is_signed<I> ? 1 : 0) / 8; const auto bits = mc::highestBit(val) + (ox::is_signed<I> ? 1 : 0) / 8;