Using static allocation and encapsulation together

Question

After reading Design Patterns for Embedded Systems in C I changed the way I implement device drivers. The book suggests that a typical device driver header file has this implementation.

/* Encapsulated device handle using 'opaque' pointer */
typedef struct device device_t;

/* Allocated dynamically */
device_t*   device_create( i2c_t* bus, const uint8_t address );

void        device_destroy( device_t* me );    
int         device_initialize( device_t* me );
int         device_deinitialize( device_t* me );
int         device_isAvailable( device_t* me );
int         device_configure( device_t* me, const device_config_t config );

Yes I know C is not object-oriented but I like how clean the code smells. I changed the code a little bit. I sacrificed device handle encapsulation in order to be able to allocate it statically.

/* Public(no ecapsulation) device handle */
typedef struct device
{
    int public_member1;
    int public_member2;
} device_t;

/* Allocated statically */
device_t    device_create( i2c_t* bus, const uint8_t address );

int         device_initialize( device_t* me );
int         device_deinitialize( device_t* me );
int         device_isAvailable( device_t* me );
int         device_configure( device_t* me, const device_config_t config );

I would rather not allocate dynamically if possible since I fear memory fragmentation but I will if it is a good design desicion. Since my project is embedded and my target an cortex-m0 I wanted to know if I could use static allocation and ecapsulation together..

Update:

I give two different implementations of device_create for the above code

Dynamically:

device_t* device_create( i2c_t* bus, const uint8_t address )
{
    /* Check input */
    if( bus == NULL )
    {
        return NULL;
    }

    /* Create handle */
    device_t* aux = (device_t*)malloc( sizeof( device_t ) );
    if( aux == NULL )
    {
        return NULL;
    }

    /* Initialize handle */
    init( aux, bus, address );

    /* Return handle */     
    return aux;
}

Statically:

device_t device_create( i2c_t* bus, const uint8_t address )
{
    /* Check input */
    if( bus == NULL )
    {
        return NULL;
    }

    /* Create handle */
    device_t aux;

    /* Initialize handle */
    init( &aux, bus, address );

    /* Return handle */     
    return aux;
}

Frankly we have no means of evaluating the suitability of this design in the context of your needs as we do not know what they are. This is certainly one reasonable way of doing things. I might perhaps tend to suspect that the design may be needlessly general for a single specific application. At least I would start out using a static configuration and omitting the deinitialization and separate intitialization unless these are known requirements of the project so as to keep things simple. — doynax, Nov 21 '17 at 13:22
It is a hobby design. Not for work. I will change the expression "fit my need in the question". The question I wanted to ask is if i could do static allocation and ecapsulation both. Thanks for your reply. — Tedi, Nov 21 '17 at 13:29
_if i could do static allocation and ecapsulation both..._ Of course you can. This is only a decision. By _static_ allocation, I assume you mean _create on stack_ as opposed to _heap_. If you know the number of instances of `device_t` you will need before runtime, then by all means, create it statically. If not, create dynamically. — ryyker, Nov 21 '17 at 13:36
Encapsulation is valid goal, but in embedded world you need to be practical. You can accompany the struct definition with comment `// Do not access members directly, use provided functions instead.` If you need more idiot-proofing than that, you have bigger problems to deal with. — user694733, Nov 21 '17 at 13:46
@Tedi: Encapsulation is certainly still possible. The most straightforward route is allocate a single global device instance and expose only an opaque structure, that is don't bother supporting binding to multiple targets or using dynamic allocation to save space unless there is a specific requirement. Generally though I see little need to hide physically hide the data, instead agree upon a coding convention to hide the members if required. For instance a trailing `_` in the member name may be used to signify private fields not to be touched except through accessors. — doynax, Nov 21 '17 at 13:50
I'm confused by your question. I don't understand why you think allocation and encapsulation are linked. Why does your prototype for the "allocated statically" version return a struct object instead of a pointer? Show us the implementation of your two versions of device_create(). — kkrambo, Nov 21 '17 at 14:48
Wow, sounds like a good book for a change, those aren't common. The method taught is called "opaque type"/"opaque pointers". You can still use this method even without dynamic memory allocation, but it is a bit tricky. I linked a duplicate post that demonstrates various ways. On embedded systems, the most common way is to create a little memory pool for each ADT/driver. — Lundin, Nov 21 '17 at 15:04
As a side note, the proposed driver doesn't seem to be using _const correctness_. To improve the code further, make sure to add `const` where the function does not change the contents of the device_t. — Lundin, Nov 21 '17 at 15:08
Yes it is a very nice book. Thanks for the link. When it comes to pointers and const I forget them. I will add them to the real driver. :) @kkrambo: I updated the question and added the implementations. — Tedi, Nov 21 '17 at 15:27

vgru · Answer 1 · 2017-11-21T14:56:50.523

A simple way to resolve the issue would be to keep the allocation semantics separate from everything else, i.e.:

// allocation stuff only
device_t*   device_create( void );
void        device_destroy( device_t* me );    

// the rest will work the same in any case
int         device_initialize( device_t* me, i2c_t* bus, const uint8_t address );
int         device_deinitialize( device_t* me );
int         device_isAvailable( device_t* me );
int         device_configure( device_t* me, const device_config_t config );

Since device_create no longer accepts any parameters, and is basically a plain malloc under the hood, you can let the rest of the code unchanged:

// heap allocation
{
     device_t * some_device = device_create();
     device_initialize (some_device, some_bus, some_address);
     device_destroy(some_device);
}

// stack allocation
{
     device_t some_other_device;
     device_initialize (&some_other_device, some_bus, some_address);
}

(Update)

If you need the type to be an opaque struct, then you won't be able to do this, and in that case the only way to return the opaque pointer is from a compilation unit which knows the type, obviously, i.e.:

#include "internal_struct_defs.h"

// private field
static device_t some_device;

// this function just returns the pointer
device_t * device_create(void)
{
     // poor man's memory pool
     return &some_device;
}

which can basically be considered a "single element memory pool".

So, to recap:

if you have several devices known at compile time, you will need to statically allocate them and some part of your program will need to know which field corresponds to which device.
if you need to instantiate and cleanup multiple unknown devices at run time, then you need some form of an allocator (your own object pool, or simply malloc).

+1 But usually with embedded I wouldn't even bother creating `device_create()` at all. Default `malloc` version wouldn't likely suffice anyway, as I would have to use some RTOS equivalent. — user694733, Nov 21 '17 at 13:39
I think that for stack allocation and encapsulation the compiler does not know the size of device_t so I could not do 'device_t some_other_device' — Tedi, Nov 21 '17 at 13:57
@Tedi I think answer just left out the complete type definition to make answer shorter. You still need it. — user694733, Nov 21 '17 at 13:59
No! No! the device_t is declared as-is in the header file i showed and the complete type definition is hidden in the source file. It uses a technique called 'opaque' pointer — Tedi, Nov 21 '17 at 14:04
@Tedi: if the struct needs to be opaque, then, yes, you won't be able to create it in a compilation unit which doesn't know its contents. I have updated my answer. — vgru, Nov 21 '17 at 14:59

Using static allocation and encapsulation together

1 Answers1