On Thu Aug 3 15:59:27 2023 +0000, Rémi Bernon wrote:
Sure, we should perhaps unlink the mapping object too. Although that's probably a good thing to do, I'm not sure to see how it makes any difference wrt this `shared_ptr` member, and why we should get rid of this member.
Sorry, I've got a bit sidetracked. Let's return to the main discussion.
why we should get rid of this member.
The reason I brought this up is simple; it's not immediately clear to the reader that both of the following holds:
* We're lending this out to other server components. * That lease is valid *only until* the mapping object is freed.
This leads to a few problems:
1. If the server mapping object gets released too early, it'll take away the mapped VMA with it while it's still borrowed. Later, someone may dereference the dangling mapping pointer, which points to unexpected memory.
2. In general, the caller of `create_shared_mapping()` has no control over the lifetime of the returned mapping pointer, making it less flexible.
3. It's extra overhead for every `struct mapping`, even if most sections aren't created via `create_shared_mapping()`.
This `shared_ptr` member would be more easily justifiable if
- there were some kind of accessor like `void *mapping_get_shared_ptr( struct mapping * );`, and - the client component accesses the shared pointer only through it.
However, it's only ever used to be *freed*. In this case, I think it's better to let the caller of `create_shared_mapping()` free it instead. It gives a more clear picture of ownership management.