1

I am trying to implement a wrapper for a library called libumqtt. Libumqtt is a C library that uses libev to have callbacks for events from the MQTT protocol.

What I didn't realize until the other day is that I cannot pass a member function to a function that expects a normal, static function. This causes problems as I was planning on launching multiple instances of libumqtt to handle multiple connections at the same time.

My code is in C++ as that is the most convenient to use with the Godot's (a game engine) GDNative module.

While researching for either a way to sandbox multiple instances of a c library or to somehow get the pointers to work anyway, I found this answer. I do not understand this quote from the answer:

If you need to access any non-static member of your class and you need to stick with function pointers, e.g., because the function is part of a C interface, your best option is to always pass a void* to your function taking function pointers and call your member through a forwarding function which obtains an object from the void* and then calls the member function.

What I am trying to do is setup callbacks that libev will use to send the data to the right instance of my object when it is handling potentially up to 500 or more connections simultaneously.

Will passing void* help me with my goals and how would I implement this? Also, how does a forwarding function work?

Edit: To Provide Code Example That Walnut Is Asking For

This example below comes from a version of my class that uses static functions. If I tried to use run this when the functions are not static, then I would get an error about not being able to pass in a member function in place of a regular function.

// Client.cpp
void Client::signal_cb(struct ev_loop *loop, ev_signal *w, int revents) {
    ev_break(loop, EVBREAK_ALL);
}

// ...

void Client::do_connect(struct ev_loop *loop, struct ev_timer *w, int revents) {
    //Godot::print("Attempt MQTT Start!!!\n");
    //write_log("debug", "MQTT Wrapper - Attempt MQTT Start!!!");
    struct umqtt_client *cl; // Move to Class Access (Private)?

    cl = umqtt_new(loop, cfg.host, cfg.port, cfg.ssl);
    if (!cl) {
        //Godot::print("Failed To Create Client!!!\n");
        //write_log("debug", "MQTT Wrapper - Failed To Create Client!!!");
        start_reconnect(loop);
        return;
    }

    //Godot::print("Setup Client Callbacks!!!\n");
    //write_log("debug", "MQTT Wrapper - Setup Client Callbacks!!!");

    // For StackOverflow: These cl->... lines do not work because of not being able to pass a member function as a regular function. These are the main callbacks I have trouble with.
    // How do I convert from `void (libumqtt::Client::*)(struct umqtt_client *)` to `void (*)(struct umqtt_client *)`?
    cl->on_net_connected = Client::on_net_connected; // Pass member function as a non-static object
    cl->on_conack = Client::on_conack; // Pass member function as a non-static object
    cl->on_suback = Client::on_suback; // Pass member function as a non-static object
    cl->on_unsuback = Client::on_unsuback; // Pass member function as a non-static object
    cl->on_publish = Client::on_publish; // Pass member function as a non-static object
    cl->on_pingresp = Client::on_pingresp; // Pass member function as a non-static object
    cl->on_error = Client::on_error; // Pass member function as a non-static object
    cl->on_close = Client::on_close; // Pass member function as a non-static object

    //Godot::print("MQTT Start!!!\n");
    //write_log("debug", "MQTT Wrapper - MQTT Start!!!");
}

void Client::initialize() {
    // For StackOverflow: These two lines cannot work in an object as libev expects signal_cb and do_connect to be regular functions. These callbacks are also necessary, but I am not sure how to handle this.
    ev_signal_init(&signal_watcher, Client::signal_cb, SIGINT);
    ev_timer_init(&reconnect_timer, Client::do_connect, 0.1, 0.0); // Fix Me - Make ev.h object

    // ...
}

Edit: I should mention I am a noob at using C and C++. The most I've done in it before is testing a buffer overflow. So, if their's anything I am obviously doing wrong, I would appreciate the tip in the comments.

Alexis Evelyn
  • 304
  • 5
  • 17
  • 1
    See this answer from a pthreads question: https://stackoverflow.com/questions/1151582/pthread-function-from-a-class/1151615#1151615 Pthreads is a different C API, of course, but the calling pattern and therefore the suggested answer is the same, i.e. pass in a pointer-to-your-C++-object as the `void *` value, and in your C callback function, cast that void-pointer back to the appropriate C++-object-pointer-type, and then call the appropriate non-static method using the casted pointer. (Making the C callback function a static method of the C++ class allows it to call a private method) – Jeremy Friesner Dec 10 '19 at 01:03
  • 1
    Can you provide a code example of the problematic call? Probably what was commented above applies to this case, but am not fully sure I understand your particular situation from the question. – walnut Dec 10 '19 at 01:08
  • 1
    Side note: Please don't put `struct` before the type in a variable declaration in C++ code ("elaborated type specifier"). It is completely redundant (except if you use names that are declared as type *and* variable, which probably should never happen), but if you make a mistake and forget to declare the type early enough, an elaborated type specifier will declare a class with the given name in sometimes surprising scopes instead of causing a "undeclared identifier" error. – walnut Dec 10 '19 at 02:29

1 Answers1

1

So the issue is that umqtt_client does not seem to provide any way of passing additional user data to the callback (the void* mentioned in your quote). It expects the callback to take just a pointer to the umqtt_client instance. (I may be wrong here, I am basing this just on a quick look at the source files.)


If your member functions don't actually access any non-static member of your class, then you can simply make them static. Then you can use them directly as normal function pointers.


Otherwise you need to obtain a pointer to your instance from the umqtt_client* pointer. One way of doing this would be to maintain a static map between the pointers, e.g. in Client add a declaration:

static std::map<umqtt_client*, Client*> umqtt_client_map;

and insert into it when creating a Client (I will assume here that you actually maintain the cl pointer as class member of Client), preferably in Client's constructor:

umqtt_client_map[cl] = this;

Then in Client's destructor (or where ever the umqtt_client object is destroyed) erase the corresponding element from the map:

umqtt_client_map.erase(cl);

Then you can use a lambda looking like this to pass as callback:

cl->on_net_connected = [](umqtt_client* ptr){
    umqtt_client_map[ptr]->on_net_connected();
};

Note that on_net_connected won't need the pointer as argument if it is a member of the class.

This also assumes that you make the class non-copyable and non-movable or that you implement the copy- and move-constructor and -assignment operators with the correct semantics of erasing and inserting into umqtt_client_map as well.


The library seems to offer a function umqtt_init instead of umqtt_new that doesn't allocate the umqtt_client object. If you use that instead you could do the following:

Wrap the umqtt_client in a small standard-layout class:

struct umqtt_client_wrapper {
    umqtt_client cl; // must be first member!
    Client* client;
    static_assert(std::is_standard_layout_v<umqtt_client_wrapper>);
};

You would then use that as member of Client instead of umqtt_client* directly and initialize the umqtt_client* with umqtt_init) andclientwiththisinClient`'s constructor. Then you can use a cast in the lambda for the callback:

cl->on_net_connected = [](umqtt_client* ptr) {
    reinterpret_cast<umqtt_client_wrapper*>(ptr)->client->on_net_connected();
};

Note that this depends on umqtt_client_wrapper being standard-layout and that umqtt_client* is its first member. Not following these rules will cause undefined behavior. The static_assert gives some assurance that at least part of it is not accidentally violated. It requires #include<type_traits> and C++17 in the form that I used here.

Again this requires special care to implement the copy- and move- special member functions of Client correctly or to delete them, but with this method no action in the destructor is required.

This approach is more performant than the other one and in principle you could avoid the extra Client pointer if you make sure that Client itself is standard-layout, but that is probably too restrictive and risky.


Another way, saving the extra indirection, is to use the wrapper as a base class of Client:

struct umqtt_client_wrapper {
    umqtt_client cl; // must be first member!    
    static_assert(std::is_standard_layout_v<umqtt_client_wrapper>);
};

Then let Client inherit from umqtt_client_wrapper and you can use:

cl->on_net_connected = [](umqtt_client* ptr) {
    static_cast<Client*>(reinterpret_cast<umqtt_client_wrapper*>(ptr))
        ->on_net_connected();
};

Note that here the first cast must be static_cast, otherwise you could easily cause undefined behavior.

The same remarks as before apply.

walnut
  • 21,629
  • 4
  • 23
  • 59
  • How would I handle putting the `this` pointer? When I put it in `[this](umqtt_client* ptr)`, I get an error about the lambda being incompatible with the function `error: assigning to 'void (*)(struct umqtt_client *)' from incompatible type '(lambda at src/Client.cpp:127:28)'`. Before doing that, I get a message about how `this` pointer cannot be implicit. – Alexis Evelyn Dec 10 '19 at 08:55
  • @AlexisEvelyn The whole point is to avoid having to use `this`. Instead you use `umqtt_client_map[ptr]` or `reinterpret_cast(ptr)->client`. (You can assign them to a pointer variable first if you want to.) – walnut Dec 10 '19 at 09:57