#57 Use of non-portable Bit Fields capability

[Anonymous]

The code base utilizes the bit fields feature of C/C++ to represent bit-packed fields in several classes. This is known to be non-portable between different compilers/architectures since they may represent structures differently in memory..
A more ideal approach is to use old-fashion bit masking to get/set individual bits within the bit-packed fields.

An example of first-hand issue experienced with the K-DIS bit fields approach:

When developing in VS2015 on the X86 architecture. A read access of the m_ui8PDUStatus field within the Header7 class return '0' . A read access of the m_ui8PDUStatusFTI returned a '1' even though these fields are joined in a union and should have both resulted in a read of 0.

This particular example may be an issue with the m_PDUStatusUnion itself, since it consists of nested unions/bitfields which may result in undesirable behavior.

[Karl Jones]

My understanding was that if the bitfields are all the same type then it should be safe. I'll look into it although i suspect that is just a bug as I also use VS2015 ;)

[Todd Klasik]

This is actually a bug specifically with PDUStatus inside Header7. The inner struct wrapping statusFTI and bit5 unioned with RAI_IAI on vs2015 results in PDUStatus becoming 3 wide (I believe a struct isnt being allowed to be smaller than a single byte here). And m_ui8PDUStatusDTI_RAI_IAI is located in the 2nd bytes of the status. Presumably Bit6 and SM are in byte 3 though thats harder to check because offset of on bit fields seems to make offsetof angry.

Here is some example output from a small test I wrote as I was debugging why Detonations and Fires were getting incorrect Descriptors.

size: m_PDUStatusUnion: 3
offsetof: m_PDUStatusUnion: 0
offsetof: m_ui8PDUStatus: 0
offsetof: unionStruct: 0
offsetof: RAI_IAI: 1

I worked around this locally by removing the inner union and doing some fancy dancing with getters and setters to properly operate with DTI_RAI_IAI

[Todd Klasik]

typedef UINT8 KUINT8;

typedef struct name {
    union
    {
        struct
        {
            KUINT8 m_ui8PDUStatusTEI : 1;                   // Bit  0.
            KUINT8 m_ui8PDUStatusLVC : 2;                   // Bits 1-2.
            KUINT8 m_ui8PDUStatusCEI : 1;                   // Bit  3.

            union
            {
                struct
                {
                    KUINT8 m_ui8PDUStatusFTI : 1;        // Bit  4.
                    KUINT8 m_ui8PDUStatusBit5 : 1;        // Bit  5.
                };

                KUINT8 m_ui8PDUStatusDTI_RAI_IAI : 2;        // Bits 4-5. Could be used for DTI, RAI or IAI.
            } RAI_IAI;

            KUINT8 m_ui8PDUStatusBit6 : 1;                      // Bit  6. Unused.
            KUINT8 m_ui8PDUStatusSM : 1;                      // Bit  7.
        } unionStruct;

        KUINT8 m_ui8PDUStatus;

    } m_PDUStatusUnion;
} test;


int main() {
    printIt("size: m_PDUStatusUnion", sizeof(name));
    printIt("offsetof: m_PDUStatusUnion", offsetof(struct name, m_PDUStatusUnion));
    printIt("offsetof: m_ui8PDUStatus", offsetof(struct name, m_PDUStatusUnion.m_ui8PDUStatus));
    printIt("offsetof: unionStruct", offsetof(struct name, m_PDUStatusUnion.unionStruct));
    printIt("offsetof: RAI_IAI", offsetof(struct name, m_PDUStatusUnion.unionStruct.RAI_IAI));
}

Here is the code I used to isolate the issue if interested.

[Karl Jones]

Thanks. I think someone was doing some work to convert from using unions. Ill try and find it.