2

Somewhat confusing title, i know.

I created a union that contains a "raw"-array and 2 structs. These 2 structs each contain a struct header_t, a union of structs and a uint8_t. i have attached only the inMsg_t since both structs are made up the same way.

typedef struct sAfRegisterSRSP{
    uint8_t Status;
} tAfRegisterSRSP;

typedef union InOutMsg_u{
    uint8_t raw[ZIGBEE_PAYLOAD_MAX];
    outMsg_t outMsg;
    inMsg_t inMsg;
}msg_t;   


typedef struct header_s{
    uint8_t len;    ///< packet len 
    uint8_t cmd0;   ///< type (Bit: 7-5) and subsystem (Bit 4-0))
    uint8_t cmd1;   ///< command identifier
}header_t;


typedef struct inMsg_s{
    header_t header;
    union payload_u{
        tAfRegisterSRSP               afRegisterSRSP;
        tAfDataRequestSRSP            afDataRequestSRSP;
        tAfIncomingMsgAREQ            afIncomingMsgAREQ;
        tAfDataConfirmAREQ            afDataConfirmAREQ;

        tZbStartRequestSRSP           zbStartRequestSRSP;
        tZbPermitJoiningRequestSRSP   zbPermitJoiningRequestSRSP;
    } payload;
    uint8_t checksum;   //just a placeholder; cannot be used practically
}inMsg_t;

Now, when i run the code the memory mapping of the header-bytes is fine. byte[0] of the raw array maps to the len-field in the header-struct. cmd0 and cmd1 as well. However, "status" from the afRegisterSRSP-struct does not point to the status-byte, but to the CRC-Byte, that's after the status byte. This is also clear when looking at the memory addresses.

The header fields are at address: 0x15F6, 0x15F7 and 0x15F8.

Then the first payload-byte is at 0x15FA and 0x15F9 is left out.

I have attached a screenshot of the memory as it is shown in the debugger.

I can't figure out why the one byte is skipped and how to fix it.

timrau
  • 22,578
  • 4
  • 51
  • 64
surreal
  • 33
  • 5
  • 3
    Please state why you expect a `struct` has to have a specific layout, including padding and endianess. Feel free to cite the applicable section of the standard. If you can't, do research about "serialisation" and "marshalling". – too honest for this site Jun 16 '16 at 12:46
  • And don't post images of text! – too honest for this site Jun 16 '16 at 12:47
  • 2
    You really should read about [Structure padding and packing](http://stackoverflow.com/questions/4306186/structure-padding-and-packing). – Some programmer dude Jun 16 '16 at 12:48
  • Don't use `union` to operate translation from structure to another one or from raw array. It's dangerous (as you have already discovered by yourself) and no portable. Prefer using function to pass from one to another – Garf365 Jun 16 '16 at 12:57
  • @Garf365 C allows type-punning between the object representations (conceptual `char` arrays) of `union` members. The real problem is when people expect the wrong things from said raw representation and/or draw wrongly broad conclusions from how one machine behaves - e.g. when they pun between architecture-defined basic types. (Or when they expect C++ to follow suit, where type punning is UB, unless the `union` members are standard-layout objects and we're alternating between their members that fall within the _common initial sequence_.) – underscore_d Jun 16 '16 at 13:09
  • Thanks @Joachim Pileborg for the link. I totally forgot about padding. – surreal Jun 16 '16 at 13:45

1 Answers1

1

The payload union is being aligned to a four byte boundary, and the header is only three bytes long. That means there is a pad byte in the C definition of the structure, but your memory mapping likely is packed, omitting all padding. You'd need to use a compiler specific flag, pragma, or attribute to specify that the various unions/structures are packed (to one byte alignment) to ensure no padding bytes are created. For example, gcc packing and Visual Studio packing.

Also, depending on your disk data format, you may need to be careful to perform byte swapping to switch from machine native endianness (often little endian, e.g. on x86) to network endianness (usually big endian, with exceptions for a few Windows specific protocols).

ShadowRanger
  • 143,180
  • 12
  • 188
  • 271
  • Accessing a `union` by a field that has a different type than what was last written is undefined behaviour. The approach is doomed from the start anyway. – too honest for this site Jun 16 '16 at 13:10
  • ...all of which, I'd say, indicate why this sort of code is a bad idea. Most people recommend using de/serialisation routines, and that's fine, but I get around this by using some extremely carefully defined templates that handle endianness and so forth in a reliable, standard-defined way. – underscore_d Jun 16 '16 at 13:11
  • 1
    @Olaf No, it _is_ defined, since C99: http://stackoverflow.com/a/11640603/2757035 C++, however, did not adopt this. – underscore_d Jun 16 '16 at 13:12
  • @underscore_d: Thanks for pointing out that difference. It still is dangerous and should only be done in a low-level function which knows about implementation details. IMO proper serialisation with shifts is still the better way (you might use a `union` to convert the `uint32_`/etc to the target types, though), as it avoids all pitfalls about endianess, padding, etc. Done correctly, this is directly portable and less error-prone. It also is extensible. Something like nanopb can help to generate the necessary management data. – too honest for this site Jun 16 '16 at 13:16
  • Thanks, i totally forgot about padding. I have now appended "__attribute__ ((packed))" (as per XC16 compiler manual) to the data structures to change the alignment to a one-byte alignment. – surreal Jun 16 '16 at 13:49