auto-dispose, reconnects, readcdr, static analysis, refactoring

[eboasson]

This is terribly large PR and not at all following the general rule that one should not mix unrelated things in a single PR. It is rather bad to not give a good example, but I have nonetheless decided to do it this way:

• Each commit (potentially) builds on top of the preceding and so it doesn’t really make life simpler anyway considering that it all ends up in my lap anyway.
• A good number of the small commits that are in here were the result of a discovery at some point, then inserted at the appropriate point in the history to have each commit a working (verified on Fedora 32 with the address sanitizer enabled).
• A large large part of it comes from two other PRs that I would have merged into master had I not held back for other reasons. They've been sitting there for long enough now, and with the 0.6.0 release process under way, ROS2 Dashing not yet having moved to releases/0.5.x branch, and the preparations for merging security support into master it really makes more sense to merge these into the security branch.

I welcome anyone commenting on it. I'd recommend looking at individual commits ... In particular I'd be much obliged if @MarcelJordense could find some time to look at: d2ca446, 9579461, cacd39a and 50fa943; and if
@dennis-adlink would find time to look at: b0a19ef, 816d016 because I think they know that code well enough to do meaningful review without taking an inordinate amount of time.

What follows is a summary:

Part the First

In which #499 and #505 are moved over to the security branch. They are very nearly identical: a few types have been given a better name, a few unsigneds got changed to uint32_ts and a complete mess-up in one of the intermediate commits in #499 such that it wouldn’t even compile. For those who like to read diffs of diffs comparing them is easy enough ...

In short, from #499:

• improve the tracing of sample content by distinguishing between unsigned and signed integers and floating-point numbers
• bring sanity to the implementation of dds_read_instance (and similar)
• add dds_readcdr (this affects the RHC plugin interface)
• refactor & reduce code duplication in the implementation of reading/taking
• strip out some wrapper functions that served no purpose in the RHC implementation

and #505:

• saner behaviour of auto-dispose when there are multiple active writers: do the auto-dispose when the instance would otherwise have gone to the NOT_ALIVE_NO_WRITERS state
• make dds_lookup_instance on a built-in topic work instead of crash because of a missing implementation of the from_sample operation …
• various bits of refactoring

(This part ends at "Rework listener tests" 24ccb39)

Part the Second

In which the security branch went for looking cover. It was there that a few mouldy spots were discovered. A number of them could have developed into real problems so it is a good thing that this happened. That’s:

d2ca4467 Vet lengths of property lists in crypto deserialization
b0a19ef2 Fix double free if local identity validation fails on certificate expiry
816d0160 Memory leak reading access control configuration
95794611 Atomic update next heartbeat time for p2p writers
cacd39a7 Fix leak when inserting p2p message in WHC fails
50fa943b Fix conversion of user_data to security plugin
9e827098 Properly pair entity_pin/mutex_lock

Part the Third

In which the reworked listener tests were subjected to a bit of further rework: it makes vastly more sense to have an init/step/fini set of functions so that this little monster can be paired with custom code. No doubt there is more work to be done (e.g., turn the individual operations into nice functions so you can use them in more contexts, some of them might come in useful).

This then led to a few more things:

• The instance state is now set to ALIVE when a sample is rejected (too old, doesn’t pass the filter), which means at least transient-local data will always go back to ALIVE after a reconnect.
• It turns out that after a disconnect, rediscovery is blocked for 10s. For no good reason that I can think of. (Being able to block it for a while can be useful, but not for doing so by default.)
• The dds_merge_listeners function changed from copying each listener from the source to the destination if and only if the destination is clear (for that listener) and the source is set (for that listener). Before, it’d copy it if the destination was clear, regardless of whether the source was set. The subtle difference in lies in the listener arguments, and I think this makes more sense. (Need to add set/get stuff that includes the argument …)
• A macro DDS_STATUS_ID_MAX got added to the interface that makes life easier for anyone wanting to range over all status IDs.
• The implementation of dds_wait_for_acks is now capable of waiting for acknowledgements from a specific reader, though this is not currently exposed in the interface. It’s worth considering, but there needs to be a compelling reason to do so, I’d say.

Part the Fourth

In which Clang’s static analyzer in its latest incarnation picked up a few dead stores (all eliminated) and GCC 10’s complained about a few things more. This does not yet eliminate all warnings from GCC 10.

[eboasson]

Thanks @MarcelJordense