Converting different classes to void* and back safely in C++

Question

I have a map of callbacks that pass information and execute various functions throughout code, very much like events in C# but in C++.

The map is defined as

std::map<std::string, std::function<void(uint8_t*)>> mCallbacks

It is passed by reference to all subprograms

Then each class binds its callbacks as such

mCallbacks["Status_Label"] = std::bind(&MenuHandler::LabelEditCallback, this, std::placeholders::_1);

Where

bool MenuHandler::LabelEditCallback(uint8_t * m_label_data)
{
    int text_size = ((int*)m_label_text)[0];
    char* v_text = (char*)&m_label_text[1];
}

And each event gets called from a different subprogram like this:

if (mCallbacks.find("Status_Label") != mCallbacks.end())
    mCallbacks.at("Status_Label")((uint8_t*)selected_text);

This makes it easy to pass data and events around the program without making a mess of objects and references

As you can see, this is extremely unsafe, and converting from a uint8_t pointer to various data formats can easily lead to corrupted stack.

The problem is, I don't have a specific structure for callback arguments, some of them may be sending text data, others may be sending numbers.

My solution is to define structs that will be cast to void* when calling the event, and back in the callback function

Something like this (untested):

struct Label_Callback_Data
{
    Label_Callback_Data(std::string v_name, std::string v_text)
    {
        labelName = v_name;
        labelText = v_text;
        size_of = sizeof(this);
    }
    int size_of;
    std::string labelName;
    std::string labelText;
};

And I would call it like this:

if (mCallbacks.find("Status_Label") != mCallbacks.end())
    mCallbacks.at("Status_Label")((uint8_t*)Label_Callback_Data("Status_Label_name", "TEXT"))

But then how would I recover it here? If I dont know the exact size of the object?

bool  MenuHandler::LabelEditCallback(uint8_t * m_label_data)
{
    //??  Label_Callback_Data text_size =  (Label_Callback_Data*)m_label_text
}

One solution is to use object with fixed size arrays, but there has to be a C++11 solution that is safe to use, maybe something using dynamic_pointer_casts?

Also, as a bonus question, how would I know if the object passed to the callback function is smaller in size than it is expecting? Is it possible to check this and just return a false from the callback function so the program doesn't crash?

Thank you, This code is not tested, so there may be logical mistakes I'm willing to correct per responses.

`((int*)m_label_text)[0]` is wrong. If `sizeof(int) == 4` (which is is on all modern 32 and 64 bit computers) then what you're getting is the values of `m_label_text[0]`, `m_label_text[1]`, `m_label_text[2]` and `m_label_text[3]`. You also have the problem of [*endianness*](https://en.wikipedia.org/wiki/Endianness) to think about for such code. If the "size" is stored as an unsigned 8-bit integer in `m_label_text[0]` then just do e.g. `size_t text_size = m_label_text[0];` — Some programmer dude, Jun 25 '19 at 20:23
You can use polymorphism. And your functions use a Base class as parameter. In your class you can use dynamic cast to verify the type — Armin Montigny, Jun 25 '19 at 20:25
casts are always a code smell. instead trying to use `void*` you should try to get rid of casts alltogether. Maybe take a look at `std::any` or `std::variant` — 463035818_is_not_a_number, Jun 25 '19 at 20:25
And unless this is an assignment or exercise I really recommend you take a look at one of the many event-handling frameworks that already exists for C++. Some of which will allow you to handle multiple arguments for the signal-handlers and can handle C++ objects, so you could have your signal handler take a `size_t` argument and a `std::string` object (or reference). See e.g. [the Boost signals 2 library](https://www.boost.org/doc/libs/1_70_0/doc/html/signals2.html). — Some programmer dude, Jun 25 '19 at 20:27
And as mentioned (by @formerlyknownas_463035818) casts, especially C-style casts, are a red flag that you're probably doing something wrong. — Some programmer dude, Jun 25 '19 at 20:28

Remy Lebeau · Accepted Answer · 2019-06-25T20:42:57.663

3

You should generally prefer to use a lambda instead of std::bind().

Try something more like this:

std::map<std::string, std::function<void(void*)>> mCallbacks;

struct Label_Callback_Data
{
    std::string labelName;
    std::string labelText;

    Label_Callback_Data(std::string v_name, std::string v_text)
        : labelName(v_name), labelText(v_text) { }
};

...

mCallbacks["Status_Label"] = [this](void *data){ this->LabelEditCallback(data); };

...

auto iter = mCallbacks.find("Status_Label");
if (iter != mCallbacks.end())
{
    Label_Callback_Data data("Status_Label_name", "TEXT");
    iter->second(&data);
}

...

bool MenuHandler::LabelEditCallback(void *m_label_data)
{
    Label_Callback_Data *data = static_cast<Label_Callback_Data*>(m_label_text);
    // use data->labelName and data->labelText as needed...
}

Alternatively, you could move the type-cast into the lambda itself, so LabelEditCallback() doesn't need to deal with void* at all:

std::map<std::string, std::function<void(void*)>> mCallbacks;

struct Label_Callback_Data
{
    std::string labelName;
    std::string labelText;

    Label_Callback_Data(std::string v_name, std::string v_text)
        : labelName(v_name), labelText(v_text) { }
};

...

mCallbacks["Status_Label"] = [this](void *data){ this->LabelEditCallback(static_cast<Label_Callback_Data*>(data)); };

...

auto iter = mCallbacks.find("Status_Label");
if (iter != mCallbacks.end())
{
    Label_Callback_Data data("Status_Label_name", "TEXT");
    iter->second(&data);
}

...

bool MenuHandler::LabelEditCallback(Label_Callback_Data *m_label_data)
{
    // use m_label_data->labelName and m_label_data->labelText as needed...
}

edited Jun 25 '19 at 20:42

answered Jun 25 '19 at 20:36

Remy Lebeau

454,445
28
366
620

but how would you verify its safe to do a `static_cast` ? Is it going to return a null pointer if it fails to do a static_cast? – Mich Jun 25 '19 at 21:23
`static_cast` is evaluated at compile-time, not runtime. Only `dynamic_cast` returns NULL on a cast failure. By virtue of the fact that there should only be one definition of each `struct` being passed around for each callback, you don't need to validate the sizes if you use the correct structs to begin with (ie, `"Status_Label"` always uses `Label_Callback_Data`, etc). Otherwise, if you want the code to be more dynamic, you will have to pass around struct sizes and/or type IDs as extra parameters to your callbacks. – Remy Lebeau Jun 25 '19 at 21:31
why "should generally prefer lambda" ? I do prefer lambdas over bind, because imho bind leads to terrible code that hurts my eyes and is comparatively difficult to use, while lambdas are more natural, but are there reasons beyond that? – 463035818_is_not_a_number Jun 26 '19 at 08:20
1

@formerlyknownas_463035818 see [Bind Vs Lambda?](https://stackoverflow.com/questions/1930903/) and [Why use std::bind over lambdas in C++14?](https://stackoverflow.com/questions/17363003/) – Remy Lebeau Jun 26 '19 at 09:08
assuming both can be used, the only plus for lambdas I found in one of the answers was "When using bind functions are less likely to be inlined", which is reason enough to prefer lambdas in general i guess – 463035818_is_not_a_number Jun 26 '19 at 10:11

score 1 · Answer 2 · answered Jun 25 '19 at 21:39

This is how I did it

...

//The container

std::map<std::string, std::function<void(std::shared_ptr<CallbackData::BlankData>)>> mCallbacks

...

//CALLBACK FUNCTION
bool InputManager::KeyboardCallback(std::shared_ptr<CallbackData::BlankData> callback_data)
{
    std::shared_ptr<CallbackData::Keyboard> keyboard_data = std::dynamic_pointer_cast<CallbackData::Keyboard>(callback_data);

    if (keyboard_data == nullptr)
        return false;

    ///...
}

...

//CALLBACK EVENT
if (mCallbacks.find("Keyboard") != mCallbacks.end())
{
    std::shared_ptr<CallbackData::Keyboard> keyboard_data = std::make_shared<CallbackData::Keyboard>(m_keyboardState);
    mCallbacks.at("Keyboard")(std::dynamic_pointer_cast<CallbackData::BlankData>(keyboard_data));
}

...

//Data structure    
namespace CallbackData
{
    struct BlankData
    {
        virtual ~BlankData() {};
    };

    struct Keyboard : public BlankData
    {
        Keyboard(uint8_t* kb_data)
        {
            kbData = kb_data;
        }
        uint8_t* kbData;
    };
}

Converting different classes to void* and back safely in C++

2 Answers2