[riot-notifications] [RIOT-OS/RIOT] sock/async: add function to retrieve session object of current DTLS event (#15755)

Aiman Ismail notifications at github.com
Wed Jan 13 01:29:09 CET 2021


@pokgak commented on this pull request.



> @@ -87,6 +87,7 @@ static int _read(struct dtls_context_t *ctx, session_t *session, uint8_t *buf,
     sock->buffer.session = session;
 #ifdef SOCK_HAS_ASYNC
     if (sock->async_cb != NULL) {
+        memcpy(&sock->async_cb_session, session, sizeof(session_t));

The `memcpy` seems unnecessary if the sessions are going to be used in the handler anyways because the handler is called directly after by `sock->async_cb`, so we don't have to worry about the memory of `session` going out of scope. A pointer would be enough. 

If the server wants to keep that information session after the handler exits, only then do the session need to be copied but that should be done by the application (dtls-client/server) and not `sock_dtls`.

> @@ -87,6 +87,7 @@ static int _read(struct dtls_context_t *ctx, session_t *session, uint8_t *buf,
     sock->buffer.session = session;
 #ifdef SOCK_HAS_ASYNC
     if (sock->async_cb != NULL) {
+        memcpy(&sock->async_cb_session, session, sizeof(session_t));

Though by not copying the session into the sock, `sock_dtls_get_event_session` would only be usable withing the async handler, as outside of it there is no guarantee that the pointer is valid and therefore should set to `NULL` when exiting the handler. This behavior should still solve the issue you're having in #15517 where you cannot access the session from the handler. Is there any uses for `sock_dtls_get_event_session` outside of the handler?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/15755#pullrequestreview-566802383
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20210112/2c12a58e/attachment.htm>


More information about the notifications mailing list