Bit field structures in C++

I just recently learned how to use Bit Field Structures in C++, and I was thinking of applying it to a project. It would certainly make the project code much easier to work with. So, I typed up a quick test program to see if it would work as I expected, and it does. But, I'm not 100% convinced that it is safe.

I am fairly confident my CRC16 union is Endianess specific, but I'm not sure about the Bit Field struct, I suspect it is... Can anyone confirm this?

• Is this usage of a bit field struct safe?

• Can it be made safer?

• Is it Endianess specific?

Note: So far, I have only tested this on 64 bit Linux.

```
#include
#include
#include
#include

typedef union
{
int16_t Value;
struct
{
int8_t Lo;
int8_t Hi;
} Bytes; //This is all I could think of to call this. If you can think of something better, let me know.
} CRC16;

#ifdef _MSC_VER
#pragma pack(push, 1)
#endif
typedef struct
{
uint8_t SOH;
uint16_t Count : 14;
uint16_t Select : 1;
uint16_t Qsync : 1;
uint8_t Resp;
uint8_t Num;
uint8_t Addr;
CRC16 Blck;
#ifdef _MSV_VER
} DataHeader;
#pragma pack(pop)
#else
} __attribute__((packed)) DataHeader;
#endif

template
void PrintHex(std::string Description, T Value, long Mask = -1)
{
using namespace std;
cout (Value) & Mask)
<< resetiosflags(ios_base::showbase)
<< endl;
}

int main()
{

DataHeader test;
int8_t packet[8] = {0x64,0x42,0x81,0x42,0x43,0x01,0x32,0x97};

//this is the most important part, as this structure would be
//passed to Windows API calls, potentially file and socket I/O
//functions, etc.
memcpy(&test, packet, sizeof(packet));

std::cout << "sizeof(DataHeader) = " << sizeof(DataHeader) << std::endl;
PrintHex("SOH", test.SOH);
PrintHex("Count", test.Count);
PrintHex("Select", test.Select);
PrintHex("Qsync", test.Qsync);
PrintHex("Resp", test.Resp);
PrintHex("Num", test.Num);
PrintHex("Addr", te

#include 

At least officially, you're supposed to prefer to use ` instead. I have a hard time getting very excited about it though.

#include 

From the looks of the code, you really wanted to include here instead. You use an std::string in one place anyway (and I don't any use of anything from ).

typedef union 
{
    int16_t Value;
    struct
    {
        int8_t Lo;
        int8_t Hi;
    } Bytes; //This is all I could think of to call this. If you can think of something better, let me know.
} CRC16;

This usage is fairly typical in C, but in C++, I'd just use:

union CRC16 { 
    // ...
};

Just like with class/struct, there's no need for a typedef in C++.

As you guessed, this is endian-specific. Given that this is apparently an old DEC protocol, chances are that it's also little-endian. That's good to the degree that it works fine with Intel processors, but bad to the degree that the normal ntohl, ntohs (etc.) are all defined in terms of big-endian network format, so you can't use them to easily write portable code that converts endianness when needed.

#ifdef _MSC_VER
#pragma pack(push, 1)
#endif
typedef struct

Same comment as above--no real need for a typedef in C++.

uint16_t Count : 14;
uint16_t Select : 1;
uint16_t Qsync : 1;

You probably want to be aware that the mapping from bitfields to bits can vary (even independent of endianness). Not necessarily something you want to change or can fix, but with a different compiler (even with the same CPU) this could rather suddenly break.

#ifdef _MSV_VER

Minor typo--this should almost certainly be _MSC_VER.

template 
void PrintHex(std::string Description, T Value, long Mask = -1)
{
    using namespace std;
    cout (Value) & Mask)
         << resetiosflags(ios_base::showbase)
         << endl;
}

This has (IMO) a problem: after it's done, the hex flag is reset, even if it may have been set before the function was called. I'd rather use ios_base::flags() to retrieve the current flags, when again to restore then when finished. As (nearly) always when taking some action like that which you want to do, then un-do later, I'd prefer to put it into a small class that uses RAII to automate the restoring:

class flag_rest {
    std::ios_base::fmtflags original;
    std::ostream &s;
public:
    flag_rest(std::ostream &s) : original(s.flags()), s(s) {}
    ~flag_rest() { s.flags(original); }
};

Using that, printhex gets a little simpler:

template
void PrintHex(std::string Description, T Value, long Mask = -1)
{
    using namespace std;
    flag_rest f(cout);

    cout (Value) & Mask)
        << endl;
}

Probably more importantly, it also restores the flag to the state it held before the call, and does so dependably (even if the function exits via an exception). Of course, it's also nice that flag_rest is fairly reusable as well.

Of course, the cast to unsigned short` means this can (will) misbehave or any type larger than a short, though I doubt you really care a lot about it.

StackExchange Code Review Q#11360, answer score: 7