RTEMS | Improve event record handling (!282)

Gedare Bloom (@gedare) gitlab at rtems.org
Wed Oct 30 04:47:35 UTC 2024




Gedare Bloom started a new discussion: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/282#note_114006


I will try to take a pass through the code changes soon, but I wanted to give my first impression is that this is a big API-level change to be trying to get in before the release of 6.1. It's not clear to me though which of these changes are for external use, or for the internal implementation. 

The API changes I noticed:
- new function `rtems_record_fetch_recommended_item_count()`
- new function `rtems_record_fetch_initialize()`
- new function `rtems_record_fetch()`
- new structure `rtems_record_fetch_control`
- new type (enum) `rtems_record_fetch_status`
- structure name change `uptime` to `rtems_record_client_uptime` (this is an improvement to avoid polluting the global namespace)
- return type change for `rtems_record_client_init`
- increment the `RTEMS_RECORD_THE_VERSION` with associated restructuring of the `rtems_record_event` structure

And the deprecated and obsoleted call:
- `rtems_record_drain`
- `rtems_record_writev`

It is unusual to directly deprecate and obsolete an API call, but, it seems this function was not documented and was not being used by the external tooling, so I'm willing to let that slide. It should still be noted and treated as a deprecation, even if it was a "hidden interface" just on the chance someone happened to be using it for something.

Relatedly, it would help as part of this refactoring if you can keep the public API cleanly defined in installed `cpukit/include/record*.h` headers, and hide the implementation details a bit more. This API leaks a lot of implementation details and doesn't offer a great abstraction for creating the client-side tools, as seen in the associated https://gitlab.rtems.org/rtems/tools/rtems-tools/-/merge_requests/36/ which basically duplicates a lot of this MR. It's not clear to me why the header files are being duplicated between rtems.git and rtems-tools.git, instead of having the tool pick up the header from an installed build. This expectation should be documented within the header file.

I would like to see some better separation between what you view as the "stable API" of the `rtems_record_xxx` interface, and what may be a bit more flexible. It is good the tooling is updated also, but if someone created their own tools based on the Record Interface, those would become badly broken. That is unlikely but I'm taking a more careful view about API changes these days. 

I can't tell if these changes need updates to the documentation in `user/tracing/eventrecording.rst` also, as I cannot tell what parts of the header files are internal and what parts are for the external/tools use.

-- 
View it on GitLab: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/282#note_114006
You're receiving this email because of your account on gitlab.rtems.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/bugs/attachments/20241030/014ad7e0/attachment-0001.htm>


More information about the bugs mailing list