Hi,
Thanks for the follow up with details. I also put some further thought into
this overnight and totally agree with your observations. With one exception
:)
On Mon, May 8, 2017 at 9:56 AM, David Sommerseth <openvpn@sf.lists.
topphemmelig.net> wrote:
> I think it is a bit too risky to actually fix the plug-in API to fix
> this. So we need to use (openvpn_plugin_handle_t *) in the v3 API.
>
We can fix it by removing the * without breaking the API. And I think that
is the right thing to do. In any case all working plugins that use v3 has
to be using handle (an not *handle) to store the context pointer and then
retrieving it as passed back in the func call.
A code that sets *handle = context_ptr is clearly wrong and would segfault
Any that sets handle = &context_ptr would get the wrong pointer back in
the func call so would not function properly.
So both those usages can be assumed not to exist.
So the only concern would be plugins which set handle =
(openvpn_plugin_handle_t *) context_ptr. That is only a cast issue and will
continue to compile and "work" even if the struct is changed to have
"openvpn_plugin_handle_t handle).
> I'll update the patch accordingly.
>
So I suggest to change the API to define handle in the return struct as
openvpn_plugin_handle_t without the *. It also avoids giving the wrong
impression that one should set handle = &context_ptr in the _open_v3() call.
> [*] By the way, this being an example, it may be best to show the
> > correct type by casting the
> > return value of calloc to (openvpn_plugin_handle_t)
>
> Yes, but shouldn't it be `(openvpn_plugin_handle_t *)` ?
>
I assumed the fix would be to change the struct.
Thanks,
Selva
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
