CODEWITHSUNDEEP
c++
Haxor99
Haxor99

Reputation: 23

Add bytes from array into single value, unsigned bytes to uint64

//glues bytes together, into 64bit
UINT64 AppendBytes64(uint8_t* val, int start)
{
    UINT64 result = 0;
    result = (val[start+0] << 0) | (val[start + 1] << 8) | (val[start + 2] << 16) | (val[start + 3] << 24) | (val[start + 4] << 32) | (val[start + 5] << 40) | (val[start + 6] << 48) | (val[start + 7] << 56);
    return result;
}

So issue is that visual studio keeps warning that arithmetic overflow, left shift count is greater than operand size, help.

Upvotes: 2

Views: 646

Answers (5)

Mark Storer
Mark Storer

Reputation: 15870

Hard coded array access of multiple indexes always sets my teeth on edge past the first two or so. Call it "a bad code smell", call it "some old fart being retentive", whatever.

It's easy to turn this into a loop, so here it is. If your compiler decides to unroll it, then it'll do a better job than your or I could have anyway. That unrolled loop just might look like one of the other answers, but I wouldn't count on it.

uint64_t AppendBytes64(uint8_t* val, int start)
{
    uint64_t total = 0;
    for (int i = 0; i < 8; ++i)
    {
        uint64_t bigByte = val[start + i];
        total <<= 8;
        total += bigByte;
    }
    return total;
}

int main() {
    uint8_t data[] = { 0x1, 0x2, 0x3, 0x0, 0x0, 0x0, 0x0, 0x0 };

    uint64_t result = AppendBytes64(data, 0);
    printf("%llx\n", result);

    return 0;
}

The above code prints: 0102030000000000

Which is what I expected, and what I was going for initially. My off-the-cuff answer was closer to "correct" than I thought. Now if you want a different "endianess", that's a whole other problem.

Yep, and upon further reading of the question, the original poster did indeed want the other endianess, so more of a requirements translation problem rather than a "bug" per se.

None the less, the answer was still wrong, as pointed out in the comments. It's funny seeing this when I so often get on my son's case when working with him on his math homework about answering the question that was actually asked versus that conclusion you just jumped to. "Sins of the father" indeed.

So if the first byte is the least significant, then we instead write AppendBytes64 thusly:

uint64_t AppendBytes64(uint8_t* val, int start)
{
    uint64_t total = 0;
    for (int i = 7; i >= 0; --i)
    {
        uint64_t bigByte = val[start + i];
        total <<= 8;
        total += bigByte;
    }
    return total;
}

Which prints: 030201

This again is hex, only this time it's also the desired output.

Which brings up an interesting point. There's almost certainly an existing endian function in an existing library that will do this for you. I'm pretty sure this is a "little endian to native" function... which means that it'll break if the underlying native endian doesn't match my own. After a brief reading on the endian library docs, I suggest going with boost::endian::little_uint64_buf_t, which supports >> for pulling consecutive uint64's out of a stream of little endian bytes.

Of course I often get little and big endian backwards, but I think I actually have it right this time (but when don't I?!).

Upvotes: 1

Ted Lyngmo
Ted Lyngmo

Reputation: 117513

Implicit integral promotion

prvalues of small integral types (such as char) may be converted to prvalues of larger integral types (such as int). In particular, arithmetic operators do not accept types smaller than int as arguments, and integral promotions are automatically applied after lvalue-to-rvalue conversion, if applicable.

To avoid the implicit conversion, you'll have to do the conversion to a suitable type yourself and using C++ explicit type conversion and the standard type for an unsigned 64 bit integer, std::uint64_t, as well as an unsigned type for indexing, std::size_t, it should look like this:

#include <cstddef>
#include <cstdint>

using std::uint64_t;

constexpr uint64_t AppendBytes64(const uint8_t* val, std::size_t start) {
    return static_cast<uint64_t>(val[start + 0]) << 0 | 
           static_cast<uint64_t>(val[start + 1]) << 8 | 
           static_cast<uint64_t>(val[start + 2]) << 16 |
           static_cast<uint64_t>(val[start + 3]) << 24 |
           static_cast<uint64_t>(val[start + 4]) << 32 |
           static_cast<uint64_t>(val[start + 5]) << 40 |
           static_cast<uint64_t>(val[start + 6]) << 48 |
           static_cast<uint64_t>(val[start + 7]) << 56;
}

constexpr makes it possible to use the function in constant expressions to get a compile time constant out of it (given constexpr input) from C++11 and forward.

Upvotes: 2

Jabberwocky
Jabberwocky

Reputation: 50831

Before the shift takes place, the operand's type is promoted to int and it seems that on your platform int is 32 bits. Left shifting a 32 bit value by more than 31 results always in 0, hence the warning.

Cast your values to UINT64 prior to shifting them.

Like: (val[start + 5] << 40) -> ((UINT64)val[start + 5] << 40)

Upvotes: 1

FaisalM
FaisalM

Reputation: 756

Type cast your val to UINT64 before shifting.

result = ((UINT64)val[start + 0] << 0) | ((UINT64)val[start + 1] << 8) | ((UINT64)val[start + 2] << 16) | ((UINT64)val[start + 3] << 24) | ((UINT64)val[start + 4] << 32) | ((UINT64)val[start + 5] << 40) | ((UINT64)val[start + 6] << 48) | ((UINT64)val[start + 7] << 56);

Upvotes: 2

Vlad from Moscow
Vlad from Moscow

Reputation: 311068

There is used the integral promotions for the operands of the type uint8_t to the type int in the shift expressions. You have to cast the operands in the shift expressions to the type UINT64.

Upvotes: 1

Related Questions