[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