[riot-notifications] [RIOT-OS/RIOT] sys/net/gnrc/tx_sync: new module (#15694)

Marian Buschsieweke notifications at github.com
Tue Jan 12 09:12:25 CET 2021


@maribu commented on this pull request.



> -            _free(pkt->data);
+            if (!IS_USED(MODULE_GNRC_TX_SYNC)
+                || (pkt->type != GNRC_NETTYPE_TX_SYNC)) {
+                _free(pkt->data);
+            }
+            else {
+                gnrc_tx_complete(pkt);
+            }

It is actually not needed to be implemented here. However, there are some advantages of putting it here:

- The point when the TX buffer is freed can safely be considered as TX has completed (or failed) and requires no changes in the code elsewhere
- Having separate "TX done" and "free buffer" commands increases the chances of bugs
    - It is more likely to forget one of them (especially since "TX done" is not enabled by default), e.g. within error handling
    - The order of those does matter, as freeing before signaling completion would result in use after free bugs. But that might not be obvious to users.
- Freeing the buffers and reporting errors / success is already bundled in the API. To me, the point when you report success or errors is implicitly the point when TX is completed. But also it would become infeasible to use mount `gnrc_neterr` on top of `gnrc_tx_sync` without changing the error reporting API.

-- 
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/15694#discussion_r555581416
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20210112/85c936bc/attachment.htm>


More information about the notifications mailing list