Should one be using macros instead of flexible array members?

Question

Let's assume the following:

I'd like to create a structure for UDP packets. Each frame usually consists of an Ethernet Header, an IP header, a UDP header and an optional payload followed by, finally, the FCS (Frame Checksum Sequence).
The payload length is unknown/flexible. Meaning that when creating the struct, the payload would have to be the last member of it (flexible array member). Consequently, there's no place for the FCS.

So I thought about what possibilities would remain.

I came up with the following piece of code:

#define UDP_PKT(name, payload_length) struct __attribute((__packed__))      \
    {                                                                       \
        struct ether_header eth;                                            \
        struct ip iph;                                                      \
        struct udphdr udph;                                                 \
        unsigned char payload[payload_length];                              \
        u_int32_t fcs;                                                      \
    } name;

As this is not allowed:

struct __attribute__((__packed__)) udp_packet
{
    struct ether_header eth;
    struct ip iph;
    struct udphdr udph;
    unsigned char payload[]; // fam, must always be the last member
    u_int32_t fcs;
};

My question: Is that the only possibility I have to include the FCS in the structure without having a fixed array (payload) size?

If so, is that a good solution? Is that considered good practice?

Relying on the structure memory layout for serialization is not a good practice by itself. — Eugene Sh., Dec 14 '18 at 20:05
Payload length is fixed at compile time, right? Otherwise your macro solution wouldn't work. — Sergey Kalinichenko, Dec 14 '18 at 20:05
It's actually fixed at runtime. I thought I could call `UDP_PKT(pkt, 22)` to get an instance of the struct. — j3141592653589793238, Dec 14 '18 at 20:06
@T.Meyer Then your macro wouldn't work in standard C, because VLAs cannot be members of a structure or a union. — Sergey Kalinichenko, Dec 14 '18 at 20:09

score 4 · Accepted Answer · answered Dec 14 '18 at 20:15

4

The size of a struct with flexible array member is determined at runtime, so your first approach would not work either. The solution is to place FCS at the end of the buffer when you are ready to serialize your struct for the wire:

struct __attribute__((__packed__)) udp_packet {
    struct ether_header eth;
    struct ip iph;
    struct udphdr udph;
    u_int32_t fcs;
    unsigned char payload[]; // Move fcs up
};

void serialize_udp_packet(const udp_packet* p) {
    // Compute buffer size, and allocate the buffer
    // Place the header into the buffer
    // Copy the payload into the buffer
    // Place FCS into the buffer at the end
}

You could even exclude fcs from the udp_packet altogether, and compute it only when you serialize the struct. One advantage of this approach is that you could freely mutate the payload without having to sync up the FCS to the changed payload all the time.

answered Dec 14 '18 at 20:15

Sergey Kalinichenko

714,442
84
1,110
1,523

Waste od resouces. Especially in the embedded systems – 0___________ Dec 14 '18 at 20:21
@P__J__ Could you be more specific of what "resources" you suggest would be "wasted"? – Sergey Kalinichenko Dec 14 '18 at 20:22
Memory. Is it specific enough? Allocating two buffers just to move one object is an overkill – 0___________ Dec 14 '18 at 20:25
Looks great, but why isn't the size already fixed at compile time in my example? – j3141592653589793238 Dec 14 '18 at 20:29
2

@P__J__ Although it's more specific than before, it isn't specific enough about the waste. This solution suggests allocating exactly as much memory as needed. You can't avoid allocating the temporary buffer for serialization, unless you are willing to put system-dependent system of bytes on the wire. – Sergey Kalinichenko Dec 14 '18 at 20:30
@T.Meyer I think you didn't fully understand what happens in `serialize_udp_packet`. It's not simply `memcpy`-ing the `struct` into the buffer, it goes through the struct in the order that matches the desired order of items in memory. This is when [`ntoh`/`hton` etc.](https://stackoverflow.com/q/21996794/335858) functions come into play. – Sergey Kalinichenko Dec 14 '18 at 20:33
No, I think I got that part; however, I'm wondering why the UDP_PKT macro shall not work as intended. Isn't the `payload_length` parameter already fixed at compile time? (I'm not talking about the FAM, but about the VLA). Because I'm compiling without errors. @dasblinkenlight – j3141592653589793238 Dec 14 '18 at 20:36
@T.Meyer Ah, I see, your question is about my comment, not the answer. The reason your macro compiles fine is that it is using a popular compiler extension, which is, however, not standard. With this extension the solution [compiles](https://ideone.com/aFO9R1); with the compiler extension turned off, [you get an error](https://ideone.com/tOarSY). – Sergey Kalinichenko Dec 14 '18 at 20:39
Yes, sorry for the confusion, I probably should have put my comment below your comment to my question. Ah, ok, that makes sense now. It compiles with GNU99 standards, but not with C99 standard. Thanks – j3141592653589793238 Dec 14 '18 at 20:44
@dasblinkenlight Just tried my code with C99(https://ideone.com/uE52MJ); this seems to compile fine. Does that make sense? – j3141592653589793238 Dec 14 '18 at 23:18
1

@T.Meyer Yes, because you pass 5 to the macro as a constant. Make a variable for it to see the error. – Sergey Kalinichenko Dec 14 '18 at 23:25
True. Things are just too obvious sometimes. I appreciate your patience – j3141592653589793238 Dec 14 '18 at 23:32

score 0 · Answer 2 · answered Dec 14 '18 at 20:23

0

Allocate memory for the payload and the checksum. Use pointer to access the checksum. Easy and efficient

answered Dec 14 '18 at 20:23

0___________

60,014
4
34
74

Should one be using macros instead of flexible array members?

2 Answers2