[ECOS] catch errors in eCos's kernel

james chen james_ch1@sina.com
Fri Aug 31 23:01:00 GMT 2001


Hi, I have caught two errors, one in sem_trywait(sem.cxx) and the other is
in Cyg_Condition_Variable::wait_inner(mutex.cxx)

1.
externC int sem_trywait  (sem_t *sem)
{
    int retval = 0;

    SEMA_ENTRY();

    Cyg_Counting_Semaphore *sema = (Cyg_Counting_Semaphore *)sem;

    if( !sema->trywait() ) retval = EAGAIN;

    SEMA_RETURN(0);
    ^^^^^^^^^^^^^^^^^^^
}
This  function  always return 0 if we decrement a semaphore or not, so it
should be  * SEMA_RETURN(retval); *

^^^^^^^^^^^^^^^^^^^^^^^

2.
The function will generate a assert "Unnecessary call to unlock_inner()"
if we call it when timeout has already fired.It occurs because if the
timeout
is in the past, the thread will be woken up immediately and will not sleep.
so we needn't use Cyg_Scheduler::unlock_reschedule() in line 749 to
reschedule.
The lines begin with "+" is my suggestion to add.

cyg_bool
Cyg_Condition_Variable::wait_inner( Cyg_Mutex *mx, cyg_tick_count timeout )
{
    CYG_REPORT_FUNCTYPE("returning %d");
    CYG_REPORT_FUNCARG1("timeout = %d", timeout);

    CYG_ASSERTCLASS( this, "Bad this pointer");
    CYG_ASSERTCLASS( mx, "Corrupt mutex");

    cyg_bool result = true;

    Cyg_Thread *self = Cyg_Thread::self();

    CYG_ASSERTCLASS( self, "Bad self thread");

    // Prevent preemption
    Cyg_Scheduler::lock();

    CYG_INSTRUMENT_CONDVAR(TIMED_WAIT, this, 0 );

    mx->unlock();

    // The ordering of sleep() and set_timer() here are
    // important. If the timeout is in the past, the thread
    // will be woken up immediately and will not sleep.

    self->sleep();

    // Set the timer and sleep reason
    self->set_timer( timeout, Cyg_Thread::TIMEOUT );

    // Only enqueue if the timeout has not already fired.
    if( self->get_wake_reason() == Cyg_Thread::NONE )
        queue.enqueue( self );

    // Avoid calling ASRs during the following unlock.
    self->set_asr_inhibit();

    // Unlock the scheduler and switch threads

   + if( self->get_wake_reason() != Cyg_Thread::NONE )
   + Cyg_Scheduler::unlock;
   + else
        Cyg_Scheduler::unlock_reschedule();

    // Allow ASRs again
    self->clear_asr_inhibit();

    CYG_ASSERTCLASS( this, "Bad this pointer");
    CYG_ASSERTCLASS( mx, "Corrupt mutex");

    self->clear_timer();

    CYG_INSTRUMENT_CONDVAR(WOKE, this, self->get_wake_reason());

    switch( self->get_wake_reason() )
    {
    case Cyg_Thread::TIMEOUT:
    case Cyg_Thread::DESTRUCT:          // which, the cv or the mutex?
    case Cyg_Thread::BREAK:
        result = false;
        break;

    case Cyg_Thread::EXIT:
        self->exit();
        break;

    default:
        break;
    }


    // When we awake, we must re-acquire the mutex.  Note that while
    // it is essential to release the mutex and queue on the CV
    // atomically relative to other threads, to avoid races, it is not
    // necessary for us to re-acquire the mutex in the same atomic
    // action. Hence we can do it after unlocking the scheduler.

    while ( !mx->lock() )
        continue;

    CYG_ASSERTCLASS( this, "Bad this pointer");
    CYG_ASSERTCLASS( mx, "Corrupt mutex");

    CYG_REPORT_RETVAL(result);

    return result;
}




More information about the Ecos-discuss mailing list