-1
\$\begingroup\$

I'm packing several values from a struct into a buffer to send them over Bluetooth.

I don't want to send the whole struct and I also don't want to deal with padding, so I'm copying just the values I need. I also don't want to use anything that requires external tooling like Protobuf.

This is what I came up with. (Shortened to 6 values, the real code has a couple more.)

char buf[6 + 6 + 8 + 8 + 1 + 1];
size_t len;
size_t offset = 0;

memcpy(buf + offset, &g_system_status.gps_date, len = 6); offset += len;
memcpy(buf + offset, &g_system_status.gps_time, len = 6); offset += len;
memcpy(buf + offset, &g_system_status.gps_start_time, len = 8); offset += len;
memcpy(buf + offset, &g_system_status.gps_fix_time, len = 8); offset += len;
memcpy(buf + offset, &g_system_status.modem_creg, len = 1); offset += len;
memcpy(buf + offset, &g_system_status.modem_cgreg, len = 1); offset += len;

To a C developer (I just dabble) is it sufficiently clear what is happening here or is there a better way?

\$\endgroup\$
1
  • 2
    \$\begingroup\$ To a C developer, this looks like a bunch of code that is out of context, it isn't even a complete function. Please (re)read How do I ask a good question?. \$\endgroup\$ Commented Jul 11, 2022 at 15:00

1 Answer 1

Reset to default
2
\$\begingroup\$

Avoid naked magic numbers

Why 6? Is that the sizeof the object?

// char buf[6 + 6 + 8 + 8 + 1 + 1];
char buf[sizeof g_system_status.gps_date + sizeof g_system_status.gps_time
    + ...];

// memcpy(buf + offset, &g_system_status.gps_date, len = 6); offset += len;
memcpy(buf + offset, &g_system_status.gps_date, 
    len = sizeof g_system_status.gps_date); offset += len;

How about endian?

When sending numbers, useful to send/receive numbers in a know endian as the sender/receiver may use different endian.

// memcpy(buf + offset, &g_system_status.gps_start_time, len = 8); offset += len;
int64_t t = hton64(g_system_status.gps_start_time);
memcpy(buf + offset, &t, len = sizeof t); offset += len;
\$\endgroup\$
7
  • 1
    \$\begingroup\$ Good idea to use sizeof However, the serialization format has to stay fixed, so perhaps there should also be an assertion that the sizeof matches expectations. \$\endgroup\$ Commented Jul 9, 2022 at 18:57
  • \$\begingroup\$ g_system_status.gps_date for example is 7 bytes long, 6 bytes as they come in as NMEA and a null byte because I use them as C strings in other places. I could change that of course, not a big fan of C strings anyway. Good point about endianness, my platform (ARM) is little-endian, which is rather unintuitive. \$\endgroup\$ Commented Jul 9, 2022 at 19:27
  • \$\begingroup\$ @200_success IMHO serialization format best done (for integers) with (U)intN_t and friends. \$\endgroup\$ Commented Jul 9, 2022 at 20:27
  • \$\begingroup\$ @200_success While the answer is decent, how did you leave the question open? \$\endgroup\$ Commented Jul 11, 2022 at 15:02
  • \$\begingroup\$ @pacmaninbw Some reviews are of large amount of code. This one is small. Yet for OP who "dabbles" in C, looked (barely) sufficient. \$\endgroup\$ Commented Jul 11, 2022 at 16:32

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.