1

I'm developing some embedded software, and trying to keep things a little more flexible. One of the things I want to be able to do is change a struct and have how the data is dealt with change throughout the rest of the application.

I've noticed that I can copy some data from a payload into a union, but for some reason not all of it. Here's what I'm doing:

union ConcentratorPacket {
 struct PacketHeader header;
 struct NetworkJoinReqPacket networkJoinReqPacket;
};

static union ConcentratorPacket latestRxPacket;

Where:

struct PacketHeader {
 uint32_t sourceAddress;
 uint8_t packetType; 
};

struct NetworkJoinReqPacket{
 struct PacketHeader header;
 uint8_t maxDataLen;
};

Later on I want to move the data from a received packet into the relevant struct within this union, so I would like to do this:

memcpy(&latestRxPacket.networkJoinReqPacket, rxPacket->payload, sizeof(latestRxPacket.networkJoinReqPacket));

Where rxPacket->payload is an array of uint8_t's, delivered in the correct order.

What I am seeing is that the packetHeader fills up nicely with this method, but the maxDataLen does not take the correct value. In fact the value it takes is payload[8] not payload[5].

The only way I've found to solve this is by doing a direct assignment for the maxDataLen, but that would need changed in every place if the struct changed for any reason, so memcpy is a preferable rather than this: 

 memcpy(&latestRxPacket.networkJoinReqPacket.header, rxPacket->payload, sizeof(latestRxPacket.networkJoinReqPacket.header));
 latestRxPacket.networkJoinReqPacket.maxDataLen = rxPacket->payload[5];

I think what I'm seeing indicated that the memcpy is treating the maxDataLen as a uint32, right justified, but I don't know how to avoid this.

It's strange because I do a similar thing somewhere else and it works fine, but the only difference is that the equivalent to maxDataLen is a uint32, not a uint8.

Any help or direction would be greatly appreciated.

timrau
  • 22,578
  • 4
  • 51
  • 64
Kureigu
  • 259
  • 4
  • 12
  • 3
    My guess it has to do with structure padding. What if you use your compilers pragmas or similar to pack the structures without padding? – Some programmer dude Sep 15 '17 at 11:33
  • Ahah, you nailed it! Wasn't familiar enough with structs to know about padding/packing just yet. Did a bit of googling and found out my compiler supports __attribute__((__packed__)), so I put in front of the PacketHeader and it made all the difference. Much appreciated! – Kureigu Sep 15 '17 at 11:44
  • 6
    Mandatory warning: Abusing structures for serialization purposes should be avoided because control over endianness and padding is limited. It's advisable to process each struct members indiviually. – user694733 Sep 15 '17 at 11:44
  • @user694733, could you elaborate please? – Kureigu Sep 15 '17 at 11:48
  • Your question is a prime example of the problem: To solve it you had to resort to compiler specific feature, which limits code portability. I quote the comment I made to similar post a while ago: *"You can start by reading [this wikipedia article](https://en.wikipedia.org/wiki/Serialization) or [this SO question](https://stackoverflow.com/q/6002528/694733). There are plenty of information on this on the net."* – user694733 Sep 15 '17 at 11:53
  • @user694733: If code will never need to be run on big-endian hardware, using structures for serialization was and remains a perfectly fine technique if structures are laid out so as to not require any padding on any plausible target. Most other approaches violate the DRY (Don't Repeat Yourself) principle by requiring the structure layout to be specified in three places--once in the structure declaration, once in the code that reads it, and once in the code that writes it. Compiler writers may view such code with disdain, but support for it is one of the advantages C has over other languages. – supercat Oct 03 '17 at 19:00
  • @supercat That's many *if*s to consider it portable. But if we ignore portability, structs still have problem of not being flexible. It doesn't handle the protocols that do require endianness switches or protocols with variable length fields to name a few. – user694733 Oct 05 '17 at 07:28

2 Answers2

0

As Some programmer dude pointed out, it was to do with struct padding/packing. It seems by default the TI compiler I am using in Code Composer Studio pads the structs to align them with the 32-bit memory.

Fortunately, __attribute__((__packed__)) is supported by the compiler.

Simply changing the definition of PacketHeader solved the issue:

struct __attribute__((__packed__)) PacketHeader {
  uint32_t sourceAddress;
  uint8_t packetType;
};
Kureigu
  • 259
  • 4
  • 12
  • 1
    It is usually not a good idea to use structs to represent data protocols.There is a reason why the compiler uses padding though - it gives more effective access. – Lundin Sep 15 '17 at 13:59
0

Be careful with using "packed" structs - it is easy to get unaligned access which could be illegal or merely inefficient, depending on the target cpu details. Consider re-arranging your struct types to avoid the need of "packed".

When you are using structs for this sort of thing, I recommend using "-Wpadded" on the compiler. This will let the compiler tell you when there is padding added to a struct. If you expect padding, add it explicitly (such as "uint8_t dummy[3];").

Also use _Static_assert generously to check that your structs are the size you expect. Your aim is for mismatches to give compile-time errors, rather than to wait for testing and debugging.

David
  • 132
  • 3