Passing a function from another class to a function

Question

The XSetErrorHandler() function from X11 lib expects a function.

If I define a function above my main() [in main.cpp] like:

int catch_error(Display *disp, XErrorEvent *xe){return 0;}

Then I do the following:

XSetErrorHandler(catch_error);

It works.

How can I put the catch_error function in a class named WindowHandler and then pass it to XSetErrorHandler();

I tried the following:

WindowHandler window_handler = new WindowHandler();
XSetErrorHandler(WindowHandler::catch_error);
XSetErrorHandler(window_handler->catch_error);
XSetErrorHandler((*window_handler).catch_error);

Error for 1) argument of type "int (WindowHandler::*)(Display *disp, XErrorEvent *xe)" is incompatible with parameter of type "XErrorHandler"

Error for 2) and 3) A pointer to a bound function may only be used to call that fucntion

Thanks for your help!

@tadman It's defined like this (**not static**): `int WindowHandler::catch_error(Display *disp, XErrorEvent *xe){ printf("Error Happened\n"); error_flag = 1; return 0; }` — auto33, Dec 10 '22 at 23:09
@jdigital I wanted to separate it from my main.cpp file and have it in the same class that's responsible for getting the active window and other window properties. The WindowHandler class also uses the error_flag variable to monitor if an error happened — auto33, Dec 10 '22 at 23:19
when the callback function is invoked, there is no object (no "this"), so it's not possible to invoke a class method or access class members. have you tried defining the method as static? — jdigital, Dec 10 '22 at 23:24
@jdigital Thanks for your input. The error does disappear when defining the function as static. Can a static function still work if you never declared an instance of the object? Because catch_error uses the int error_flag variable, which wouldn't exist if I never created the window_handler object right? — auto33, Dec 10 '22 at 23:30
you can use a static method even if the object has not been instantiated. static methods cannot access non-static class members. if error_flag is a class member, is will also need to be static. — jdigital, Dec 10 '22 at 23:32

joergbrech · Accepted Answer · 2022-12-11T07:39:08.333

The problem is that a non-static member function expects an implicit first argument, which is a pointer or a reference to the class (in this case WindowHandler). So the signature differs from the free catch_error function, if we take the implicit argument into account.

I don't know how XSetErrorHandler is declared, but the function might accept functors: In this case, you could wrap the call to WindowHandler::catch_error in a lambda expression that captures a pointer to your WindowHandler instance.

WindowHandler* window_handler = new WindowHandler();
XSetErrorHandler(
    [window_handler](Display *disp, XErrorEvent *xe) {
        return window_handler->catch_error(disp, xe);
    }
);
delete window_handler; // if you really must use new (why would you ever?), please also delete.

Here is some further reading on pointers to member functions: https://isocpp.org/wiki/faq/pointers-to-members

Edit:

I just realized, that XSetErrorHandler does not accept functors, but only function pointers https://tronche.com/gui/x/xlib/event-handling/protocol-errors/XSetErrorHandler.html. In this case, WindowHandler::catch_error must be static. You can only treat a lambda as a function pointer, if it does not capture anything.

If you really want to capture the error_flag in a member of WindowHandler, the only option I see for you is to have one global instance of WindowHandler (a so-called singleton). Then the static catch_error function can set the member of this singleton, no implicit this argument is required.

Note that global variables and singletons are discouraged and should be avoided if possible. There are some situations, where this is not possible. Logging and error handling are often in these scenarios.

#include <iostream>
#include <memory>

struct Display {};
struct XErrorEvent {};

void XSetErrorHandler(
      int (*handler)(Display *, XErrorEvent *)
) {}

class WindowHandler 
{
private:
    // constructor is private: No other class should be allowed to construct instances
    WindowHandler() = default;

    int catch_error_impl(Display*, XErrorEvent*) {
        std::cout << "Error happened\n";
        error_flag = 1;
        return 0;
    }  

public:
    // don't copy or move a singleton
    WindowHandler(WindowHandler const&) = delete;
    WindowHandler(WindowHandler&&) = delete;

    // the global WindowHandler singleton gets instantiated the first time the get_instance_ptr function is called
    static WindowHandler* get_instance_ptr() {
        // calling new in the argument of unique_ptr ctor is ok. memory is freed when the dtor is called
        static auto handler = std::unique_ptr<WindowHandler>(new WindowHandler());
        return handler.get();
    }

    static int catch_error(Display* disp, XErrorEvent* xe) {
        return get_instance_ptr()->catch_error_impl(disp, xe);
    } 

    int error_flag {0};
};

int main()
{
    XSetErrorHandler(WindowHandler::catch_error);
    std::cout << WindowHandler::get_instance_ptr()->error_flag << std::endl;
}

https://godbolt.org/z/MTWcfK3bs

I thought declaring on the heap was preferrable because the stack runs out of space faster. That's why I use new and malloc. I tried putting the lambda function in and get the following error: no suitable conversion function from "lambda [](Display *disp, XErrorEvent *xe)->int" to "XErrorHandler" exists — auto33, Dec 10 '22 at 23:35
Are you using C or C++? In modern C++ you allocate memory on the heap ideally using smart pointers. Regarding the lambda: I just checked that `XSetErrorHandler` does not accept functors, only function pointers. In that case, your member function MUST be static. I will edit the answer accordingly. — joergbrech, Dec 10 '22 at 23:51
Sure, no problem! Note that the singleton exists on the heap, like you wanted. This only makes sense for heavyweight classes with lots of data. If the `WindowHandler` only stores an int there really is no benefit. To the contrary, you add one unnecessary indirection each time you access the instance. For a lightweight `WindowHandler` it would be preferable to create the singleton on the stack. Then the static `get_instance` method should return a reference. — joergbrech, Dec 11 '22 at 00:28
_create the singleton on the stack_: more likely you mean create it as a static, which is a common pattern for singletons. — jdigital, Dec 11 '22 at 01:08
I have a lot to learn after making this post, but I'll be sure to explore each topic here to get a full understanding. Thank again all for your help! — auto33, Dec 11 '22 at 01:48
@jdigital no, though I do create it as static, I actually meant to write "on the stack". See the whole discussion on stack and heap usage under this answer. I wanted to justify the use of a static unique_pointer, which IMHO does not make sense for this simple example. — joergbrech, Dec 11 '22 at 07:06
if you create the singleton on the stack, the space is "freed" when you exit the method, so that won't work for an object that is meant to be global. — jdigital, Dec 11 '22 at 18:19
Even if it's static and you are returning a reference? Are you sure? https://godbolt.org/z/6jzncbhcP — joergbrech, Dec 11 '22 at 18:32
if it's a static, then it's not created on the stack. see https://stackoverflow.com/questions/408670/stack-static-and-heap-in-c/409072#409072 — jdigital, Dec 11 '22 at 18:38
Ah! Thanks for the refresher. I thought static variables also existed either in the stack or on the heap. My bad. — joergbrech, Dec 11 '22 at 18:54

Passing a function from another class to a function

1 Answers1

Edit: