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.


_______________________________________________
bugs mailing list
[email protected]
http://lists.rtems.org/mailman/listinfo/bugs

Reply via email to