[PATCH] top: Resolve compile errors.

Joel Sherrill joel.sherrill at oarcorp.com
Mon Nov 3 14:44:18 UTC 2014



On November 3, 2014 8:12:11 AM CST, Gedare Bloom <gedare at rtems.org> wrote:
>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.

Without addressing this, the code did not compile on some targets. The representation of CPU usage statistics is selected by the port or user.

Jennifer can address your style comments


>
>> +
>> +#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
>_______________________________________________
>devel mailing list
>devel at rtems.org
>http://lists.rtems.org/mailman/listinfo/devel



More information about the devel mailing list