Local const copy of global pointer for embedded C

Question

I am a junior embedded software, and I spend most of my time writing C code for dspic33 microcontrollers.

My embedded code is timer interrupts based, with a couple of global variables. Let's consider the simplest code possible:

typedef struct data_t
{
    uint16_t a;
    uint16_t b;
} data;

data my_data = {0, 0};
int16_t main(void)
{
    my_data.a = 10;

    while(1)
    {
        // can access and modify my_data here
    }
}

and from another file:

extern my_data data;
void timer_interrupt()
{
    if (my_data.a == 10)
    {
        // stuff here
    }
}

This code works fine, but if I add scope control on my_data by replacing the variable declaration by:

static data my_data = {10, 10};

my_data is not accessible outside the main file anymore. So cool, I can create a global pointer and continue to access my_data member anywhere:

static data my_data = {10, 10};
data * const my_data_ptr = &my_data;

// from another file
extern data * const my_data;
void timer_interrupt()
{
    if (my_data_ptr->a == 10)
    {
        // do stuff here
    }
}

I prevent eventual reassignment by making the pointer constant, and I am sure my_data_ptr will point to the my_data's address for the entire program lifetime. So far, I meet all the requirements - my personal good practice requirements - I limit the scope of my global variables and preserve the variables' integrity.

Now let's imagine two functions, with requirements I define that function one needs read-only my_data and function two needs a read and write on my_data, something like:

// read-only access
void function_one()
{
    const data * const local_my_data_copy = my_data_ptr;
    
    if (local_my_data_copy->a == 10)
    {
        // do stuff here
    }
}

// read and write access
void function_two()
{
    data * local_my_data_copy = my_data_ptr;
    
    if (local_my_data_copy->a == 0)
    {
        // do stuff here
        local_my_data_copy->a = 20;
    }
}

So finally, my question is it relevant to create a local copy of my_data_ptr in every function that needs to access/modify the pointed data? From my perspective, it is relevant for variables access control and can prevent mistakes during code writing. This implementation requests more stack and data space usage because of the extra variables declaration needed, and microcontrollers are memory limited. Hard for me to determine the CPU cost of this implementation. Please let me know your point of view on this piece of code.

Answer 1

Access to global data[] deserves special consideration given it is to read/written by an interrupt routine and non-interrupt code.

Instead of protecting via const and auxiliary pointers, consider only access functions - no variable access for the non-interrupt code.

uint16_t data_get_a(void);
uint16_t data_get_b(void);
void data_get_ab(uint16_t *a, uint16_t *b);
void data_set_ab(uint16_t a, uint16_t b);

In non-interrupt code, there is reduced need for speediness, but great need for access integrity such as setting members .a and .b with no possibility of an interrupt in between.

These helper functions can provide temporary interrupt disabling or other mechanics to insure atomic access. That may be very important. See atomic. Perhaps as:

foo() {
  save timer interrupt state
  disable timer interrupt
  access data
  restore timer interrupt state
} 

Answer 2

Your function_two() circumvents the const "promise" of my_data. That alone is bad practice, but since it is allowed the safer solution is the avoidance of global data. Your use of a pointer-to-const to enforce read-only semantics is only valid if you truly only ever want it to be read-only - and clearly you don't. Creating a global pointer to a static only creates a useless level of indirection and little else - you have not avoided a global or mad the code significantly safer. It is safer only in the sense that the coder has to explicitly circumvent it in the way you have, so avoids accidental modification. There is so much more control you can impose as the author of this code to avoid future maintainers (which may be you) shooting themselves in the foot.

The solution in this case is that any access to the shared data should be through accessor functions that have visibility of the data. In this case that might mean implementing function_one() and function_two() in the same file as the static my_data, but that may not be desirable for good cohesion and loose coupling. The generic solution then is to write access functions that control and enforce your desired semantics.

Example (flawed - read-on):

static volatile data my_data = {10, 10};
const data* getMyData(){ return &my_data ; }
void setMyData( const data* d ){ my_data = *d ; }

The advantages of getter/setter functions over global access (indirect or otherwise), include:

• You can enforce safe access to non-atomic data objects - like the one in this question (by various means - that is perhaps another question).
• In debugging, a breakpoint in the get/set will trap all accesses without the need to place a breakpoint on every access, or use data watch breakpoints.
• You can enforce common validation, range-limiting or asserts in a setter to ensure only valid data is written.
• You can perform "on-access" transformations such as converting ADC units to millivolts on read, or vice-versa for writes. Thus decoupling the data presentation from its internal representation, Doing such conversions in the get/set avoids possibly expensive processing in the interrupt context.

You can of course have access functions for individual members of the structure, allowing for example to enforce read-only for some members by not defining a setter, or even write-only semantics by defining only a setter - a semantic that cannot be enforced on a global using const. Overall it is far more flexible and much safer then using a global and trying to enforce semantics that const cannot support.

Really read the earlier linked article A pox on globals by Jack Ganssle. It will be enlightening and stand you in good stead as an embedded systems developer for the future.

---

Some other points:

You really should declare data shared between contexts volatile (see https://www.embedded.com/introduction-to-the-volatile-keyword/). But be aware that volatile alone is not a complete solution to safe shared memory between contexts.

You have to be aware for example that if the data access from the main thread (or any other thread context) is not atomic, it may be interrupted and the data modified part way through the process of reading it. For 32 bit integers on a 16 bit device that may mean for example you end up with the high-word from the old value and the low word from the new value - of vice versa. Even for data types that are atomic, operations on then need not be. For example i += j implies a read-modify-write operation that might be interruptible. For data structures it may mean that one member is inconsistent with another.

I raise the point here, because your code clearly exhibits these problems, but equally it is clearly not real code and you may already be aware of these issues. It is also not what your question is about and you might post a new question if you need solutions, but for what it is worth I provide one possible solution for the getter earlier:

data* getMyData( data* d )
{
    // read my_data until a consistent copy has been obtained. 
    do
    {
        *d = mydata ;

    } while( *d != my_data ) ;

    return d ;
}

A final point, it is largely irrelevant if you take the advice above, but regarding:

[...] and I am sure my_data_ptr will point to the my_data's address for the entire program lifetime.

There is no need to trust that to chance or maintenance error - you can enforce that assertion:

      const data* const my_data_ptr = &my_data;
 //   ^^^^^