reading binary from a file gives negative number

Question

Hey everyone this may turn out to be a simple stupid question, but one that has been giving me headaches for a while now. I'm reading data from a Named Binary Tag file, and the code is working except when I try to read big-endian numbers. The code that gets an integer looks like this:

long NBTTypes::getInteger(istream &in, int num_bytes, bool isBigEndian)
{
    long result = 0;
    char buff[8];

    //get bytes
    readData(in, buff, num_bytes, isBigEndian);

    //convert to integer
    cout <<"Converting bytes to integer..." << endl;
    result = buff[0];
    cout <<"Result starts at " << result << endl;
    for(int i = 1; i < num_bytes; ++i)
    {
        result = (result << 8) | buff[i];
        cout <<"Result is now " << result << endl;
    }
    cout <<"Done." << endl;

    return result;
}

And the readData function:

void NBTTypes::readData(istream &in, char *buffer, unsigned long num_bytes, bool BE)
{
    char hold;

    //get data
    in.read(buffer, num_bytes);

    if(BE)
    {
        //convert to little-endian
        cout <<"Converting to a little-endian number..." << endl;
        for(unsigned long i = 0; i < num_bytes / 2; ++i)
        {
            hold = buffer[i];
            buffer[i] = buffer[num_bytes - i - 1];
            buffer[num_bytes - i - 1] = hold;
        }
        cout <<"Done." << endl;
    }
}

This code originally worked (gave correct positive values), but now for whatever reason the values I get are either over or underflowing. What am I missing?

Answer 1

Your byte order swapping is fine, however building the integer from the sequences of bytes is not.

First of all, you get the endianness wrong: the first byte you read in becomes the most significant byte, while it should be the other way around.

Then, when OR-ing in the characters from the array, be aware that they are promoted to an int, which, for a signed char, sets a lot of additional bits unless you mask them out.

Finally, when long is wider than num_bytes, you need to sign-extend the bits.

This code works:

union {
    long s;           // Signed result
    unsigned long u;  // Use unsigned for safe bit-shifting
} result;

int i = num_bytes-1;
if (buff[i] & 0x80)
    result.s = -1; // sign-extend
else
    result.s = 0;
for (; i >= 0; --i)
    result.u = (result.u << 8) | (0xff & buff[i]);
return result.s;