Packed Union

[iotexpert]

typedef union
{
/// Provides access via field names
struct fields
{
uint8_t family[1]; ///< family identifier (1 byte, LSB - read/write first)
uint8_t serial_number[6]; ///< serial number (6 bytes)
uint8_t crc[1]; ///< CRC check byte (1 byte, MSB - read/write last)
} fields; ///< Provides access via field names

uint8_t bytes[8];              ///< Provides raw byte access

} OneWireBus_ROMCode;

Should probably be

typedef union
{
/// Provides access via field names
struct fields
{
uint8_t family[1]; ///< family identifier (1 byte, LSB - read/write first)
uint8_t serial_number[6]; ///< serial number (6 bytes)
uint8_t crc[1]; ///< CRC check byte (1 byte, MSB - read/write last)
} __PACKED fields; ///< Provides access via field names

uint8_t bytes[8];              ///< Provides raw byte access

} OneWireBus_ROMCode;

I don't think that there is a guarantee that compiler will align the family, seri... to bytes.

[DavidAntliff]

That's an interesting point, and you may be right, I'm not sure.

From what I can tell from reading articles such as http://www.catb.org/esr/structure-packing/#_structure_alignment_and_padding, the alignment is (usually? always?) taken from the smallest field, in this case a uint8_t (byte), or 1 byte, so no padding occurs, and it would be likely to be ok, but I'm not able to find out definitively across all C compilers. Unfortunately the __attribute((packed)) tag is non-standard, and I'm not sure where __PACKED is defined (is that in the IDF?).

In C at least, the first element of the struct (and the union) is guaranteed to be at offset zero, and the union array is guaranteed to be contiguous. ESR seems to think that if you follow his guidelines then you don't need to use implementation-specific declarations or tags.

It's also complicated by a potential difference in behaviour when compiled with a C++ compiler, using #pragma pack, and again this is implementation-defined. Also the struct doesn't have to start at offset zero either.

Can you offer an alternative representation of this data that is more portable?

[csobrinho]

Hi David, what about using an union of 8 bytes and the three fields totalling 8 bytes?

[DavidAntliff]

@csobrinho hmm, I don't quite follow - can you suggest some example code, please?

For future reference, here's the original code concerned:

esp32-owb/include/owb.h

Line 82 in 6f90bcd

typedef union

I'm still on the fence about this whole thing because I haven't been able to find a truly portable solution, yet...

[csobrinho]

Nevermind, I only saw the diff and didn't see it was already inside a union. Disregard my comment :)