enet
enet copied to clipboard
Update enet_packet_destroy documentation to reflect usage
I was working on a project and kept getting a segfault until i read deeper into the docs to know that enet_packet_send takes ownership of the packet. It might be beneficial to document that nearer the destroy function, but honestly I'm not sure the best way to clarify.
The tutorial mentions:
Once the packet is handed over to ENet with enet_peer_send(), ENet will handle its deallocation and enet_packet_destroy() should not be used upon it.
It is somewhat misleading to say that enet_peer_send handles the destruction, since it merely queues the packet for sending (which is also why it can't already be destroyed).
I do agree that it may be helpful to make a note about this in general. Though, people could read the tutorial as well before making up their own usage pattern. :-)
It is mentioned, but at least in my case i was working through a codebase that was messy in other ways and did leak memory as time went on, so I was looking for unfreed memory already. enet_peer_send also doesn't really address that it takes ownership of the packet in its documentation.
Maybe the right thing would be to just add a comment to the functions that take ownership of the packet's memory?
Current behavior in ENet 1.3.17 is:
- If
enet_peer_sendsucceeds (returned 0), it has taken ownership of theENetPacketthat usercode passed. Usercode should not deallocate the packet afterwards, or do anything else with it. - If
enet_peer_sendfails (returned something < 0), it has not taken ownership of the packet. Usercode still owns the packet. Usercode should manually deallocate now withenet_packet_destroy, or perhaps wait and re-send the sameENetPacketlater.
There are several possible routes forward:
- Explain in the doxygen that
enet_peer_sendowns the packet if and only if it returned 0, and suggest that usercode, accordingly, (not) callenet_packet_destroyon that packet. This is what this PR aims for, at least for the success case. - Amend the tutorial: Its example code should check the return value and deallocate the packet on failure. The nearby paragraph that explains
enet_peer_send's deallocation behavior should also say when it deallocates. - Change the implementation of
enet_peer_sendto always assume ownership, even on failure to send. But this is a breaking change: I've written a wrapper aroundenet_peer_sendthat deallocates the packet on failure. Such wrappers would break.
What's best? I'm happy to write my own PR for the docs.
I believe that library tutorials should, in general, be rock-solid production-ready code that people should be able to readily copy-and-paste. For the record, eihrul disagreed in IRC:
<SimonN> Where should I send a pull request for the tutorial?
Tutorial code should ideally be production-ready,
people will happily copy & paste.
<eihrul__> it's not meant to be copy and pasted
<SimonN> In practice, people will do it though.
<eihrul__> the goal is still to be a tutorial
and not to be a complicated example of a real world application