Casting to structure via size_t

Question

Where could you see the fallacy of this approach?

I will explain: in the api_method_add method, I add the User data to the "work_list" list and return a "pointer" to the added list item. BUT, since the User who called my api does not have access to the definition of the worrk_struct structure, I cannot return a direct pointer to the worrk_struct structure to him - so I return the addresses to the beginning of the structure placed in the size_t variable.

And when the user calls the "api_method_set" method, he passes a pointer to the beginning of the structure in which he wants to change the data, and I already bring this passed address to the beginning of the structure to the real structure "work_struct". What could be the problem with such an approach?

struct user_struct
{
     float float_;
     double double_;
};

class my_class
{

public:

     size_t api_method_add(user_struct user_struct_)
     {
         work_list.emplace_back();

         (work_list.back()).user_struct_ = user_struct_;

         return (size_t)&work_list.back();
     }


     void api_method_set(size_t pointer)
     {
         std::cout << ((work_struct*)pointer)->user_struct_.double_ << std::endl;
     }


private:

     struct work_struct
     {
         user_struct user_struct_;
         int int_;
         std::string string_;
     };

     std::list<work_struct>work_list;
};




int main()
{

     user_struct user_struct_;
     user_struct_.double_ = 777;

     my_class my_class_;

     size_t pointer = my_class_.api_method_add(user_struct_);

     my_class_.set(pointer);
}

You might consider creating a typesafe handle type that is exposed to the user instead, that would reduce the risk of them passing something else instead — UnholySheep, Jun 03 '23 at 08:17
Make a std::list to access common values. In general it is a bad design to have a list with mixed datatypes (and figuring out at runtime what you actually got, by casting to something else). Do not do it. So maybe you can explain us WHAT you want to do? — Pepijn Kramer, Jun 03 '23 at 08:17
@John3136 Blindly using `void*` will lose typesafety is you delegate your errors to runtime. For me compile time safety is about the most important feature of C++ (detect errors early, preferably at compile time). The only reason for me to use void* would be in type-erasure scenarios where typesafety is guaranteed in other ways) — Pepijn Kramer, Jun 03 '23 at 08:18
@John3136, I can use void* in the principle. If I use void* instead of size_t, will there be any problems? — Ramy Lebo, Jun 03 '23 at 08:19
@Pepijn Kramer, But I like this design. He is comfortable for me. I'm just concerned about the potential real gaps that could happen with this kind of design. — Ramy Lebo, Jun 03 '23 at 08:22
Adding items might seem fine, but how are you going to get anything else then your "base structure" out of your list? And "feeling comfortable" and "it seems to work" are not good enough (in big projects) — Pepijn Kramer, Jun 03 '23 at 08:23
Maybe I didn't quite understand you yet. :) If the class only needs the "index" internally the add method should not have to return anything to the caller. Or you return the index, consider changing list to `std::unordered_map` and insert new items + index in that. — Pepijn Kramer, Jun 03 '23 at 08:32
@Pepijn Kramer, I only need to add my User_struct Data and after I have added an element to the list - I then need to selectively use one of the possible added elements in this list. The user doesn't need the data from the list, it's the class that needs the "index" in the list that it currently needs to use for processing. It turned out to be chaotically explained, but perhaps you did what I had in mind. That is, the task is to tell the class which "work_list" element to use at the time of the call. — Ramy Lebo, Jun 03 '23 at 08:32
You are leaking "internal design here : ` size_t pointer = my_class_.api_method_add(user_struct_); my_class_.set(pointer);` the api_method_add should be private inside your class. — Pepijn Kramer, Jun 03 '23 at 08:34
@Pepijn Kramer, api_method_add - should just be public. It is only for the one who uses my class. Why would I keep it closed? How will the user add data? — Ramy Lebo, Jun 03 '23 at 08:37
@RamyLebo It sounds like you want to pass kind of a opaque handle back through your API, which can be stored by the client for later use to access other functions from your API later, without letting the client need to know (include) the exact implementation internals, right? — πάντα ῥεῖ, Jun 03 '23 at 08:46
@RamyLebo Note that public functions can return private internal classes/types using `auto` variables, which the client can use. And no, your design isn't good, rather unstable. IMHO you should go with interfaces, your clients can/need to implement. — πάντα ῥεῖ, Jun 03 '23 at 08:48
If you are going to cast a pointer to an integer it should be `uintptr_t` not `size_t`. `uintptr_t` is guaranteed to be large enough for a pointer (which `size_t` is not). It also more clearly indicates how the code is working, — john, Jun 03 '23 at 08:48
@ramy rather design an abstract interface for the clients, as mentioned. That's the common way to decouple such stuff. — πάντα ῥεῖ, Jun 03 '23 at 08:53
Nit. The preceding and trailing `'_'` characters are bound to cause confusion when mixed and mingled the way you have. — David C. Rankin, Jun 03 '23 at 09:22
If you need to allow for adding and removing objects, it may be undesireable to allow for the pointer of an object that has been removed to refer to a new object that was added after the removal and reuses the memory previously used by the old object. Furthermore if the user happens to know about the meaning of the value returned, they could get access to the data in the internal structs using something like `uintptr_t pointer = ...; auto mem = reinterpret_cast(pointer); mem[0] = 22;` — fabian, Jun 03 '23 at 10:15
There's little point in hiding this kind of info and you may as well just return a pointer to a class forward declared in public headers and only defined in headers the user of the API doesn't get access to. This way you'll get some type safety. You may want to wrap the pointer though to allow for the conversion to a base class handle usually available to defined classes. — fabian, Jun 03 '23 at 10:15
You can return a `work_struct*` to an opaque (to the outside world) `work_struct`. Also, a `std::size_t` isn't the right type for holding a pointer, you ought to be using `std::intptr_t` or `std::uintptr_t`. — Eljay, Jun 03 '23 at 12:19
*"Where could you see the fallacy of this approach?"* -- this seems rather open-ended for a SO question, and answers would be applicable only to you and anyone else who happens to independently write the same code (somewhat unlikely given how specialized the code is). So even though I see three fallacies, of varying severity, I think this question should be improved before it is answered. Could you focus on one aspect at a time (e.g. [casting a pointer to `size_t`](https://stackoverflow.com/q/3567905)), and present that aspect in an abstract manner that is not so closely tied to your project? — JaMiT, Jun 03 '23 at 13:42

score 2 · Answer 1 · answered Jun 03 '23 at 10:35

Apologies for all the confusion in comments. But I finally get what you are doing. And then this would be my solution, completely typesafe (no casting required).

#include <iostream>
#include <unordered_map>

struct user_struct_t
{
    double double_value;
    int int_value;
};

class my_class_t
{
public:
    // nodiscard means caller MUST do something with the return value.
    [[nodiscard]] std::size_t add_data(user_struct_t&& data)
    {
        // increase unique identifier for each structure added
        ++s_id;
        // store the data at that id
        m_work_list[m_id] = data;

        // return the unique identifier (your size_t)
        return m_id;
    }

    void api_method_set(const std::size_t id)
    {
        // use at (to verify id exists!) and if so
        // get data from the list (actually a map)
        auto& data = m_work_list.at(id);
        std::cout << data.double_value << "\n";
    }

private:
    // unique
    std::size_t m_id;
    std::unordered_map<std::size_t, user_struct_t> m_work_list;
};


int main()
{
    my_class_t my_class;
    auto id = my_class.add_data({ 2.0,42 });
    my_class.api_method_set(id);
    return 0;
}

Thanks, yes, this option could be used, but it will be slower than direct access by pointer. — Ramy Lebo, Jun 03 '23 at 11:28
Yes it is a bit slower (not much std::unordered_map is quite fast). And it will be safe against resizing of the map (the map will grow if more items are added so your memory use will have less overhead) — Pepijn Kramer, Jun 03 '23 at 11:55

Casting to structure via size_t

1 Answers1