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