RTEMS | Improve event record handling (!282)
Gedare Bloom (@gedare)
gitlab at rtems.org
Sat Nov 2 04:31:05 UTC 2024
Merge request https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/282 was reviewed by Gedare Bloom
--
Gedare Bloom started a new discussion on cpukit/include/rtems/record.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/282#note_114212
> + * from the current processor before the next processor is selected.
> + */
> + size_t cpu_todo;
`size_t` usually should be used for sizes with units in bytes. I think this is just a count and it should be `uintxx_t` for some appropriate amount.
--
Gedare Bloom started a new discussion on cpukit/include/rtems/record.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/282#note_114213
> + * rtems_record_fetch().
> + */
> + size_t count;
ditto.
--
Gedare Bloom started a new discussion on cpukit/include/rtems/record.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/282#note_114214
> + * rtems_record_fetch().
> + */
> +size_t rtems_record_fetch_recommended_item_count( void );
same here with `size_t`. What does "recommended" mean? The name of this function is strange, because at first it seems like it should be fetching items. I think add `get` to the name, like, `rtems_record_get_fetch_recommended_item_count()` or `rtems_record_fetch_get_recommended_item_count()` still a bit strange but less likely to be confusing. At any rate, the documentation should be clear what is meant by recommended item count.
--
Gedare Bloom started a new discussion on cpukit/include/rtems/record.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/282#note_114215
> + rtems_record_fetch_control *control,
> + rtems_record_item *items,
> + size_t count
`items` and `count` are not documented. Again use a different type than `size_t`. You could make the `count` an in,out parameter also?
--
Gedare Bloom started a new discussion on cpukit/include/rtems/recordclient.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/282#note_114216
> - uint32_t time_accumulated;
> - } uptime;
> + uint64_t uptime_bt;
`bintime` would be more clear than `bt`
--
Gedare Bloom started a new discussion on cpukit/libtrace/record/record-fetch.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/282#note_114217
> +#define RECORD_FETCH_HEADER_ITEMS 2
> +
> +size_t rtems_record_fetch_recommended_item_count( void )
This appears to be a constant value. Probably the documentation should indicate how this is determined.
--
View it on GitLab: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/282
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/20241102/2343cde4/attachment-0001.htm>
More information about the bugs
mailing list