iox-#64 Raw pointer API for publisher and subscriber

[elBoberido]

Pre-Review Checklist for the PR Author

1. Code follows the Rust coding style and is formatted with rustfmt
2. Branch follows the naming format (iox-123-this-is-a-branch)
3. Commits messages are according to this guideline
   • Commit messages have the issue ID (iox-#42 commit text)
   • Commit author matches Eclipse Contributor Agreement (and ECA is signed)
4. Update the PR title
   • Follow the same conventions as for commit messages
   • Link to the relevant issue
5. Relevant issues are linked
6. Add sensible notes for the reviewer
7. All checks have passed (except task-list-completed)
8. Assign PR to reviewer

Notes for Reviewer

This PR introduces a RawSample and RawSampleMut in order to make the iceoryx-sys API more robust and ease the interoperability with C libraries from the high level API.

Checklist for the PR Reviewer

• Commits are properly organized and messages are according to the guideline
• Code according to our coding style and naming conventions
• PR title describes the changes

Post-review Checklist for the PR Author

1. All open points are addressed and tracked via issues

References

• Closes Raw pointer API for publisher and subscriber #64

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 79.65368% with 47 lines in your changes missing coverage. Please review.

Project coverage is 50.77%. Comparing base (553140e) to head (4d6dd67).

Files with missing lines	Patch %	Lines
iceoryx-sys/src/sample.rs	53.24%	36 Missing ⚠️
src/subscriber.rs	0.00%	6 Missing ⚠️
iceoryx-sys/src/subscriber.rs	93.10%	2 Missing ⚠️
src/sample_mut.rs	95.23%	2 Missing ⚠️
iceoryx-sys/src/publisher.rs	94.73%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #65      +/-   ##
==========================================
+ Coverage   48.27%   50.77%   +2.50%     
==========================================
  Files          19       20       +1     
  Lines        1075     1154      +79     
==========================================
+ Hits          519      586      +67     
- Misses        556      568      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[elBoberido]

@phil-opp it took quite some time but I think it is now ready ... in case you still need it :)

[elfenpiff]

Shouldn't it be mut self here?

[elBoberido]

Hehe, in theory yes but I took inspiration from NonNull::as_ptr and NonNull::as_mut_ptr. Initially I tried to use NonNull instead of an own type but then I discovered that I could not get a *const T out of it and finally replicated NonNull into these two structs. On the other hand, I already deviated from the NonNull implementation by returning a *const T for as_payload_ptr. Just have to think what exactly the reason for the NonNull::as_mut_ptr implementation could be.

[elBoberido]

The counsel with the pillow helped. mut self would even cause a warning. The method does no borrow but takes the ownership of the sample. The question is now why not using a reference? My guess is that taking the ownership and invoking a copy is more performant than using a reference. The struct has the the same size as a raw pointer and therefore passing by value or by reference has the same cost. Using the reference does add an indirection which is unnecessary in this case. It should not be a big penalty but I would keep the design similar to NonNull

[elfenpiff]

What happens when I call release_raw multiple times with the same cloned sample? I think RawSample can be cloned and copied.

I think it should be an unsafe function and the user has to ensure that:

• the sample was not released before
• the sample was taken/acquired via the subscriber it is being released

[elBoberido]

Currently nothing bad happens since the C++ code base takes care of this. iceoryx just prints a warning with [Warning]: ICEORYX error! POPO__CHUNK_SENDER_INVALID_CHUNK_TO_FREE_FROM_USER. Since nothing bad happens I thought it does not need to be unsafe

[elfenpiff]

Ouh, I thought that iceoryx would handle it in a more severe manner. In this particular case, I agree with you but it still feels weird and a little bit wrong.

[elBoberido]

Indeed. There is quite a lot which can go wrong but all is handled in a graceful way. I don't know anymore why we chose this route at that time

[elBoberido]

Btw, I fixed all clippy warnings but I'm hesitant to commit them to this PR since it's adding additional 130 changes in 8 files. So I first wanted to ask if it's okay for you? I can also easily create a separate PR after this one is merged.

Edit: Nevermind, I created a separate PR.

[elBoberido]

@elfenpiff I fixed your findings and I'm going to merge this now. If there are still changes you would like to see I'm happy to create a follow up PT