Parsing UART packets / state machines / packed structs

[uer166]

You cannot counter someone's argument by simply insinuating they are incompetent, ignoring "facts" and so on.

Unlike you, nctnico is highly competent and I deeply respect his competence. He just has a few weird "blind spots" where his opinions, which are just matter of taste which I could accept as opinions, like disliking structs in networking and preferring bit manipulation reconstruction, make him close his eyes from objective facts. I'm sure he still does excellent job.

I have offered all the relevant information, unlike you; in other words, please get out of our argument, your comments are completely unhelpful. We have enough trolls on this forum already. Thank you in advance.

I see through this Sir, when you speak it is to be regarded as "facts" as "objectively true" but the words of another (who disagrees with you) are merely "opinions" as the words of one who has "closed" their eyes, has "blind spots".

You are a brave man indeed, hiding behind a keyboard and unhesitatingly insulting me and my presumed lack of competence. Is that how you speak to coworkers who dare to disagree with you? Is that really the way you'd communicate in person? I doubt it, I bet you're as nice as pie under those circumstances, nobody who bullies as you do here, would last long in the workplace.

If you have nothing technical to add to the discussion, feel free to avoid the thread. I don't mind technical discourse but this is not useful.

[Siwastaja]

The above opened my eyes a little bit, since the first few bytes:
|0x55|0xAA|LENGTH|ADDRESS|0x01|CMD|

Are actually predictable and are all the same given a specific CMD. the CMD defines the length, and the ADDRESS is always a fixed number. This means I can easily sync on the first 6 bytes, which gives a nice 48-bit sequence. This would avoid the need for a 255-byte buffer and any re-sync should be pretty quick. This is kinda like having a CRC since the length and address is always a unique fixed pair in the messages that I care about.

One can use something like this:

Code: [Select]

typedef struct
{
   uint8_t id;
   uint8_t len;
   void (*handler)(void* msg);
} msg_meta_t;

#define MSG(id, prefix) {.id=(id), .len=sizeof (prefix ## _msg_t), .handler = prefix ## _handler}

static const msg_meta_t MSGS[] =
{
   MSG(42, measure_voltage),
   MSG(57, self_destruct)
};

Or, even better, make the id contiguous from 0 (not a sparse table) and use the meta table index directly as msgid.

But with such sparse table (current project of mine uses this approach because message type space is 16 bits wide and we may have many hundreds of message types eventually, where each product only implements subset of):

When receiving msgid, loop through the meta table:

Code: [Select]

for(int i=0; i<NUM_ELEM(MSGS); i++)

Now you get the length from the table. If you still prefer to send it over wire - not a bad idea, now you can do a sanity check if these two match. But if you trust yourself to keep both parties up-to-date, no need to transfer the len field.

When done, and CRC checks out, you just call the handler through the function pointer, giving them the other two of the buffers. (In this example, we used void* pointer that we cast back to the desired type. I know it is safe to do so because the original buffer type is an union with the type in it, but if this concerns you, feel free to make that meta function pointer another union, with all possible argument types, then you can pass the actual correct type without casting it.)

I prefer to utilize preprocessor copy-and-paste to
1) reduce typing the same things over and over again, reducing human errors when changing things
2) force to use unified naming scheme, like
self_destruct_handler()
which uses
self_destruct_msg_t
as payload input type.

This is not super-fancy and not super-high level, but it's some kind of sweet spot for a simple language like C.

[NorthGuy]

Yup, in fact I am in control for the request packets. I.e. I can expect silence until the time where I send out a formatted "give me cell voltages" or whatever. I can see your point about efficiency loss here being similar to overhead, maybe something like Consistent Overhead Byte Stuffing would help.

Syncing to 0xaa55, given that 0xaa55 may occur in the middle of the packet, complicates things, so try to avoid it.

If you ask for cell voltages, you know what are you going to receive. The length is probably fixed too. So, you do very simple thing - you send your request, and then you use DMA (or interrupt if you don't have DMA) to collect data into a buffer until you receive the needed number of bytes or until the operation times out. Then you parse the received data - verify the preamble, length, message type, checksum etc. You don't need BMS data very often, so you have plenty of time to do so. No need for a state machine or anything of that sort. Don't complicate things without a need.

[NorthGuy]

I already explained precisely why mapping structs on buffer is bad: not all platforms support it  AND it doesn't solve byte order (many protocols use big endian byte order while many CPUs nowadays are little endian). It is not much simpler than that.

What do you mean by "not all platforms support"? If platform supports pointers, then you certainly can interpret such pointers as pointers to structs. Do you have any example of a platform which wouldn't support it?

If a field contains a big endian (e.g. a port number in IP protocol) you can easily invert it, or you can keep all your port numbers in big endiam format. Why is that a big deal?

[SiliconWizard]

Any platform I've ever used in C from 8-bit to 64-bit had compilers allowing struct packing. The only problem to be aware of is again alignment, which may be a problem on some platforms for accessing certain ill-aligned struct members directly. That's the case in particular for the 16-bit PIC MCUs for accessing 16-bit or 32-bit struct members. They need to be aligned.

As to endianness, yeah it's not a big deal. If you make your own protocols don't bother with big-endian as it's pretty rare nowadays on processors. Buf if you have to deal with it, you can just swap bytes "in place" as NorthGuy said, for the members that require it. Still usually eons simpler, faster and more readable than hand-decoding packets byte per byte.

The real potential issue is the alignment, and that's something you have to consider for the target you're programming for. And for the protocol you're designing as well, unless you have no choice and have to use an existing one. Otherwise, defining your data packets to have "fields" that are all naturally aligned will pretty much cover all targets, and it's not rocket science.

[Nominal Animal]

If you want to use function pointers, then a pattern where each character received is handled by a function which returns the pointer to the function that handles the next character, should work:

Code: [Select]

#include <stdint.h>
#include <stddef.h>

// Known states:
static void *wait_55(uint8_t rx);
static void *wait_AA(uint8_t rx);
static void *wait_len(uint8_t rx);
static void *wait_addr(uint8_t rx);
static void *wait_01(uint8_t rx);
static void *wait_cmd(uint8_t rx);
static void *wait_data(uint8_t rx);
static void *wait_check(uint8_t rx);
static void *wait_skip(uint8_t rx);
static void *wait_skipcheck(uint8_t rx);
static void *in_error(uint8_t rx);

// Handler for the next incoming byte
void *      (*BMS_UART_State)(uint8_t) = wait_55;

// Handler for the currently received command (unused if skipped)
void *      (*BMS_UART_Handler)(void) = NULL;

uint_fast16_t BMS_UART_Checksum = 0;
uint_fast16_t BMS_UART_Datasum = 0;
uint8_t      BMS_UART_Data[255];  // Or whatever is the longest supported command
uint8_t      BMS_UART_Have = 0;
uint8_t      BMS_UART_Length = 0;
uint8_t      BMS_UART_Address = 0;
uint8_t      BMS_UART_Command = 0;

uint8_t      BMS_UART_Error = 0;
enum {
    BMS_UART_ERROR_NO_AA = 1,
    BMS_UART_ERROR_NO_01,
    BMS_UART_ERROR_CHECKSUM,
};

// State machine, in all its glory
void BMS_UART_StateMachine_Rx(uint8_t rx)
{
    BMS_UART_State = BMS_UART_State(rx);
}

// Implementation of individual states
static void *wait_55(uint8_t rx)
{
    if (rx == 0x55)
        return wait_AA;
    else
        return wait_55;
}

static void *wait_AA(uint8_t rx)
{
    if (rx == 0xAA)
        return wait_len;
    else {
        BMS_UART_Error = BMS_UART_ERROR_NO_AA;
        return in_error;
    }
}

static void *wait_len(uint8_t rx)
{
    BMS_UART_Length = rx;
    return wait_addr;
}

static void *wait_addr(uint8_t rx)
{
    BMS_UART_Address = rx;
    return wait_01;
}

static void *wait_01(uint8_t rx)
{
    if (rx == 0x01) {
        return wait_cmd;
    } else {
        BMS_UART_Error = BMS_UART_ERROR_NO_01;
        return in_error;
    }
}

static void *wait_cmd(uint8_t rx)
{
    BMS_UART_Command = rx;
    BMS_UART_Datasum = BMS_UART_Length + 0x26 + BMS_UART_Command;
    BMS_UART_Checksum = 0;
    BMS_UART_Have = 0;

    if (BMS_UART_Length == 0xCA && BMS_UART_Address == 0xFE && BMS_UART_Command == 0x64) {
        // Command 0x64, address 0xFE, length 0xCA is handled by function NULL:
        BMS_UART_Handler = NULL;
        return wait_data;

    } else
    if (BMS_UART_Length == 0xB0 && BMS_UART_Address == 0x0B && BMS_UART_Command == 0x1E) {
        // Command 0x1E, address 0x0B, length 0xB0 is handled by function NULL:
        BMS_UART_Handler = NULL;
        return wait_data;

    } else {
        // Everything else is skipped.
        return wait_skip;
    }
}

static void *wait_skip(uint8_t rx)
{
    if (BMS_UART_Have < BMS_UART_Length) {
        BMS_UART_Datasum += rx;
        BMS_UART_Have++;
        return wait_skip;
    } else {
        BMS_UART_Checksum = rx;
        BMS_UART_Datasum = 0xFFFF - BMS_UART_Datasum;
        return wait_skipcheck;
    }
}

static void *wait_skipcheck(uint8_t rx)
{
    BMS_UART_Checksum = (BMS_UART_Checksum << 8) | rx;
    if (BMS_UART_Checksum == BMS_UART_Datasum) {
        // No error, but we ignored this command.
        return wait_55;
    } else {
        BMS_UART_Error = BMS_UART_ERROR_CHECKSUM;
        return in_error;
    }
}

static void *wait_data(uint8_t rx)
{
    if (BMS_UART_Have < BMS_UART_Length) {
        BMS_UART_Datasum += rx;
        BMS_UART_Data[BMS_UART_Have++] = rx;
        return wait_data;
    } else {
        BMS_UART_Checksum = rx;
        BMS_UART_Datasum = 0xFFFF - BMS_UART_Datasum;
        return wait_check;
    }
}

static void *wait_check(uint8_t rx)
{
    BMS_UART_Checksum = (BMS_UART_Checksum << 8) | rx;
    if (BMS_UART_Checksum == BMS_UART_Datasum) {
        if (BMS_UART_Handler)
            BMS_UART_Handler();
        return wait_data;
    } else {
        BMS_UART_Error = BMS_UART_ERROR_CHECKSUM;
        return in_error;
    }
}

// Error state
static void *in_error(uint8_t rx)
{
    if (rx == 0x55) {
        return wait_AA;
    } else {
        return in_error;
    }
}

Take a note at the wait_cmd() function.  In it, you detect if the command, address, and length is something you are interested in, and if so, assign a handler function to be called when it is received with a matching checksum.  Otherwise, the data is ignored.

I'm not sure I like this over the switch (state) { ... } idiom, though.

The key feature of this state machine is that it detects errors, and has a dedicated error state (from which it will continue when a valid 0x55 0xAA sequence is received).  And, of course, that it does not store all data, only data from commands you want to support, while still handling all commands of the stated form correctly (and even checking their checksum).
Obviously, you do not need to use function pointers to implement this exact state machine.

[Siwastaja]

Any platform I've ever used in C from 8-bit to 64-bit had compilers allowing struct packing. The only problem to be aware of is again alignment, which may be a problem on some platforms for accessing certain ill-aligned struct members directly. That's the case in particular for the 16-bit PIC MCUs for accessing 16-bit or 32-bit struct members. They need to be aligned.

But, once again, with the packed attribute, compiler uses correct instructions to access unaligned 16- or 32-bit member; e.g. 8-bit loads and shifting + orring to combine the result into a register. Just like what you do manually in C by writing val = (uint32_t)in[1]<<24 | ... and so on. This is obviously slower than aligned loads, and the reason not to pack structs "just because".

It only becomes a problem if pointer to a member is used, but then the compiler will warn you, as I found out, even with the minimal default warnings on GCC. Casting the pointer to the whole packed struct is fine because compiler knows the struct is packed and does not assume any alignment.

But, when unsure, use unions instead of pointer casting. Then the language itself guarantees alignment, and nothing can go wrong. By thinking about your datatypes and all their intended uses, you get almost completely rid of casting buffers into something else. Especially if you don't have to deal with libraries.

[westfw]

the language itself guarantees alignment

While I'm a big fan of mapping packets unto structures, it is possible to end up with the start of your packet at an unaligned address due to encapsulation or options or whatever coming before it.  In which case you may need to use accessor functions/macros that are not more complicated than using similar functions all the time, but (IMO) more readable.
"src = GETLONG(pak->ipsrcaddr)" vs "src = get_ip_srcaddr(pak)"

OTOH, there's IP's "Fragment Offset", which is substantially inconvenient on little-endian CPUs. :-)

[Nominal Animal]

"src = GETLONG(pak->ipsrcaddr)" vs "src = get_ip_srcaddr(pak)"

Why not

Code: [Select]

// Generic accessors
static inline uint_fast16_t  get_u16be(const unsigned char *src)
{
    return ((uint_fast16_t)(src[0]) << 8) | src[1];
}

static inline uint_fast32_t  get_u32be(const unsigned char *src)
{
    return ((uint_fast32_t)(src[0]) << 24)
         | ((uint_fast32_t)(src[1]) << 16)
         | ((uint_fast32_t)(src[2]) << 8)
         | src[3];
}

// Dedicated accessors
static inline uint32_t  get_ipv4_length(const void *gram)
{
    return get_u16be((unsigned char *)gram + 2);
}

static inline uint32_t  get_ipv4_srcaddr(const void *gram)
{
    return get_u32be((unsigned char *)gram + 24);
}

static inline uint32_t  get_ipv4_dstaddr(const void *gram)
{
    return get_u32be((unsigned char *)gram + 28);

}where the inline is just a reminder to us humans that these are accessor functions intended to be put into a header file?

The generic accessor is named according to the data type, whereas the dedicated accessor is named after the logical field, and internally "encodes" the size, type, and offset of that field.

On x86-64, where unaligned accesses are supported, get_ipv4_srcaddr() compiles to just one mov and a bswap.  On ARM Thumb, to one ldr and a rev.  On RISC-V, it generates a bit more code, but no more than any unaligned 32-bit load would.

There is only a small conceptual jump into memory-safe accessor primitives from here, too.

[nctnico]

Any platform I've ever used in C from 8-bit to 64-bit had compilers allowing struct packing. The only problem to be aware of is again alignment, which may be a problem on some platforms for accessing certain ill-aligned struct members directly. That's the case in particular for the 16-bit PIC MCUs for accessing 16-bit or 32-bit struct members. They need to be aligned.

And ARM platforms have a habit of needing alignment as well. I have run into the situation where an Atmel SOC would silently corrupt unaligned data. And this was due to mapping a type directly onto the buffer.

As to endianness, yeah it's not a big deal. If you make your own protocols don't bother with big-endian as it's pretty rare nowadays on processors. Buf if you have to deal with it, you can just swap bytes "in place" as NorthGuy said, for the members that require it. Still usually eons simpler, faster and more readable than hand-decoding packets byte per byte.

I never suggested hand decoding packet byte-per-byte. Use get / set routines for specific field types that work with offsets. In my experience that is much more practicle and less error prone than putting a struct together because -again- fields in a protocol are typically specified with an offset and size. These get / set routines can take care of byte swapping which then wraps up everything that deals with hardware at the lowest level possible. IMHO is it utterly nuts that the BSD socket API requires the application layer to deal with endianess. That should have been handled at a much lower level.

Also keep in mind some protocols may have messages that have different fields depending on the message type so one struct typically isn't enough. Another advantage of not mapping structs is that you can use a window that moves over the incoming data without needing to care about alignment. Especially with protocols that come in over continuous streams (like UART and TCP/IP) you don't know the start & end of a packet; it makes more sense to use the receive buffer to examine the data 'in situ' rather than copying something that could be a packet but may be garbage.

This may be drivel to some but I have implemented a crapload of protocols over the years and know very well what works and what doesn't.

[NorthGuy]

Another advantage of not mapping structs is that you can use a window that moves over the incoming data without needing to care about alignment. Especially with protocols that come in over continuous streams (like UART and TCP/IP) you don't know the start & end of a packet; it makes more sense to use the receive buffer to examine the data 'in situ' rather than copying something that could be a packet but may be garbage.

That is because you have used an inappropriate buffering method. If you use a circular buffer then everything gets misaligned and the buffer may wrap at any point, so the data can only be accessed byte by byte. Accessor functions create an illusion of structural approach, but do not solve the problem - they only add bloat to hide the ugliness.

Instead, create a number of aligned buffers (one, two, or many) where you can collect the "would-be-packets" from UART, then, when the packet is confirmed, pass the pointer to the buffer further for processing. Once the contents of the packet is identified pass a pointer the specific data structure within the packet to analyze the contents.

[Siwastaja]

Any platform I've ever used in C from 8-bit to 64-bit had compilers allowing struct packing. The only problem to be aware of is again alignment, which may be a problem on some platforms for accessing certain ill-aligned struct members directly. That's the case in particular for the 16-bit PIC MCUs for accessing 16-bit or 32-bit struct members. They need to be aligned.

And ARM platforms have a habit of needing alignment as well. I have run into the situation where an Atmel SOC would silently corrupt unaligned data. And this was due to mapping a type directly onto the buffer.

... which (the type) clearly was not a packed struct.

The title here says: "packed structs".

Case closed. Can we now go forward or do we need to continue this broken record game?

Also keep in mind some protocols may have messages that have different fields depending on the message type so one struct typically isn't enough

Which is why I suggested unions.

it makes more sense to use the receive buffer to examine the data 'in situ' rather than copying something that could be a packet but may be garbage.

Which is why I suggested instead of copying, using an union so that the same buffer type can be accessed as the struct.

This may be drivel to some but I have implemented a crapload of protocols over the years and know very well what works and what doesn't.

Clearly you have. Sometimes it is a good idea to look at new, modern ideas that improve the code. But if you refuse to even read the messages you reply to, and instead build a strawman against any idea except of your own, then we can do nothing to help.

[nctnico]

Combining a struct in a union won't help if the buffer itself isn't aligned (for example when moving a 'window' over a buffer). And mapping a type or struct onto a buffer is the same thing. It will break when alignment is required. It is that simple. And this has nothing to do with being 'modern' or something like that (C compilers have been supporting packed structs since the dawn of time) but it is all about creating robust, portable code that works as expected no matter what. Even in the hands of less experienced engineers.

Actually, you have to think about whether packed structs have been designed to be used on dealing with external protocols. Saving memory at the expense of speeds seems like a very reasonable explaination to me. Especially since you can also pack bitfields.

[Siwastaja]

Portability no matter what? Sure, that's a valid reason, but it's not the only one.

If you want portability so that the MCU could one day be replaced with a big endian MCU, without code needing to be modified at all, then sure, write wrappers with both endianness options from day one. You need to do all the same work, just proactively.

(I have actually encountered one big endian machine lately, namely Teltonika RUT955 embedded PC. Interfacing it with ARM microcontrollers required taking care of endianness on one of the ends - the Teltonika. I still used structs, and no, there was nothing magical involved.)

These are design choices to make. In my MCU projects, these are not factors. Code which I write for Cortex-M4 for example, relies on some other Cortex-M features such as interrupt handlers being regular functions. I also rely on it staying little endian - and I also rely on using a compiler with packed struct available. All ARM CM capable compilers are such, have always been. I think it's a safe assumption no one wants to suddenly compile my commercial projects on some specialized scientific or toy compiler.

Life is such choices. And it has absolutely nothing to do with "robustness". A train can be robustly designed and built even if it needs certain width of tracks and cannot fly. These are limitations, and not some arbitrary limitations, but well thought out ones. Every project has those.

And I happen to be 110% sure the uer166 guy:
* will always use a compiler with packed attribute
* will only use little endian CPUs in his BMS communication

Worst case? When the unimaginable happens and uer166 directly connects his BMS to Teltonika router, then add endianness wrappers. Same work, but Just-In-Time.

[NorthGuy]

You usually don't need to use packed structs. Most of the protocols will use struct fields which are naturally aligned. There may be padding added to ensure this, which you explicitly add to your struct where needed. Thus, once you ensure that the beginning of the struct is aligned, the rest of the fields get aligned automatically.

[Siwastaja]

You usually don't need to use packed structs. Most of the protocols will use struct fields which are naturally aligned. There may be padding added to ensure this, which you explicitly add to your struct where needed. Thus, once you ensure that the beginning of the struct is aligned, the rest of the fields get aligned automatically.

Well designed protocols do this for performance. You need to remember the ages old good trick, which is still useful: when the order of fields does not matter for you (and usually it does not), just order the fields from biggest to smallest (e.g. floats go first, chars last), for least amount of padding.

Then, for protocols, you can add padding yourself. The advantage over compiler-generated padding is also that these padding fields become natural "reserved" fields which allows you to extend the protocol later.

But I'm sure this is somehow wrong, too, definitely not professional and robust

[Sherlock Holmes]

You usually don't need to use packed structs. Most of the protocols will use struct fields which are naturally aligned. There may be padding added to ensure this, which you explicitly add to your struct where needed. Thus, once you ensure that the beginning of the struct is aligned, the rest of the fields get aligned automatically.

Well designed protocols do this for performance. You need to remember the ages old good trick, which is still useful: when the order of fields does not matter for you (and usually it does not), just order the fields from biggest to smallest (e.g. floats go first, chars last), for least amount of padding.

Then, for protocols, you can add padding yourself. The advantage over compiler-generated padding is also that these padding fields become natural "reserved" fields which allows you to extend the protocol later.

But I'm sure this is somehow wrong, too, definitely not professional and robust

Perhaps such field ordering could be done with greater flexibility, I agree that the "largest" first is a sensible strategy sometimes but .Net does that automatically, one has to take special steps to order them oneself.

[SiliconWizard]

Any platform I've ever used in C from 8-bit to 64-bit had compilers allowing struct packing. The only problem to be aware of is again alignment, which may be a problem on some platforms for accessing certain ill-aligned struct members directly. That's the case in particular for the 16-bit PIC MCUs for accessing 16-bit or 32-bit struct members. They need to be aligned.

But, once again, with the packed attribute, compiler uses correct instructions to access unaligned 16- or 32-bit member; e.g. 8-bit loads and shifting + orring to combine the result into a register. Just like what you do manually in C by writing val = (uint32_t)in[1]<<24 | ... and so on. This is obviously slower than aligned loads, and the reason not to pack structs "just because".

I have certainly run into compilers (xc16 here, don't remember the version, but it's based on GCC although the port is handled by Microchip and never made it to mainline GCC, so it's possibly very not on par with what GCC is supposed to be doing) that wouldn't automatically handle unaligned access in the way you describe, and accessing unaligned struct members would crash the CPU.

Given that the "packed" attribute AFAIK is compiler-specific and not standard C, you're of course not guaranteed any of this anyway. Let me know if this has made it to the standard though. Maybe C23, as I think they finally introduced attributes?

That said, I checked on godbolt using Clang and GCC and various targets and your statement seems to hold, so I'm gonna assume that it should be safe if you use a reasonably recent version with a reasonably well supported target. I'm again not too sure about 16-bit PICs here, but xc16 is kind of a nasty beast.

[Nominal Animal]

I use both (packed) structures and (character) buffer accessors.

The structures work really well with fixed structures, like say PNG files.  When the actual layout depends on the content, the buffer accessors work much better than trying to use structures.  IPv4 datagrams are on the borderline, because the first 20 bytes of the header are fixed, and only options may vary; so I might use a packed structure (as all C compilers I use do support those) for the first 20 bytes, but accessors for the options.

I do not understand the argument here, because in my opinion, the two do not overlap that much.

I use packed structures with fixed data structures when I have ensured they work and documented the need for them to work, and buffer accessors for varying data structures and when packed structures may not be available.

Even when using structures with associated data, I tend to use the struct fields through accessors when the field deals with things like data length to ensure memory safety: that through normal use of the interface, one will not accidentally access past the known/received data, because the content happens to be buggy or malicious.

This checking is important.  The recent CVE-2022-41674 is a good example.  Code did not check that the MaxBSSID field in a WiFi packet had a sensible value (between 1 and 8, inclusive), so a buffer overrun ensued.  Oopsie.

Thus, even when using a packed struct, you should never trust its contents to be safe; and to make sure, we use helper accessors to do the checking for us.  (Well, not for pure data fields, those that just state some fact, but do not affect the layout or access to other data.)

In other words, the one wrong use case of packed structs is to avoid using accessor functions.  The two are not mutually exclusive.


For worry-free binary data stream protocols, I use prototype binary representations of known values as both the identifier as well as byte order indicator, so that the data generator does not need to care about the byte order at all.  On the host computer, the datastream can be trivially 'normalized' to local-endian, or kept as-is.  To detect the byte order, one simply tests the byte order permutations (including 'none'/'as-is') that yield a recognized value.  For example, for IEEE 754 Binary32 'float' type, one can use 0x3F2C1880 = 0.67224884f.

For structures, on a computer, the "packed" datastream can easily be expanded into a stream/array of native structures.  It is an extra copy, yes, but it turns out that due to the cache behaviour of current machines, doing it as soon as the data is read or received, is actually very, very cheap in terms of CPU or wall time used, because on typical computers we tend to be memory bandwidth limited, and not at all CPU limited.  Even in Python (when interfacing to microcontrollers) this makes sense –– for example, one should definitely convert an incoming bytes stream into NumPy.arrays via struct.unpack() for manipulation.  (On the prototype part, just try the different byte order prefixes on the struct formats, until the values match the expected, and then instantiate that using struct.Struct for mass unpacking.)

I really like the prototype approach, because not only does a single value (of each type used) ensure byte order, but they also act like protocol and format identifiers, 'magic pattern', in the Unix file command magic sense.

[Siwastaja]

In other words, the one wrong use case of packed structs is to avoid using accessor functions.

One could also claim that having one accessor function per member for validation is not always the best strategy. Sometimes validity is in combination of variables, for example: assert(a<10 && b<10 && (a+b)<15).

I like to do the sanity check of the len field in the protocol level handler (which is agnostic to the payloads), but when len and CRC check out and thus non-corrupted data is received (leaving logical mistakes / bugs at the sender), I use a handler function which gets the whole payload type struct, and verifies the sanity of data fields "as it goes", in the same way one usually checks function input arguments.

In fact, I don't think there is any fundamental difference in verifying the input values in a "normal" function in a program, or input values coming in a struct that was received through UART.

But as is obvious to most of sensible programmers, different paradigms work for different problems.

[Nominal Animal]

In other words, the one wrong use case of packed structs is to avoid using accessor functions.

One could also claim that having one accessor function per member for validation is not always the best strategy. Sometimes validity is in combination of variables, for example: assert(a<10 && b<10 && (a+b)<15).

I agree.  It is equally valid to just verify all the fields at some point.

In my own code, when the structures are accessed randomly –– so, a tree or other structured data thingy –– I sometimes unpack and validate the structures at reception/reading, because that tends to have the best cache footprint and I/O thoroughput.  Idea being, since we touch the data anyway, we could make it cheaper to access.

Similarly, when receiving data via an UART, I prefer methods that update checksums and calculate command hashes on the fly, online.

In fact, I don't think there is any fundamental difference in verifying the input values in a "normal" function in a program, or input values coming in a struct that was received through UART.

So much of the code I write is centered around working with structured data, that I find the idea that verifying input could be considered somehow special or different, completely bonkers.

I like to think of all data suspicious, because that way any oddity or issue in processing that cannot be compensated becomes an issue for the user to deal with.  I absolutely hate programs that silently corrupt data, because "those errors occur so rarely it is not worth it to check for them", because with my luck, it is my most precious data that gets silently corrupted.  No, I want programs to do their best, but also tell me if something odd or unexpected happened.

In an embedded appliance, especially microcontrollers, it can be a difficult user interface question to decide how to handle communications errors.  But, if you start with "I'll worry about that later", then you basically decide to handle them randomly, because that kind of functionality is a design issue, and not something that can be bolted on top later on.  It has never been succesfully fixed later on, only semi-patched with ugly kludges.
Security is like that: it is either designed in, or it does not exist.

[SiliconWizard]

Key advice.
Validate your inputs.

A gigantic number of bugs, crashes (that can lead to death) and security exploits have come, come and will come from not validating inputs.

Even a very large number of memory-related bugs are linked to not properly validated inputs. That's the case for - I'd dare say - most buffer overflows.

Yet it looks like most quality-oriented discussions and programming language features these days obsess over memory access and management, and almost nothing about input validation (and more generally invariant checks, and "contracts".) That's probably because we tend to focus on adding safety nets rather than on prevention, and that doesn't just apply to software.

[westfw]

Validate your inputs.

Yeah.  Once upon a time, I wrote one of the first implementations of IP option support.  It was pretty neat; you could put "record route" options in ping packets and get interesting data, and etc.

It was "done" just before an "InterOp" trade show.  So we took it to the show and loosed it upon the "show network" (you could get away with things like that in those days.)  Broadcast pings with options.  AFAIK, everything we sent out was correct.  But other systems all over crashed and burned.   I hadn't done much to validate things coming in, so some of the other system tried to respond, and sent out ... really horribly formatted attempts.  We crashed too!

It was a very educational experience for a lot of people/companies!  Me included, I think.

[Nominal Animal]

All that I describe below has quite naturally lead me to where I am now, and to say:

Validate your inputs.
Make sure you understand the limitations of the processing or calculations.
Present the results in an effective, useful form.
Let the user know if anything unexpected happens.
Document your intent and purpose for your code in detail.


I started doing web backend stuff very early in the era, before the turn of the century.
I had web clients that did not care about accept-charset, and simply sent data using whatever encoding they liked.
I had to use hidden input fields to detect the character set (codepage 437, codepage 1252, ISO 8859-1, ISO 8859-15, MacRoman, early UTF-8).

Since the first Microsoft web servers were vulnerable to path exploits –– ../../../../../../system32/cmd.exe "escaping" the web root, and referring to the command-line interpreter ––, I saw lots of these in my server logs.  I also found out that many users had difficulties remembering the difference between uppercase and lowercase, and complaining about case sensitivity in URL path components.
So, early on, I did my own URL path and query string verification too: removed any illegal sequences, replaced common non-ASCII characters with their closest ASCII equivalents, lowercased it all, and removed all but alphanumerics (including dashes not at the beginning or end) between slashes.
It, too, had a twofold reason: one was security, the other helped users if they accidentally used say /pähkähullu/ instead of /pahkahullu/.

Not verifying the data simply would have never worked at all.

Later on, when I got back to Uni and started doing computational physics (ie. computer simulations, mostly molecular dynamics simulations), I soon learned the most important rule in simulation at the visceral level: you first check if it makes any sense.  If the total energy in an isolated system changes, your simulation is faulty, and so on.

My favourite early failure in simulation was with Morse copper (Morse potential fitted roughly to copper), where I investigated the limit at which a system composed of perfect cubic lattice, one half of which was at absolute zero, and the other half had very high random (Boltzmann-distributed) kinetic vectors, would end up melted; with the system being periodic and infinite.  My error was to join the two parts so that atoms at the interface were much closer than normal in the lattice.  The effects I observed from this specific type of "superheating" were really odd.  I saw everything from acoustic waves with wavelength much longer than my system, to a molten droplet encased in solid Morse copper, apparently shielded by the Leidenfrost effect.

That taught me that even tiny errors in the inputs could make the outputs completely unphysical and irrelevant (albeit interesting).

Around the same time, I also started working on sonification and visualization, the former helping me focus on what is important and useful (in that case, when monitoring a running simulation), and the latter on how to control the information conveyed, to convey the most important data to the viewer in an intuitive manner.  Showing atoms as gleaming marbles is pretty, but the intuitive associations and extrapolations such images generate are completely wrong.
For example, "atoms colliding" sounds like a trivial, well defined phenomenon.  Except it isn't, when you actually investigate say ion implantation in solid matter, and try to find when "atoms collide".  You can say that the ion collides, yes, but as to the solid matter, the atoms there are in continuous interaction and movement (due to finite temperature; even at room temperature the movement is significant), so the macro-scale concept of "collision" has no useful meaning or relevance: they all "collide" with each other, all the time, exchanging energy.

That taught me intuition is useful, but easily mistaken; and most importantly, aimable/directable.  So, I learned to be suspicious of my own assumptions and intuition, and shifted my learning more towards understanding (in an intuitive manner) the underlying concepts correctly, rather than just applying existing knowledge to solve a specific problem.

(I've left out my endeavours related to the demoscene, art, multimedia, et cetera, so the above is a simplified storyline, picking only the major plot-relevant points here.  I do consider myself lucky for having such a winding path, because every step in a different domain has taught me something useful and applicable here.)

All this has taught me to consider the inputs with suspicion; consider the limits and errors inherent in the processing; and how to present the results in an effective and useful manner.  And when privileged information is at stake, the privileges needed at each step, and how to stop information leaking (including backwards in the above chain).  It all starts at the conceptual level, at the core design level.

Of course, I'm no "ideas man, leaving the details for the lesser mortals to implement".  At the core design level, I constantly bump into details and problems I haven't worked on before, so I create practical test cases –– unit tests, in the form of complete executable programs or scripts; sometimes brute-force searching for the 'optimal' solution in the solution phase space –– to find out.  I nowadays write these for even ideas I have, just to work out if that idea would work in practice.  I quickly learned to file them away well: I use a separate directory for each test case, with a README, sources, and an executable, that when run without any command-line parameters tells exactly what it does.  This way I can do a quick find or grep to find an old test case based on keywords I may recall, or at least limit the subset to check (by running the test cases) to a dozen or less, and a quick review of the README followed by the source code tells me all pertinent information.

And that is also the source of why I always remind new programmers to write comments that describe their intent, and not what the code does.  We/I can always read the code to see what it does, but lacking crosstime telepathy, I cannot divine whether that was the intent of the programmer or not.
I myself had to learn to do this later on, after already being adept (paid to write code) in several programming languages, so it is very hard for me: even now, I still slip into the useless type of comments, unless I'm careful.

[nctnico]

And that is also the source of why I always remind new programmers to write comments that describe their intent, and not what the code does.  We/I can always read the code to see what it does, but lacking crosstime telepathy, I cannot divine whether that was the intent of the programmer or not.

I agree here. I deeply loath 'documentation' made using Doxygen because the output just lacks the one important thing: why is the code the way it is and what are the relations of functions? On top of that, any decent IDE can produce an overview of functions realtime; I don't need Doxycrap for that.