0

I have a struct of different data types, which should be written into the non-volatile memory. It is possible to write it word after word, but I'm unsure, how I can read a word from my struct.

The struct looks like this:

typedef struct
{
    uint8_t value1;
    int16_t value2;
    uint32_t value3;
    float value4;
} __attribute__(( aligned(4))) test_struct_t;

How is it possible, to write word by word from this struct? With the aligned(4) attribute this should be possible, right?

HansPeterLoft
  • 489
  • 1
  • 9
  • 28
  • Encapsulate it in an union with another struct of words/array? – Jose Aug 18 '20 at 08:16
  • Skip the __attribute__ -- you should only ever use those things as a last resort. When your program is loaded into RAM, does it require any intervention to copy the globally defined data structures, such as the FILE structures for stdin, stdout, stderr? No, it just copies them as a straight image. You can do the same to copy them out, but from a good software perspective, you might consider a serialization library like google protocol buffers. – mevets Aug 18 '20 at 14:19

3 Answers3

1

How is it possible, to write word by word from this struct? With the aligned(4) attribute this should be possible, right?

Hmm, no, the answer to that is that writing the struct to a non-volatile storage system using 32bit "words" would make the resulting data unportable across systems due to possible Byte Endianness concerns.

EDIT: If the data isn't going anywhere

If the stored data isn't leaving the system, Endianness isn't an issue and that part of my original answer (further on) can be safely ignored.

I would still consider manually marking the padding in the struct and updating the naming as detailed below (see: Ease of use / Portability).

Also, if the NVM has a memory address and the standard library is available (or a compiler builtin function) - memcpy will be a simple solution and there's no need to worry about alignments and other implementation details.

Can we use a uint32_t *?

The issue is that if we use a pointer that violates the standard (invokes undefined behavior), than the compiler is free to assume that the data in the struct was never (properly) accessed and optimize away our read/write operations.

For example, this might leave us with old data in the struct even though we thought we performed a "read" operation (that was optimized away).

There is a good solution offered in @Lundin's answer.

Lundin's solution is one way to deal with the restrictions stated in section 6.5 (paragraph 7) of the C11 standard - restrictions that C compilers rely upon when optimizing your code:

An object shall have its stored value accessed only by an lvalue expression that has one of the following types:

— a type compatible with the effective type of the object,

— a qualified version of a type compatible with the effective type of the object,

...

— an aggregate or union type that includes one of the aforementioned types among its members (including, recursively, a member of a subaggregate or contained union), or

— a character type.

A similar issue occurs in section 6.2.7 (paragraph 2) of the standard:

declarations that refer to the same object or function shall have compatible type; otherwise, the behavior is undefined.

@Lundin's approach uses a union type, which is the second to last solution. My approach used the last approach (a char type) when storing the data.

Another approach would use void * to move the calculation to an independent function (same approved as memcpy). This will prevent the compiler from assuming anything as void * conversions are always allowed.


Original Answer:


I think the problem of writing to non-volatile storage is divided into 2 main issues:

  1. Byte Endianness - different systems have different memory models and we want the non-volatile storage to be as system agnostic as possible.

  2. Ease of use / Portability - we don't want to rewrite a lot of pieces of code every time we update the structure and we want to minimize compiler specific instructions.

Byte Endiannes

If we store the data on one system and than load it in another system, the byte ordering may be different between systems, breaking our data scheme.

To solve Byte Endiannes it's important to read and store 32bit numbers and 16 bit numbers in a way that is byte order agnostic. This means storing the specific bits in a pre-determined bytes.

I wrote a few macros for that (based on the facil.io open source framework):

/** write uint16_t to a buffer independently of system's of endieness. */
#define u2buf16(dest, i)                                                       \
  do {                                                                         \
    ((uint8_t *)(dest))[0] = ((uint16_t)(i)) & 0xFF;                           \
    ((uint8_t *)(dest))[1] = (((uint16_t)(i)) >> 8) & 0xFF;                    \
  } while (0)

/** write uint32_t to a buffer independently of system's of endieness. */
#define u2buf32(dest, i)                                                       \
  do {                                                                         \
    ((uint8_t *)(dest))[0] = ((uint32_t)(i)) & 0xFF;                           \
    ((uint8_t *)(dest))[1] = (((uint32_t)(i)) >> 8) & 0xFF;                    \
    ((uint8_t *)(dest))[2] = (((uint32_t)(i)) >> 16) & 0xFF;                   \
    ((uint8_t *)(dest))[3] = (((uint32_t)(i)) >> 24) & 0xFF;                   \
  } while (0)

/** read uint16_t from a buffer independently of system's of endieness. */
#define buf2u16(src)                                                           \
  ((uint16_t)(((uint8_t *)(src))[0]) | ((uint16_t)(((char *)(src))[1]) << 8))

/** read uint32_t from a buffer independently of system's of endieness. */
#define buf2u32(src)                                                           \
  (((uint32_t)(((uint8_t *)(src))[0])) |                                       \
   (((uint32_t)(((uint8_t *)(src))[1])) << 8) |                                \
   (((uint32_t)(((uint8_t *)(src))[2])) << 16) |                               \
   (((uint32_t)(((uint8_t *)(src))[3])) << 24))

Ease of use / Portability

The best way to deal with this is to encapsulate the code in proper functions.

Before I did that, I updated the example in a number of small ways:

  1. I removed the __attribute__(( aligned(4))) instruction, as it's both compiler specific and unnecessary.

  2. I renamed the struct's postfix from _t to _s.

    The _t type postfix is reserved by the POSIX standard and mustn't be used in user-code.

    The _s postfix is a common way to indicate struct (or _u for unions, and _p for pointers, etc'). This is a personal preference and depends on where you work.

  3. I added a uint8_t reserved; field where padding would have naturally occurred, both making room for future updates and making sure no unknown padding was present in the struct (here I use this quality only during testing).

This came out as:

typedef struct {
  uint8_t value1;
  uint8_t reserved; /* reserved for future versions */
  int16_t value2;
  uint32_t value3;
  float value4;
} test_struct_s;

For the function decelerations (API) I used the root of the type name (test_struct) as the name spaced appended the function names at the end.

This approach too is a common way to manage namespaces and is a personal preference that depends on the guidelines in your workspace.

They came up as:

static void test_struct_write(char *dest, test_struct_s *src);
static void test_struct_read(test_struct_s *dest, char *src);

Now, the first thing to do is to write tests for the code.

By writing a bit in every byte before a read/write roundtrip, it is possible to effectively test the read/write roundtrip for correctness.

In addition, we want to make sure that each field in the tested struct had a different bit set, so we make sure to test that we're not mixing up values.

/** test read/write behavior. */
static void test_struct_rw_test(void) {
  /*
   * write defferent values to each struct field, making sure all bytes have a
   * set bit at least once during the test.
   *
   * perform a read/write roundtri[ and test that the set bit has the same
   * value.
   */
  for (size_t i = 0; i < 32; ++i) {
    union {
      float f;
      uint32_t i;
    } u;
    /* a float with a single bit set somewhere */
    u.i = ((uint32_t)1U << ((i + 1) & 31));
    /* a different bit is set in every struct field */
    test_struct_s r, s = {
                         .value1 = (1U << ((i + 0) & 31)),
                         .reserved = (1U << ((i + 1) & 31)),
                         .value2 = (1U << ((i + 2) & 31)),
                         .value3 = (1U << ((i + 3) & 31)),
                         .value4 = u.f,
                     };
    char buf[sizeof(s)];
    test_struct_write(buf, &s);
    test_struct_read(&r, buf);
    /* we can use memcmp only because we control the padded bytes with
     * `reserved` */
    if (memcmp(&s, &r, sizeof(s))) {
      fprintf(stderr, "FATAL: Read/Write rountrip test failed (at %zu)\n", i);
      fprintf(stderr, "%u!=%u\n%u!=%u\n%d!=%d\n%u!=%u\n%f!=%f\n", s.value1,
              r.value1, s.reserved, r.reserved, s.value2, r.value2, s.value3,
              r.value3, s.value4, r.value4);
      exit(-1);
    }
  }
}

Next we need to actually code the read / write functions themselves.

As you will notice, I am assigning each 8 bit sequence to a specific byte, allowing the code to be endianness and system agnostic.

The buffer being written to or read from MUST be (at least) 96 bits long (12 bytes), or the functions will overflow.

/** "safely" write test_struct_s to a buffer. buffer MUST be 96 bits long. */
static void test_struct_write(char *dest, test_struct_s *src) {
  union {
    float f;
    uint32_t i;
  } u;
  if (sizeof(float) != sizeof(uint32_t))
    goto system_error; /* will be tested by the compiler and optimized away... */
  u.f = src->value4;
  dest[0] = src->value1;
  dest[1] = src->reserved;
  u2buf16(dest + 2, src->value2);
  u2buf32(dest + 4, src->value3);
  u2buf32(dest + 8, u.i);
  return;

system_error:
  fprintf(stderr,
          "FATAL: Program requires a modern system where floats are 32 bits "
          "(sizeof(float) == %zu)\n",
          sizeof(float));
  exit(-1);
}

/** "safely" read test_struct_s from a buffer. buffer MUST be 96 bytes long. */
static void test_struct_read(test_struct_s *dest, char *src) {
  if (sizeof(float) != sizeof(uint32_t))
    goto system_error;
  union {
    float f;
    uint32_t i;
  } u;
  dest->value1 = src[0];
  dest->reserved = src[1];
  dest->value2 = buf2u16(src + 2);
  dest->value3 = buf2u32(src + 4);
  u.i = buf2u32(src + 8);
  dest->value4 = u.f;
  return;

system_error:
  fprintf(stderr,
          "FATAL: Program requires a modern system where floats are 32 bits "
          "(sizeof(float) == %zu)\n",
          sizeof(float));
  exit(-1);
}

And we're done.

The whole of the code looks something like this:

#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

/*
 * added a `reserved` field to mark natural struct padding AND control padded
 * bytes.
 *
 * now, sizeof(test_struct_s) == 96
 */
typedef struct {
  uint8_t value1;
  uint8_t reserved; /* reserved for future versions */
  int16_t value2;
  uint32_t value3;
  float value4;
} test_struct_s;

static void test_struct_write(char *dest, test_struct_s *src);
static void test_struct_read(test_struct_s *dest, char *src);

/** write uint16_t to a buffer independently of system's of endieness. */
#define u2buf16(dest, i)                                                       \
  do {                                                                         \
    ((uint8_t *)(dest))[0] = ((uint16_t)(i)) & 0xFF;                           \
    ((uint8_t *)(dest))[1] = (((uint16_t)(i)) >> 8) & 0xFF;                    \
  } while (0)

/** write uint32_t to a buffer independently of system's of endieness. */
#define u2buf32(dest, i)                                                       \
  do {                                                                         \
    ((uint8_t *)(dest))[0] = ((uint32_t)(i)) & 0xFF;                           \
    ((uint8_t *)(dest))[1] = (((uint32_t)(i)) >> 8) & 0xFF;                    \
    ((uint8_t *)(dest))[2] = (((uint32_t)(i)) >> 16) & 0xFF;                   \
    ((uint8_t *)(dest))[3] = (((uint32_t)(i)) >> 24) & 0xFF;                   \
  } while (0)

/** read uint16_t from a buffer independently of system's of endieness. */
#define buf2u16(src)                                                           \
  ((uint16_t)(((uint8_t *)(src))[0]) | ((uint16_t)(((char *)(src))[1]) << 8))

/** read uint32_t from a buffer independently of system's of endieness. */
#define buf2u32(src)                                                           \
  (((uint32_t)(((uint8_t *)(src))[0])) |                                       \
   (((uint32_t)(((uint8_t *)(src))[1])) << 8) |                                \
   (((uint32_t)(((uint8_t *)(src))[2])) << 16) |                               \
   (((uint32_t)(((uint8_t *)(src))[3])) << 24))

/** "safely" write test_struct_s to a buffer. buffer MUST be 96 bytes long. */
static void test_struct_write(char *dest, test_struct_s *src) {
  union {
    float f;
    uint32_t i;
  } u;
  if (sizeof(float) != sizeof(uint32_t))
    goto system_error;
  u.f = src->value4;
  dest[0] = src->value1;
  dest[1] = src->reserved;
  u2buf16(dest + 2, src->value2);
  u2buf32(dest + 4, src->value3);
  u2buf32(dest + 8, u.i);
  return;

system_error:
  fprintf(stderr,
          "FATAL: Program requires a modern system where floats are 32 bits "
          "(sizeof(float) == %zu)\n",
          sizeof(float));
  exit(-1);
}

/** "safely" read test_struct_s from a buffer. buffer MUST be 96 bytes long. */
static void test_struct_read(test_struct_s *dest, char *src) {
  if (sizeof(float) != sizeof(uint32_t))
    goto system_error;
  union {
    float f;
    uint32_t i;
  } u;
  dest->value1 = src[0];
  dest->reserved = src[1];
  dest->value2 = buf2u16(src + 2);
  dest->value3 = buf2u32(src + 4);
  u.i = buf2u32(src + 8);
  dest->value4 = u.f;
  return;

system_error:
  fprintf(stderr,
          "FATAL: Program requires a modern system where floats are 32 bits "
          "(sizeof(float) == %zu)\n",
          sizeof(float));
  exit(-1);
}

/** test read/write behavior. */
static void test_struct_rw_test(void) {
  /*
   * write defferent values to each struct field, making sure all bytes have a
   * set bit at least once during the test.
   *
   * perform a read/write roundtri[ and test that the set bit has the same
   * value.
   */
  for (size_t i = 0; i < 32; ++i) {
    union {
      float f;
      uint32_t i;
    } u;
    /* a float with a single bit set somewhere */
    u.i = ((uint32_t)1U << ((i + 1) & 31));
    test_struct_s r, s = {
                         .value1 = (1U << ((i + 0) & 31)),
                         .reserved = (1U << ((i + 1) & 31)),
                         .value2 = (1U << ((i + 2) & 31)),
                         .value3 = (1U << ((i + 3) & 31)),
                         .value4 = u.f,
                     };
    char buf[sizeof(s)];
    test_struct_write(buf, &s);
    test_struct_read(&r, buf);
    /* we can use memcmp only because we control the padded bytes with
     * `reserved` */
    if (memcmp(&s, &r, sizeof(s))) {
      fprintf(stderr, "FATAL: Read/Write rountrip test failed (at %zu)\n", i);
      fprintf(stderr, "%u!=%u\n%u!=%u\n%d!=%d\n%u!=%u\n%f!=%f\n", s.value1,
              r.value1, s.reserved, r.reserved, s.value2, r.value2, s.value3,
              r.value3, s.value4, r.value4);
      exit(-1);
    }
  }
}

int main(void) {
  test_struct_rw_test();
  fprintf(stderr, "PASSED\n");
  return 0;
}
Myst
  • 18,516
  • 2
  • 45
  • 67
  • "The _t type postfix is reserved by the POSIX standard and mustn't be used in user-code." Embedded systems don't care about POSIX. `_t` was industry standard notation long before POSIX came along. It is unfortunate that POSIX was so poorly implemented, it is constantly colliding with the C standard and de facto standards. – Lundin Aug 19 '20 at 07:41
  • @Lundin - Interesting history on `_t`, good to know. However, I find that using a more informative postfix is often helpful. In addition it self-documents the code. (i.e., `_s` vs. `_p`). This isn’t something I invented. It’s a common approach. – Myst Aug 19 '20 at 07:51
0

The following solution seems to work fine:

test_struct_t test_t;

uint16_t size = sizeof(test_struct_t)/sizeof(uint32_t);

uint32_t *ptr = (uint32_t *)&test_t;

for(uint16_t i = 0; i < size; i++)
{
    writeWordNVM(i, *ptr);
    ptr++;
}
HansPeterLoft
  • 489
  • 1
  • 9
  • 28
0

Apart from alignment, the issue here is the C type system, where each position in memory that contains a declared variable or has been de-referenced by the program through a value access, has an internal "type tag" formally known as effective type. This is used for optimization purposes, so that the compiler can know if pointer types of different kinds may alias. That is, if they could refer to the same memory location. Informally this whole set of rules is called the "strict aliasing rules". What is the strict aliasing rule?

Simple example of pointer aliasing: if you have void func (float* f){ *f = 1.0f; } and a global int foo;, the compiler is free to assume that the function writing to *f won't modify the contents of foo when doing so.

Traditionally, embedded system compilers never used optimization based on aliasing assumptions, but with gcc entering the embedded world, this is no longer necessarily the case. If you compile with gcc -fno-strict-aliasing, you can dodge the problem, but the code won't be portable.

This is kind of a language defect. To portably avoid the strict aliasing rules, you must use cumbersome tricks such as union type punning. That is, create a union containing the struct and an array of uint32_t both, then do all de-referencing on the array member. Example:

#include <stdio.h>
#include <stdint.h>

typedef struct
{
  uint8_t   value1;
  int16_t   value2;
  uint32_t  value3;
  float     value4;
} test_struct_t;

_Static_assert(_Alignof(test_struct_t) == _Alignof(uint32_t), 
               "Inconsistent alignment in test_struct_t");

typedef union
{
  struct 
  {
    uint8_t  value1;
    int16_t  value2;
    uint32_t value3;
    float    value4;
  };
  
  uint32_t u32 [sizeof(test_struct_t)/sizeof(uint32_t)];
  
} test_union_t;


int main(int argc, char **argv) 
{ 
  test_union_t test = {.value1 = 1, .value2 = 2};
  printf("%.8X\n", test.u32[0]);
  return 0; 
}

Here I first made a typedef of the struct just to check with static assert if the alignment is ok, removing the non-standard __attribute__(( aligned(4))).

Then when creating the type punning union, I made the same struct anonymous just so that we don't have to type out test.test_struct.value1 rather than test.value1, kind of hiding the fact that it's a union.

I then initialized some members of the union by name and de-referenced them through a type punned uint32_t. This prints 00020001 on my little endian machine, apparently there's a padding byte after value1, which is as expected.

Lundin
  • 195,001
  • 40
  • 254
  • 396
  • However, saving the data word by word might not be portable due to endieness differences across systems. IMHO, the whole approach should be revisited (see my answer for details). – Myst Aug 18 '20 at 15:08
  • @Myst Since it is an embedded system, porting between different CPUs isn't as likely as porting between different compilers. So it is often far more important to write code that doesn't rely on poorly-defined behavior or non-standard extensions, than to write endianess-independent code. – Lundin Aug 19 '20 at 07:37
  • oh, cool. I wasn’t aware that this is for an embedded system and the stored data isn’t “traveling” between systems. – Myst Aug 19 '20 at 07:42
  • @Myst "Write struct to NVM" makes it very likely, since hosted systems do not typically have NVM, apart from hard drives with file systems. – Lundin Aug 19 '20 at 07:46