views:

77

answers:

1

I have the following function for reading a big-endian quadword (in a abstract base file I/O class):

unsigned long long File::readBigEndQuadWord(){
  unsigned long long qT = 0;
  qT |= readb() << 56;
  qT |= readb() << 48;
  qT |= readb() << 40;
  qT |= readb() << 32;
  qT |= readb() << 24;
  qT |= readb() << 16;
  qT |= readb() << 8;
  qT |= readb() << 0;
  return qT;
}

The readb() functions reads a BYTE. Here are the typedefs used:

typedef unsigned char   BYTE;
typedef unsigned short  WORD;
typedef unsigned long   DWORD;

The thing is that i get 4 compiler warnings on the first four lines with the shift operation:

warning C4293: '<<' : shift count negative or too big, undefined behavior

I understand why this warning occurs, but i can't seem to figure out how to get rid of it correctly. I could do something like:

qT |= (unsigned long long)readb() << 56;

This removes the warning, but isn't there any other problem, will the BYTE be correctly extended all the time? Maybe i'm just thinking about it too much and the solution is that simple. Can you guys help me out here? Thanks.

+1  A: 

Your way of removing the warning is correct. As you probably do already know, the warning is occurring because you're trying to shift the contents of a byte beyond the boundaries of a word, then store it in the quadword. This operation is undefined. (It will evaluate the right-side of the assignment before assigning the value.) By explicitly casting first, there is now enough space to do the shift, so there's nothing to complain about.

Arguably, the compiler should be able to figure out you're going to store it in the quadword, so it should allocate a quadword first and do the shift there, but it might not have been made smart enough to figure it out.

Also, I'm not sure about this, but possibly compiling this for x64 will also not generate the warning, as one word is 64 bits?

lc
Thanks for your answer. I myself thought that the compiler should be clever enough to see i'm assigning it to a variable big enough to hold all data (i'm using MSVS2008). But the warning brought doubts about the correctness of my code, so i asked here.
PeterK
If the compiler did figure out what you're doing with the result and change the intermediate types accordingly, then it would be breaking the language standard. Type promotion is well defined, and the result type of an operator only ever depends on the operand types.
Mike Seymour