Modular Design Patterns in C

Question

I have created a modular design pattern which provide a single interface that can be used create instances with swapable back-end components, however I'm not entirely satisfied with it.

My practical implementation involves creating a generic interface to certain kinds of device drivers. The hope is that I can create an interface which exposes a layer meant as an adapter (for initializing drivers which non-interface conforming implementations) and to bring up framework infrastructure.

To bring focus to the design pattern itself I am showing a simplified example.

The source code can be found here.

say I have some application with a main.c like so:

#include <stdio.h>
#include <stdlib.h>
#include "boatModuleIF.h"

int main(int argc, char *argv[]) {

    char *type = argv[1];
    int typeNum = atoi(type);

    moduleIF_t *IF = init_moduleIF(typeNum);

    if (IF) {
        IF->printCfg(IF->ctx);
    }
    else {
        printf("Failed to init module");
    }

    return 0;
}

I use boatModuleIFDefs as a header which is common to all boatModuleComponents (i.e. boatModule, boatModuleIF, cnc, beneteau)

It exposes the following to those components: boatModuleIFDefs.h

#ifndef __MODULEIFDEFS_H_
#define __MODULEIFDEFS_H_

typedef struct moduleIF_CTX *moduleIF_CTX_t;

typedef struct {
    void (*printCfg)(moduleIF_CTX_t ctx);

    moduleIF_CTX_t ctx;
} moduleIF_t;

__attribute__((weak)) extern moduleIF_t *init_moduleIF_CNC();
__attribute__((weak)) extern moduleIF_t *init_moduleIF_Beneteau();

#endif // __MODULEIFDEFS_H_

Note that the init function for the CNC and Beneteau module interfaces are declared as weak symbols. This means that this generic code can expose a symbol which may or may not be defined. This is used as a proxy to determine whether our application has been compiled with the cnc.c "driver".

with boatModuleIF's header and source implemented as:

boatModuleIF.h

#ifndef __TESTMODULEIF_H_
#define __TESTMODULEIF_H_
#include "boatModuleIFDefs.h"

enum {
    TYPE_CNC = 0,
    TYPE_BENETEAU,
};

moduleIF_t *init_moduleIF(int type);

#endif // __TESTMODULEIF_H_

boatModuleIF.c

#include <stdlib.h>
#include <stdio.h>
#include <stdint.h>
#include "boatModuleIFDefs.h"
#include "boatModuleIF.h"

moduleIF_t *init_moduleIF(int type) {
    switch (type) {
        case TYPE_CNC:
            if (init_moduleIF_CNC) {
                return init_moduleIF_CNC();
            }
            else {
                printf("failed cnc");
                return 0;
            }
            break;
        case TYPE_BENETEAU:
            if (init_moduleIF_Beneteau) {
                return init_moduleIF_Beneteau();
            }
            else {
                printf("failed ben");
                return 0;
            }
            break;
        default:
            return 0;
    }
}

boatModule also has a defs file which looks like:

#ifndef __TESTMTRDEFS_H_
#define __TESTMTRDEFS_H_

#include <stdbool.h>
#include <stdint.h>
#include <stdlib.h>

typedef struct {
    uint8_t  size;
    uint8_t  speed;
} boatCfg_t;

__attribute__((weak)) extern boatCfg_t dfltBoatCfg;

#endif // __TESTMTRDEFS_H_

Note that the dfltBoatCfg is defined as a weak symbol similarily to our init functions. This is useful if we have multiple kinds of configurations that may not be applicaple to all sailboats.

boatModule's header and source implemented as:

boatModule.h

#ifndef __TESTMODULE_H_
#define __TESTMODULE_H_
#include "boatModuleIFDefs.h"

moduleIF_t *init_moduleIF();

#endif // __TESTMODULE_H_

boatModule.c

#include <stdlib.h>
#include <stdio.h>
#include <stdint.h>
#include "boatModuleIFDefs.h"
#include "boatModule.h"
#include "boatModuleDefs.h"

struct moduleIF_CTX {
    uint8_t size;
    uint8_t speed;
};

static void printCfg(moduleIF_CTX_t ctx) {
    printf("speed %d size %d", ctx->speed, ctx->size);
}

moduleIF_t *init_moduleIF() {
    struct moduleIF_CTX * const CTX = calloc(1, sizeof(struct moduleIF_CTX));

    if (&dfltBoatCfg) {
        CTX->size = dfltBoatCfg.size;
        CTX->speed = dfltBoatCfg.speed;
    }

    moduleIF_t * const IF = (moduleIF_t *) calloc(1, sizeof(moduleIF_t));
    IF->ctx = CTX;
    IF->printCfg = printCfg;
    return IF;
}

Then we have two source files for two different boatModule implementations (which here are a stand-in for drivers)

cnc.c

#include "boatModuleDefs.h"

boatCfg_t dfltBoatCfg = {
    .size = 10,
    .speed= 10,
};

beneteau.c

#include "boatModuleDefs.h"

boatCfg_t dfltBoatCfg = {
    .size = 5,
    .speed= 5,
};

The interesting part (imho) is really in the makefile however.

makefile

all: joinedModules

joinedModules: boatModule_Cnc.o boatModule_Beneteau.o boatModuleIF.o boatModuleDefs.h
    gcc boatModuleIF.o boatModule_Cnc.o boatModule_Beneteau.o main.c -o getBoatCfg

boatModuleIF.o:
    gcc -c boatModuleIF.c

boatModule_Beneteau.o: boatModule.o beneteau.o boatModuleDefs.h
    ld -r beneteau.o boatModule.o -o boatModule_Beneteau.o
    objcopy --redefine-sym printCfg=printCfg_Beneteau boatModule_Beneteau.o
    objcopy --redefine-sym init_moduleIF=init_moduleIF_Beneteau boatModule_Beneteau.o
    objcopy --redefine-sym dfltBoatCfg=dfltBoatCfg_Beneteau boatModule_Beneteau.o

boatModule_Cnc.o: boatModule.o cnc.o boatModuleDefs.h
    ld -r cnc.o boatModule.o -o boatModule_Cnc.o
    objcopy --redefine-sym printCfg=printCfg_CNC boatModule_Cnc.o
    objcopy --redefine-sym init_moduleIF=init_moduleIF_CNC boatModule_Cnc.o
    objcopy --redefine-sym dfltBoatCfg=dfltBoatCfg_CNC boatModule_Cnc.o

boatModule.o: beneteau.o cnc.o boatModuleDefs.h
    gcc -c -fPIE boatModule.c

cnc.o: boatModuleDefs.h
    gcc -c -fPIE cnc.c

beneteau.o: boatModuleDefs.h
    gcc -c -fPIE beneteau.c

Essentially what I am doing is merging both the module object file and a backend component together to create a new merged object file. I can then redefine the global symbols used by both to prevent symbol collisions in such a way that maintains the "simplicity" of the generic module interface files.

This has some benefits, notably:

• During initialization of the interface I don't need some messy switch case which would have to check whether each type boat has each kind of boat config.
• Supports code reuse.
• If I want to add new configuration's there's little that needs to be added (except for one addtional if statement in the moduleif init and the definition in the boat/driver that actually cares about that config).
• If I want to add a new boat then all I need is to add a new init weak symbol and some tweaks to the makefile (really just adding the source name, the rest could be done automatically with make rules)

The issue is this doesn't quiet feel right and I'm wondering if this is some kind of code smell?

It could make evaluating the code much more difficult (e.g. I have not defined any symbol named dfltBoatCfg, it's redefined as dfltBoatCfg_CNC and dfltBoatCfg_Beneteau) however this could be mitigated with good comments/documentation.

If so is there some better/modified approach I could take would allow me to create this kind of modular design pattern that is future proofed against maintanence hell in the event that we should have to support many types of boats with many different configurations?

Any and all feedback is much appreciated.

Answer 1

Don't hide pointers

Don't hide pointers in a typedef:

typedef struct moduleIF_CTX *moduleIF_CTX_t;

This makes it really hard to spot when things are passed by value or by pointer in the rest of the code. You could make it more explicit:

typedef struct moduleIF_CTX *moduleIF_CTX_ptr_t;

But I would just do this:

typedef struct moduleIF_CTX moduleIF_CTX;

Yes, you can typedef a struct to its own name, and it does what you want. And now you can use this as follows:

typedef struct {
    void (*printCfg)(moduleIF_CTX *ctx);
    moduleIF_CTX *ctx;
} moduleIF;

It's both a bit less to type and more explicit at the same time.

Consider using a dynamically created registry for modules

The main drawback of your approach is having to change init_moduleIF() every time you add or remove a module. It would be nicer if modules could somehow register themselves in an array or list. One way to do that is to write functions that are called at program startup and/or library loading time. These are functions that go into a special dynamic linker section. How to get functions in there is not standard C, but with GCC at least you can use __attribute((constructor)) to mark a function as having to run before main() is started. Here is an example of how it could be used. First, ensure moduleIF contains its type identifier, and a pointer to the next moduleIF:

typedef struct moduleIF {
    const int type; // or char *moduleName...
    struct moduleIF *next;

    struct moduleIF *(*init)(void);
    void (*printCfg)(moduleIF_CTX *ctx);
    ...
} moduleIF;

Then add a global variable that holds a pointer to the first module, and create a function to add new modules to that list:

moduleIF *modules = NULL;

void registerIF(moduleIF *if) {
    if->next = modules;
    modules = if;
}

Then in a module itself, like boatModule_Cnc.c, do:

static moduleIF boatIF {
    .type = TYPE_CNC,
    .init = init_Cnc,
    .printCfg = printCfg_Cnc,
};

static void registerBoatIF(void) __attribute__((constructor));
static void registerBoatIF(void) {
    registerIF(&boatIF);
}

If you now link multiple modules into a single binary, then all of their constructor functions will be called before main() starts, so at that time modules will be a linked list containing all the registered module interface definitions. You can then change init_moduleIF() to:

moduleIF *init_moduleIF(int type) {
    for (moduleIF *if = modules; if; if = if->next) {
        if (if->type == type && if->init) {
            return if->init();
        }
    }

    return NULL;
}

About using weak and renamed symbols

I think this approach is less flexible than the one I outlined above. Also, if you know at compile time which .o files you are going to link together, then instead of using weak symbols, you can also compile boatModuleIF.c with -Dinit_moduleIF_CNC=NULL if you know the CNC module is not compiled in, or define something else and use #ifdefs inside init_moduleIF() to compile only those cases that are valid. I think that is just as pretty (or ugly, depending what you think of it) as using weak symbols, except that it relies less on linker tricks.

As for renaming the symbols, this is also unnecessary. printCfg() is static, and init_moduleIF_*() will just put a pointer to this local version of printCfg() in the moduleIF_t that is returned.

You do need to rename dfltBoatCfg, because otherwise only of the dfltBoatCfg instances survives the weak linking. But if you don't rename that symbol, it will compile and link without errors.

Another way to solve this issue is by not doing any symbol renaming tricks, and not having boatModule.c linked in multiple times. Instead, link it once in the final binary, and have it take a pointer argument that can be used to point to a custom boatCfg_t, like so:

moduleIF_t *init_moduleIF(const boatCfg_t *boatCfg) {
    ...
    if (cfg) {
        CTX->size = boatCfg.size;
        CTX->speed = boatCfg.speed;
    }
    ...
}

And for example in cnc.c, write:

#include "boatModuleDefs.h"

static const boatCfg_t boatCfg = {
    .size = 10,
    .speed= 10,
};

moduleIF_t *init_moduleIF_CNC() {
    return init_moduleIF(boatCfg);
}

It's just a few lines extra, but now I can just see what happens by looking at the source, instead of having to understand your build system as well. This approach is also more portable.