sysrepo / sysrepo-cpp

C++ bindings for the sysrepo library
BSD 3-Clause "New" or "Revised" License
7 stars 6 forks source link

Support for the plugins API #3

Closed dumitrupaul closed 1 year ago

dumitrupaul commented 2 years ago

Hello,

Is there a plan to bring wrappers around the sr_plugin_init_cb and sr_plugin_cleanup_cb ?

The way I'm handling a plugin right now is by initiating a new connection in the init callback and storing a shared_ptr to the Subscription class. Is there a better way of handling this, I was thinking by wrapping the raw session pointer given by the callback (maybe with wrapUnmanagedSession which is not exposed through the API) ?

Kind regards, Paul.

jktjkt commented 2 years ago

We're not using sysrepo-plugind internally, so we do not plan to implement this feature at this time. Patches are welcome, here are instructions for our workflow.

syyyr commented 2 years ago

Hi, as jktjkt said, we probably won't be implementing anytime soon, but the approach with wrapUnmanagedSession would work. It is not a part of the public API, since we don't need it outside of sysrepo-cpp. However, your usecase is a good reason for making it public. You would still have to define the C-style sr_plugin_init_cb functions and then use some sort of a dynamic-allocated context where you would save the Session created by wrapUnmanagedSession. So I imagine (very roughly) something like this:

struct MyCtx {
    sysrepo::Session sess;
    // possibly other fields, like `sysrepo::Subscription sub;`
};

int sr_plugin_init_cb(sr_session_ctx_t *session, void **private_data)
{
    auto myCtx = new MyCtx{wrapUnmanagedSession(session)};
    // now do stuff with myCtx.sess

     *private_data = static_cast<void*>(myCtx);
}

void sr_plugin_cleanup_cb(sr_session_ctx_t *session, void *private_data)
{
    delete static_cast<MyCtx*>(private_data);
}

I am not sure if it would be possible to somehow turn the new/delete into something more C++-ish (like a smart pointer), probably yes. Also, I haven't checked if this is actually correct, but in principle it should be enough.

Anyway, I imagine that at some point, sysrepo-cpp would do something like this for you, via a macro or a template, but for now, what I can do for you is just making the wrapping function public. Will that suffice?

syyyr commented 2 years ago

For now, I have made the function public https://gerrit.cesnet.cz/c/CzechLight/sysrepo-cpp/+/5354

dumitrupaul commented 2 years ago

Thanks, this would certainly help avoiding the creation of a new connection and using the given session. Your example shows how the old version 1 sysrepo cpp bindings used to work, although by using the Session constructor to wrap the raw pointer auto sess = std::make_shared<Session>(session). Nevertheless I think I would give a try to a cleaner cpp approach without using raw pointers and submit a PR.

syyyr commented 2 years ago

Just FYI the wrapUnmanagedSession patch got merged as 60d48794b451b6f58518c422fdb9c31314bed790.

Let me know if you have more quesions.