Memory layout of struct having bitfields

Question 1

No, bitfields are not good for this purpose. The layout is compiler-dependant.

It's generally not a good idea to use bitfields for data where you want to control the resulting layout, unless you have (compiler-specific) means, such as #pragmas, to do so.

The best way is probably to implement this without bitfields, i.e. by doing the needed bitwise operations yourself. This is annoying, but way easier than somehow digging up a way to fix this. Also, it's platform-independent.

Define the header as just an array of 16-bit words, and then you can compute the checksum easily enough.

Question 2

The C11 standard says:

An implementation may allocate any addressable storage unit large enough to hold a bitfield. If enough space remains, a bit-field that immediately follows another bit-field in a structure shall be packed into adjacent bits of the same unit. If insufficient space remains, whether a bit-field that does not fit is put into the next unit or overlaps adjacent units is implementation-defined. The order of allocation of bit-fields within a unit (high-order to low-order or low-order to high-order) is implementation-defined.

I'm pretty sure this is undesirable, as it means there might be padding between your fields, and that you can't control the order of your fields. Not just that, but you're at the whim of the implementation in terms of network byte order. Additionally, imagine if an unsigned int is only 16 bits, and you're asking to fit a 32-bit bitfield into it:

The expression that specifies the width of a bit-field shall be an integer constant expression with a nonnegative value that does not exceed the width of an object of the type that would be specified were the colon and expression omitted.

I suggest using an array of unsigned chars instead of a struct. This way you're guaranteed control over padding and network byte order. Start off with the size in bits that you want your structure to be, in total. I'll assume you're declaring this in a constant such as IP_PACKET_BITCOUNT: typedef unsigned char ip_packet[(IP_PACKET_BITCOUNT / CHAR_BIT) + (IP_PACKET_BITCOUNT % CHAR_BIT > 0)];

Write a function, void set_bits(ip_packet p, size_t bitfield_offset, size_t bitfield_width, unsigned char *value) { ... } which allows you to set the bits starting at p[bitfield_offset / CHAR_BIT] bit bitfield_offset % CHARBIT to the bits found in value, up to bitfield_width bits in length. This will be the most complicated part of your task.

Then you could define identifiers for VER_OFFSET 0 and VER_WIDTH 4, HLEN_OFFSET 4 and HLEN_WIDTH 4, etc to make modification of the array seem less painless.

Question 3

Although question was asked long time back, there's no answer with explaination of your result. I'll answer it, hopefully it'll be useful to someone.

I'll illustrate the bug using first 16 bits of your data structure.

Please Note: This explaination is guarranteed to be true only with the set of your processor and compiler. If any of these changes, behaviour may change.

Fields:

unsigned int ver   : 4;
unsigned int hlen  : 4;
unsigned int stype : 8;

Assigned to:

dgram.ver   = 4;
dgram.hlen  = 5;
dgram.stype = 0;

Compiler starts assigning bit fields starting with offset 0. This means first byte of your data structure is stored in memory as:

Bit offset: 7     4     0
            -------------
            |  5  |  4  |
            -------------

First 16 bits after assignment look like this:

Bit offset:     15   12    8     4     0
                -------------------------
                |  5  |  4  |  0  |  0  |
                -------------------------
Memory Address: 100          101

You are using Unsigned 16 pointer to dereference memory address 100. As a result address 100 is treated as LSB of a 16 bit number. And 101 is treated as MSB of a 16 bit number.

If you print *ptr in hex you'll see this:

*ptr = 0x0054

Your loop is running on this 16 bit value and hence you get:

00000000 0101 0100
-------- ---- ----
   0       5    4

Solution: Change order of elements to

unsigned int hlen  : 4;
unsigned int ver   : 4;
unsigned int stype : 8;

And use unsigned char * pointer to traverse and print values. It should work.

Please note, as others've said, this behavior is platform and compiler specific. If any of these changes, you need to verify that memory layout of your data structure is correct.

Question 4

For Chinese users, I think you can refer blog for more details, really good.

In summary, due to endianness, there is byte order as well as bit order. Bit order is the order how each bit of one byte saved in memory. Bit order has same rule with byte order in sense of endianness issue.

For your picture, it's designed in network order which is big endian. So your struct defination is actually for big endian. Per your output, your PC is little endian, so you need change struct field orders when use.

The way to show each bits is incorrect since when get by char, the bit order has changed from machine order (little endian in your case) to normal order which we human use. You may change it as following per refered blog.

void
dump_native_bits_storage_layout(unsigned char *p, int bytes_num)
{

    union flag_t {
        unsigned char c;
        struct base_flag_t {
            unsigned int p7:1,
                        p6:1,
                        p5:1,
                        p4:1,
                        p3:1,
                        p2:1,
                        p1:1,
                        p0:1;
        } base;
    } f;

    for (int i = 0; i < bytes_num; i++) {
        f.c = *(p + i);
        printf("%d%d%d%d %d%d%d%d ",
                        f.base.p7,
                        f.base.p6, 
                        f.base.p5, 
                        f.base.p4, 
                        f.base.p3,
                        f.base.p2, 
                        f.base.p1, 
                        f.base.p0);
    }
    printf("\n");
}

//prints 16 bits at a time
void print_dgram(struct ip_dgram dgram)
{
    unsigned char* ptr = (unsigned short int*)&dgram;
    int i,j;
    //print only 10 words
    for(i=0 ; i<10 ; i++)
    {
        dump_native_bits_storage_layout(ptr, 1);
        /* for(j=7 ; j>=0 ; j--)
        {
            if( (*ptr) & (1<<j) ) printf("1");
            else printf("0");
            if(j%8==0)printf(" ");
        }*/
        ptr++;
        //printf("\n");
    }
}

Question 5

@unwind

A typical use case of Bit Fields is interpreting/emulation of byte code or CPU instructions with given layout. "Don't use it, because you cannot control it" is the answer for children.

@Bruce

For Intel/GCC I see a packed LITTLE ENDIAN bit layout, i.e. in struct ip_dgram field ver is represented by bits 0..3, field hlen is represented by bits 4..7 ...

For correctness of operation it is required to verify the memory layout against your design at runtime.

struct ModelIndicator
{
    int a:4;
    int b:4;
    int c:4;
};

union UModelIndicator
{
    ModelIndicator i;
    int v;
};

// test packed little endian
static bool verifyLayoutModel()
{
    UModelIndicator um;
    um.v = 0;
    um.i.a = 2; // 0..3
    um.i.b = 3; // 4..7
    um.i.c = 9; // 8..11
    return um.v = (9 << 8) + (3 << 4) + 2;
}

int main()
{
    if (!verifyLayoutModel())
    {
        std::cerr << "Invalid memory layout" << std::endl; 
        return -1;
    }
    // ...
}

At the earliest, when above test fails, you need to consider compiler pragmas or adjust your structures accordingly, resp. verifyLayoutModel().

Question 6

Compiler dependant or not, It depends whether you want to write a very fast program or if you want one that works with different compilers. To write for C a fast, compact application, use a stuct with bit fields/. If you want a slow general purpose program , long code it.

Question 7

I agree with what unwind said. Bit fields are compiler dependent.

If you need the bits to be in a specific order, pack the data into a pointer to a character array. Increment the buffer the size of the element being packed. Pack the next element.

pack( char** buffer )
{
   if ( buffer & *buffer )
   {
     //pack ver
     //assign first 4 bits to 4. 
     *((UInt4*) *buffer ) = 4;
     *buffer += sizeof(UInt4); 

     //assign next 4 bits to 5
     *((UInt4*) *buffer ) = 5;
     *buffer += sizeof(UInt4); 

     ... continue packing
   }
}