Possible error in coremutex.c

[Joel Sherrill]

Is this the diff you are proposing?


Index: cpukit/score/src/coremutex.c
===================================================================
RCS file: /usr1/CVS/rtems/cpukit/score/src/coremutex.c,v
retrieving revision 1.31
diff -u -r1.31 coremutex.c
--- cpukit/score/src/coremutex.c    29 Jul 2008 02:21:15 -0000    1.31
+++ cpukit/score/src/coremutex.c    15 May 2009 17:28:30 -0000
@@ -63,7 +63,7 @@
     the_mutex->holder     = _Thread_Executing;
     the_mutex->holder_id  = _Thread_Executing->Object.id;
     if ( _CORE_mutex_Is_inherit_priority( &the_mutex->Attributes ) ||
-         _CORE_mutex_Is_priority_ceiling( &the_mutex->Attributes ) )
+         _CORE_mutex_Is_priority_ceiling( &the_mutex->Attributes ) ) {
      
 #ifdef __RTEMS_STRICT_ORDER_MUTEX__
        _Chain_Prepend_unprotected( &_Thread_Executing->lock_mutex,
@@ -72,6 +72,7 @@
 #endif
 
       _Thread_Executing->resource_count++;
+    }
   } else {
     the_mutex->nest_count = 0;
     the_mutex->holder     = NULL;


============================================

If so, my reading of the code agrees with you. I would like to hear
the opinion of others.

--joel
Manuel Coutinho wrote:
>
> Hi
>
>  
>
> Was seeing the differences between using and not using 
> __RTEMS_STRICT_ORDER_MUTEX__ and stumbled in what (I) find to be a 
> possible bug.
>
>  
>
> The coremutex.c has the following:
>
>  
>
>   if ( initial_lock == CORE_MUTEX_LOCKED ) {
>
>     the_mutex->nest_count = 1;
>
>     the_mutex->holder     = _Thread_Executing;
>
>     the_mutex->holder_id  = _Thread_Executing->Object.id;
>
>     if ( _CORE_mutex_Is_inherit_priority( &the_mutex->Attributes ) ||
>
>          _CORE_mutex_Is_priority_ceiling( &the_mutex->Attributes ) )
>
>      
>
> #ifdef __RTEMS_STRICT_ORDER_MUTEX__
>
>        _Chain_Prepend_unprotected( &_Thread_Executing->lock_mutex,
>
>                                    &the_mutex->queue.lock_queue );
>
>        the_mutex->queue.priority_before = 
> _Thread_Executing->current_priority;
>
> #endif
>
>  
>
>       _Thread_Executing->resource_count++;
>
>   } else {
>
>     the_mutex->nest_count = 0;
>
>     the_mutex->holder     = NULL;
>
>     the_mutex->holder_id  = 0;
>
>   }
>
>  
>
>  
>
>  
>
> In the older RTEMS versions, the
>
>  
>
> _Thread_Executing->resource_count++;
>
>  
>
> operation was inside the “if”
>
>  
>
> if ( _CORE_mutex_Is_inherit_priority( &the_mutex->Attributes ) ||
>
>          _CORE_mutex_Is_priority_ceiling( &the_mutex->Attributes ) )
>
>  
>
> and now, if the strict order is defined, it is not.
>
>  
>
>  
>
>  
>
> My guess is that the 2 operations of the strict order mutex and the 
> resource_count++ operation must be inside the “if”, because it makes 
> sense to do those operations with priority ceiling or inherence protocols.
>
> Like:
>
>  
>
>   if ( initial_lock == CORE_MUTEX_LOCKED ) {
>
>     the_mutex->nest_count = 1;
>
>     the_mutex->holder     = _Thread_Executing;
>
>     the_mutex->holder_id  = _Thread_Executing->Object.id;
>
>     if ( _CORE_mutex_Is_inherit_priority( &the_mutex->Attributes ) ||
>
>          _CORE_mutex_Is_priority_ceiling( &the_mutex->Attributes ) ){ 
> *<<<---- added bracket*
>
>      
>
> #ifdef __RTEMS_STRICT_ORDER_MUTEX__
>
>          _Chain_Prepend_unprotected( &_Thread_Executing->lock_mutex,
>
>                                    &the_mutex->queue.lock_queue );
>
>          the_mutex->queue.priority_before = 
> _Thread_Executing->current_priority;
>
> #endif
>
>  
>
>         _Thread_Executing->resource_count++;
>
>       }  *<<<---- added bracket*
>
>   } else {
>
>     the_mutex->nest_count = 0;
>
>     the_mutex->holder     = NULL;
>
>     the_mutex->holder_id  = 0;
>
>   }
>
>  
>
>  
>
> That is, add the missing brackets.
>
>  
>
> Should the brackets be added?
>
>  
>
> Kind regards
>
> Manuel Coutinho
>


-- 
Joel Sherrill, Ph.D.             Director of Research & Development
joel.sherrill at OARcorp.com        On-Line Applications Research
Ask me about RTEMS: a free RTOS  Huntsville AL 35805
   Support Available             (256) 722-9985