[PATCH] top: Resolve compile errors.

Gedare Bloom gedare at rtems.org
Mon Nov 3 14:12:11 UTC 2014


On Mon, Nov 3, 2014 at 7:58 AM, Jennifer Averett
<jennifer.averett at oarcorp.com> wrote:
> ---
>  cpukit/libmisc/cpuuse/cpuusagetop.c | 83 ++++++++++++++++++++++++++++---------
>  1 file changed, 63 insertions(+), 20 deletions(-)
>
> diff --git a/cpukit/libmisc/cpuuse/cpuusagetop.c b/cpukit/libmisc/cpuuse/cpuusagetop.c
> index 7e7348a..ecafd00 100644
> --- a/cpukit/libmisc/cpuuse/cpuusagetop.c
> +++ b/cpukit/libmisc/cpuuse/cpuusagetop.c
> @@ -30,13 +30,11 @@
>  #include <rtems/score/todimpl.h>
>  #include <rtems/score/watchdogimpl.h>
>
> -
>  /*
>   * Common variable to sync the load monitor task.
>   */
>  static volatile int rtems_cpuusage_top_thread_active;
>
> -
>  typedef struct {
>    void                  *context;
>    rtems_printk_plugin_t  print;
> @@ -44,13 +42,56 @@ typedef struct {
>
>  #define RTEMS_CPUUSAGE_TOP_MAX_LOAD_TASKS (20)
>
> +
> +static inline bool Equal_to_uint32_t( uint32_t * lhs, uint32_t * rhs )
> +{
> +   if ( *lhs == *rhs )
> +     return true;
> +   else
> +     return false;
> +}
> +
> +static inline void Set_uint32_t( uint32_t * lhs, uint32_t * rhs )
> +{
> +   *lhs = *rhs;
> +}
> +
> +static inline bool Less_than_uint32_t( uint32_t * lhs, uint32_t * rhs )
> +{
> +   if ( *lhs < *rhs )
> +    return true;
> +   else
> +    return false;
> +}


What is the purpose for these functions? If they're only used in the
macros below, you could probably just write them directly into the
macro body. Otherwise, I'd prefer they use all lower-case names for
consistent readability. [In the future, it may be sensible to include
a way to distinguish 'private' (static-local) functions and variables,
but for now we have no such convention, so we should stick to naming
things in the usual way.]

> +
> +#ifndef __RTEMS_USE_TICKS_FOR_STATISTICS__
> +  #define _Thread_CPU_usage_Equal_to( _lhs, _rhs ) \
> +          _Timestamp_Equal_to( _lhs, _rhs )
> +#else
> +  #define _Thread_CPU_usage_Equal_to( _lhs, _rhs ) \
> +          Equal_to_uint32_t( _lhs, _rhs )
(*_lhs == *_rhs)

> +#endif
> +
> +#ifndef __RTEMS_USE_TICKS_FOR_STATISTICS__
> +#define  _Thread_CPU_usage_Set_to_zero( _time ) \
> +         _Timestamp_Set_to_zero( _time )
> +#else
> +#define  _Thread_CPU_usage_Set_to_zero( _time ) \
> +         Set_uint32_t( _time, 0 );
This is almost certainly wrong, and could be replaced by
*_time = 0

The function Set_uint32_t defined above takes pointers, passing 0 (or
any literal integer) and treating it as a pointer, then derefencing
it, is bad.

> +#endif
> +
> +#ifndef __RTEMS_USE_TICKS_FOR_STATISTICS__
> +#define _Thread_CPU_usage_Less_than( _lhs, _rhs ) \
> +        _Timestamp_Less_than( _lhs, _rhs )
> +#else
> +#define _Thread_CPU_usage_Less_than( _lhs, _rhs ) \
> +         Less_than_uint32_t( _lhs, _rhs )
(*_lhs < *_rhs)

> +#endif
> +
>  /*
>   * rtems_cpuusage_top_thread
>   *
> - *  DESCRIPTION:
> - *
>   * This function displays the load of the tasks on an ANSI terminal.
> - *
>   */
>
>  static void
> @@ -62,25 +103,29 @@ rtems_cpuusage_top_thread (rtems_task_argument arg)
>    int                       j;
>    int                       k;
>    Objects_Information*      information;
> -    char                    name[13];
> +  char                      name[13];
>    int                       task_count = 0;
>    uint32_t                  seconds, nanoseconds;
>    rtems_cpu_usage_plugin_t* plugin = (rtems_cpu_usage_plugin_t*)arg;
>    Thread_Control*           load_tasks[RTEMS_CPUUSAGE_TOP_MAX_LOAD_TASKS + 1];
> -  unsigned long long        load[RTEMS_CPUUSAGE_TOP_MAX_LOAD_TASKS + 1];
> +  Thread_CPU_usage_t        load[RTEMS_CPUUSAGE_TOP_MAX_LOAD_TASKS + 1];
> +  Thread_CPU_usage_t        zero;
> +  Timestamp_Control         uptime;
> +  uint32_t                  ival, fval;
>
> -  while (true)
> -  {
> +  while (true) {
>      #ifndef __RTEMS_USE_TICKS_FOR_STATISTICS__
> -      Timestamp_Control  uptime, total, ran, uptime_at_last_reset;
> +      Timestamp_Control  total, ran, uptime_at_last_reset;
>      #else
>        uint32_t           total_units = 0;
>      #endif
>
>      rtems_cpuusage_top_thread_active = 1;
>
> +    _Thread_CPU_usage_Set_to_zero( &zero);
>      memset (load_tasks, 0, sizeof (load_tasks));
> -    memset (load, 0, sizeof (load));
> +    for (i=0; i< (RTEMS_CPUUSAGE_TOP_MAX_LOAD_TASKS + 1); i++)
> +      _Thread_CPU_usage_Set_to_zero( &load[i] );
>
>     /*
>       * Iterate over the tasks and sort the highest load tasks
> @@ -98,8 +143,7 @@ rtems_cpuusage_top_thread (rtems_task_argument arg)
>          for ( k=1 ; k <= information->maximum ; k++ ) {
>            the_thread = (Thread_Control *)information->local_table[ k ];
>            if ( the_thread ) {
> -
> -            Thread_CPU_usage_t l = the_thread->cpu_time_used;
> +            Thread_CPU_usage_t usage = the_thread->cpu_time_used;
>
>              /*
>               *  When not using nanosecond CPU usage resolution, we have to count
> @@ -107,14 +151,15 @@ rtems_cpuusage_top_thread (rtems_task_argument arg)
>               *  guideline as to what each number means proportionally.
>               */
>              #ifdef __RTEMS_USE_TICKS_FOR_STATISTICS__
> -              total_units += l;
> +              total_units += usage;
>              #endif
>
>              /* Count the number of tasks and sort this load value */
>              task_count++;
>              for (i = 0; i < RTEMS_CPUUSAGE_TOP_MAX_LOAD_TASKS; i++) {
>                if (load_tasks[i]) {
> -                if ((l == 0) || (l < load[i]))
> +                if ( _Thread_CPU_usage_Equal_to( &usage, &zero) ||
> +                     _Thread_CPU_usage_Less_than( &usage, &load[i]))
>                    continue;
>                  for (j = (RTEMS_CPUUSAGE_TOP_MAX_LOAD_TASKS - 1); j >= i; j--){
>                    load_tasks[j + 1] = load_tasks[j];
> @@ -122,7 +167,7 @@ rtems_cpuusage_top_thread (rtems_task_argument arg)
>                  }
>                }
>                load_tasks[i] = the_thread;
> -              load[i]  = l;
> +              load[i]  = usage;
>                break;
>              }
>            }
> @@ -153,13 +198,12 @@ rtems_cpuusage_top_thread (rtems_task_argument arg)
>         #ifndef __RTEMS_USE_TICKS_FOR_STATISTICS__
>          " ID         | NAME                | RPRI | CPRI   | SECONDS       | PERCENT\n"
>         #else
> -         " ID        | NAME                | RPRI | CPRI   | TICKS         | PERCENT\n"
> +        " ID         | NAME                | RPRI | CPRI   | TICKS         | PERCENT\n"
>         #endif
>         "------------+---------------------+---------------+---------------+------------\n"
>      );
>
> -    for (i = 0; i < RTEMS_CPUUSAGE_TOP_MAX_LOAD_TASKS; i++)
> -    {
> +    for (i = 0; i < RTEMS_CPUUSAGE_TOP_MAX_LOAD_TASKS; i++) {
>
>        if (!load_tasks[i])
>          break;
> @@ -183,7 +227,6 @@ rtems_cpuusage_top_thread (rtems_task_argument arg)
>        #ifndef __RTEMS_USE_TICKS_FOR_STATISTICS__
>        {
>          Timestamp_Control last;
> -        uint32_t          ival, fval;
>
>          /*
>           * If this is the currently executing thread, account for time
> --
> 1.8.1.4
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel


More information about the devel mailing list