HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Compile-time plugin system

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
pluginsystemtimecompile

Problem

Background

For a piece of software that I need to get extensible, I wanted to design a
simple compile-time "plugin" system in C++.

My use case was that the program (actually a library) can accept several input
formats at the same time according to a requested interface version. There is
a factory that selects the right input parser (let's call that the plugin),
but the idea was to find a mechanism that doesn't require to change a common
file registering all the known plugins, but rather that each files can
register itself by the simple fact of being compiled-in.

Here, plugins are just cpp files, as in the project it was not an option to
compile a dynamic library and load the plugins from them. That's why it's a
"compile-time plugin", though the word "plugin" may not make much sense in this
case...

I refered to this SO question: Compile-time plugin / Automated Factory Registration with separate modules constraint

The plugin Mechanism


plugins.h

```
#include
#include
#include

namespace PluginSystem {

/ Base class for plugins /
class IPlugin {
public:
virtual void DoSomething() = 0;
};

/*
* Base class for PluginRegistrar
* See PluginRegistrar below for explanations
*/
class IPluginRegistrar {
public:
virtual IPlugin* GetPlugin() = 0;
};

/*
* This is the factory, the common interface to "plugins".
* Plugins registers themselves here and the factory can serve them on
* demand.
* It is a Singleton
*/
class PluginFactory {
public:
/ Get Singleton instance /
static PluginFactory& Instance();
/ Register a new plugin /
void Register(IPluginRegistrar * registrar, std::string name);
/ Get an instance of a plugin based on its name /
/ throws out_of_range if plugin not found /
IPlugin* GetPlugin(std::string name);

private:
/ Holds pointers to plugin registrars /
std::map reg

Solution

Interface

class IPluginRegistrar {
public:
    virtual IPlugin* GetPlugin() = 0;
};


Is GetPlugin() ever going to return a nullptr? I don't think that would be appropriate. So this should return a reference (assuming the IPluginRegistrar retains ownership of the plugin). Alternatively it should return a smart pointer that indicates the type of ownership of the object that is being returned.

Another location where you are using pointers inappropriately:

void Register(IPluginRegistrar * registrar, std::string name);


Can registrar be nullptr? Who owns the registrar object? You need to think about your ownership semantics of the interface. You are using a very C style of just passing pointers around. This is not how things are done in C++ (or at least modern C++).

Yep this is an old trick.

#define REGISTER_PLUGIN(CLASSNAME) \
namespace { \
    static PluginSystem::PluginRegistrar \
    CLASSNAME##_registrar( #CLASSNAME ); \
};


But I am not sure it really but you anything (apart from an ugly macro).

REGISTER_PLUGIN(MyPlugin);
// or
PluginSystem::PluginRegistrar   myPluginRegitra;


I prefer the second one as it is easier to see what is actually happening.

C++ Old School

This was how you removed the copy constructor and copy assignment operator in C++03 (but you did it wrong). You are not supposed to have {} just put the ;. In your technique you can actually still use them in some contexts that the compiler can't catch. If you don't provide definitions (by using the ;) then it still compiles fine if you don't use these operators but if you accidentally do use them in contexts where they are allowed you get linker errors.

private:
    PluginFactory(PluginFactory const&) {};
    void operator=(PluginFactory const&) {}; // Note: usually this would return PluginFactory&


Even then. This is very old school. Modern C++ has a better technique.

PluginFactory(PluginFactory const&)  = delete;
    void operator=(PluginFactory const&) = delete;


Bugs

registrar = registry_[name]; /* throws out_of_range if plugin unknown */


No it does not. If name is not in the map then it creates an entry in the map and the Value part of the map is initialized with zero-initialization (and since it is a pointer it will creeate nulltr). Now your second line will crash (as you are calling a method through a nullptr).

To not update the map. You must call find.

auto find = registry_.find(name);
if (find == registry_.end()) {
   throw std::out_of_range("Bad Name");
}
return find->second->GetPlugin();


Overall

Its pretty good. Your main issue is just deciding who owns object (do the register functions really need to dynamically create objects). Can you have stateless plugins?

Once you have decided on ownership then you have define the semantics you want to use to describe the ownership and thus how to enforce it.

Note: Ownership semantics is all about who is responsible for deleting the object at the end of its life cycle.

Code Snippets

class IPluginRegistrar {
public:
    virtual IPlugin* GetPlugin() = 0;
};
void Register(IPluginRegistrar * registrar, std::string name);
#define REGISTER_PLUGIN(CLASSNAME) \
namespace { \
    static PluginSystem::PluginRegistrar<CLASSNAME> \
    CLASSNAME##_registrar( #CLASSNAME ); \
};
REGISTER_PLUGIN(MyPlugin);
// or
PluginSystem::PluginRegistrar<MyPlugin>   myPluginRegitra;
private:
    PluginFactory(PluginFactory const&) {};
    void operator=(PluginFactory const&) {}; // Note: usually this would return PluginFactory&

Context

StackExchange Code Review Q#119812, answer score: 3

Revisions (0)

No revisions yet.