--- a/packages/kernel/current/ChangeLog
+++ b/packages/kernel/current/ChangeLog
@@ -1,3 +1,32 @@
+2003-07-01  Nick Garnett  <nickg@balti.calivar.com>
+
+	* include/clock.hxx: Made Cyg_Counter::add_alarm() and
+	Cyg_Counter::rem_alarm() private functions. They no longer lock
+	the scheduler, so should not be called directly, only via the
+	Cyg_Alarm functions.
+
+	* include/clock.inl: Removed inline version of
+	Cyg_Alarm::disable(). It's no longer a one-liner and is thus
+	better as a proper function.
+
+	* src/common/clock.cxx (Cyg_Counter::tick): Rewrote the unsorted
+	list option. If the function of one alarm adds or removes other
+	alarms, then is was possible for the list to become corrupted. The
+	new implementation attempts to avoid this problem.
+	(Cyg_Alarm::initialize): Added scheduler locking to protect the
+	enabled flag.
+	(Cyg_Alarm::enable): Ditto.
+	(Cyg_Alarm::disable): Ditto. Moved here from clock.inl.
+	(Cyg_Alarm::add_alarm): Removed scheduler locking, it is now
+	always called from functions that have already locked it. Added an
+	assertion to double-check this.
+	(Cyg_Alarm::rem_alarm): Ditto.
+
+	* cdl/kernel.cdl:
+	* tests/kalarm0.c:
+	Added new test to test that alarms can be added and removed in
+	alarm functions safely.
+	
 2003-06-25  Thomas Binder  <Thomas.Binder@frequentis.com>
 
 	* src/common/clock.cxx (Cyg_Counter::rem_alarm): Bugfix: call

--- a/packages/kernel/current/cdl/kernel.cdl
+++ b/packages/kernel/current/cdl/kernel.cdl
@@ -323,7 +323,7 @@ cdl_package CYGPKG_KERNEL {
             no_define
             calculated { 
                 "tests/bin_sem0 tests/bin_sem1 tests/bin_sem2 tests/bin_sem3 tests/clock0 tests/clock1 tests/clockcnv tests/clocktruth tests/cnt_sem0 tests/cnt_sem1 tests/except1 tests/flag0 tests/flag1 tests/intr0 tests/kill tests/mbox1 tests/mqueue1 tests/mutex0 tests/mutex1 tests/mutex2 tests/mutex3 tests/release tests/sched1 tests/sync2 tests/sync3 tests/thread0 tests/thread1 tests/thread2" 
-                . ((CYGFUN_KERNEL_API_C) ? " tests/kclock0 tests/kclock1 tests/kexcept1 tests/kflag0 tests/kflag1 tests/kintr0 tests/klock tests/kmbox1 tests/kmutex0 tests/kmutex1 tests/kmutex3 tests/kmutex4 tests/ksched1 tests/ksem0 tests/ksem1 tests/kthread0 tests/kthread1 tests/stress_threads tests/thread_gdb tests/timeslice tests/tm_basic tests/fptest" : "")
+                . ((CYGFUN_KERNEL_API_C) ? " tests/kclock0 tests/kclock1 tests/kexcept1 tests/kflag0 tests/kflag1 tests/kintr0 tests/klock tests/kmbox1 tests/kmutex0 tests/kmutex1 tests/kmutex3 tests/kmutex4 tests/ksched1 tests/ksem0 tests/ksem1 tests/kthread0 tests/kthread1 tests/stress_threads tests/thread_gdb tests/timeslice tests/tm_basic tests/fptest tests/kalarm0" : "")
                 . ((!CYGPKG_INFRA_DEBUG && !CYGPKG_KERNEL_INSTRUMENT && CYGFUN_KERNEL_API_C) ? " tests/dhrystone" : "")
                 . ((CYGPKG_KERNEL_SMP_SUPPORT && CYGFUN_KERNEL_API_C) ? " tests/smp" : "")
                 . ((!CYGINT_HAL_TESTS_NO_CACHES && CYGFUN_KERNEL_API_C) ? " tests/kcache1 tests/kcache2" : "")

--- a/packages/kernel/current/include/clock.hxx
+++ b/packages/kernel/current/include/clock.hxx
@@ -91,6 +91,12 @@ class Cyg_Counter
     volatile cyg_tick_count counter;    // counter value
 
     cyg_uint32          increment;      // increment per tick
+
+    // Add an alarm to this counter
+    void add_alarm( Cyg_Alarm *alarm );
+
+    // Remove an alarm from this counter
+    void rem_alarm( Cyg_Alarm *alarm );
     
 public:
 
@@ -116,12 +122,6 @@ public:
     // Advance counter by some number of ticks
     void tick( cyg_uint32 ticks = 1);
 
-    // Add an alarm to this counter
-    void add_alarm( Cyg_Alarm *alarm );
-
-    // Remove an alarm from this counter
-    void rem_alarm( Cyg_Alarm *alarm );
-
 };
 
 // -------------------------------------------------------------------------

--- a/packages/kernel/current/include/clock.inl
+++ b/packages/kernel/current/include/clock.inl
@@ -121,16 +121,5 @@ inline cyg_tick_count Cyg_Clock::convert
 }
 
 // -------------------------------------------------------------------------
-// -------------------------------------------------------------------------
-// Alarm inlines
-
-// Ensure alarm disabled
-inline void Cyg_Alarm::disable()
-{
-    if( enabled ) counter->rem_alarm(this);
-}
-
-
-// -------------------------------------------------------------------------
 #endif // ifndef CYGONCE_KERNEL_CLOCK_INL
 // EOF clock.inl

--- a/packages/kernel/current/src/common/clock.cxx
+++ b/packages/kernel/current/src/common/clock.cxx
@@ -164,7 +164,7 @@ void Cyg_Counter::tick( cyg_uint32 ticks
         // So we only have a fraction of the alarms to check here.
         
         alarm_list_ptr = &(alarm_list[
-            (counter/increment) % CYGNUM_KERNEL_COUNTERS_MULTI_LIST_SIZE ] );
+                               (counter/increment) % CYGNUM_KERNEL_COUNTERS_MULTI_LIST_SIZE ] );
     
 #else
 #error "No CYGIMP_KERNEL_COUNTERS_x_LIST config"
@@ -211,27 +211,30 @@ void Cyg_Counter::tick( cyg_uint32 ticks
         } 
 #else
 
-        // With an unsorted list, we must scan the whole list for
-        // candidates. We move the whole list to a temporary location
-        // before doing this so that we are not disturbed by new
-        // alarms being added to the list. As we consider and
-        // eliminate alarms we put them onto the done_list and at the
-        // end we then move it back to where it belongs.
+        // With unsorted lists we must scan the whole list for
+        // candidates. However, we must be careful here since it is
+        // possible for the function of one alarm to add or remove
+        // other alarms to/from this list. Having the list shift under
+        // our feet in this way could be disasterous. We solve this by
+        // detecting when the list has changed and restarting the scan
+        // from the beginning.
         
-        Cyg_Alarm_List done_list;
-
-        Cyg_Alarm_List alarm_list;
+        Cyg_DNode_T<Cyg_Alarm> *node = alarm_list_ptr->get_head();
 
-        alarm_list.merge( *alarm_list_ptr );
-        
-        while( !alarm_list.empty() )
+        while( node != NULL )
         {
-            Cyg_Alarm *alarm = alarm_list.rem_head();
+            Cyg_Alarm *alarm = CYG_CLASSFROMBASE( Cyg_Alarm, Cyg_DNode, node );
+            Cyg_DNode_T<Cyg_Alarm> *next = alarm->get_next();
 
             CYG_ASSERTCLASS(alarm, "Bad alarm in counter list" );
             
             if( alarm->trigger <= counter )
             {
+                Cyg_DNode *next_next, *next_prev;
+                Cyg_Alarm *head;
+                
+                alarm_list_ptr->remove(alarm);
+
                 if( alarm->interval != 0 )
                 {
                     // The alarm has a retrigger interval.
@@ -242,25 +245,44 @@ void Cyg_Counter::tick( cyg_uint32 ticks
                 }
                 else alarm->enabled = false;
 
+                // Save some details of the list state so we can
+                // detect if it has changed.
+                next_next = next->get_next();
+                next_prev = next->get_prev();
+                head = alarm_list_ptr->get_head();
+                
                 CYG_INSTRUMENT_ALARM( CALL, this, alarm );
                 
-                // call alarm function
+                // Call alarm function
                 alarm->alarm(alarm, alarm->data);
 
-                // all done, loop
-            }
-            else
-            {
-                // add unused alarm to done list.
-                done_list.add_tail(alarm);
+                // If that alarm function has added or removed an
+                // alarm on this list that might affect this scan,
+                // then start again from beginning. The main test here
+                // is whether the link fields of the next node have
+                // changed, implying that it may have been moved. The
+                // test against the list head detects the case where
+                // the next node was the only node in the list, when
+                // moving is would have left the links unchanged.
+                
+                if( next_next != next->get_next()           ||
+                    next_prev != next->get_prev()           ||
+                    head      != alarm_list_ptr->get_head() )
+                {
+                    node = alarm_list_ptr->get_head();
+                    continue;
+                }
             }
+
+            // If the next node is the head of the list, then we have
+            // looped all the way around. The node == next test
+            // catches the case where we only had one element to start
+            // with.
+            if( next == alarm_list_ptr->get_head() || node == next )
+                node = NULL;
+            else
+                node = next;
         }
-
-        // Return done list to real list. If any alarms have been
-        // added to the alarm list while we have been scanning then
-        // the done list will be added behind them.
-        
-        alarm_list_ptr->merge( done_list );
         
 #endif        
         Cyg_Scheduler::unlock();
@@ -278,9 +300,8 @@ void Cyg_Counter::add_alarm( Cyg_Alarm *
 
     CYG_ASSERTCLASS( this, "Bad counter object" );
     CYG_ASSERTCLASS( alarm, "Bad alarm passed" );
+    CYG_ASSERT( Cyg_Scheduler::get_sched_lock() > 0, "Scheduler not locked");
     
-    Cyg_Scheduler::lock();
-
     // set this now to allow an immediate handler call to manipulate
     // this alarm sensibly.
     alarm->enabled = true;
@@ -314,7 +335,6 @@ void Cyg_Counter::add_alarm( Cyg_Alarm *
             // The alarm is all done with, disable it
             // unlock and return.
             alarm->enabled = false;
-            Cyg_Scheduler::unlock();
             return;
         }
     }
@@ -382,7 +402,6 @@ void Cyg_Counter::add_alarm( Cyg_Alarm *
         
 #endif
 
-    Cyg_Scheduler::unlock();            
 }
 
 // -------------------------------------------------------------------------
@@ -394,18 +413,16 @@ void Cyg_Counter::rem_alarm( Cyg_Alarm *
 
     CYG_ASSERTCLASS( this, "Bad counter object" );
     CYG_ASSERTCLASS( alarm, "Bad alarm passed" );
+    CYG_ASSERT( Cyg_Scheduler::get_sched_lock() > 0, "Scheduler not locked");
     
     Cyg_Alarm_List *alarm_list_ptr;     // pointer to list
 
 #if defined(CYGIMP_KERNEL_COUNTERS_SINGLE_LIST)
 
     alarm_list_ptr = &alarm_list;
-    Cyg_Scheduler::lock();
 
 #elif defined(CYGIMP_KERNEL_COUNTERS_MULTI_LIST)
 
-    Cyg_Scheduler::lock();
-
     alarm_list_ptr = &(alarm_list[
         ((alarm->trigger+increment-1)/increment) %
                               CYGNUM_KERNEL_COUNTERS_MULTI_LIST_SIZE ] );
@@ -423,7 +440,6 @@ void Cyg_Counter::rem_alarm( Cyg_Alarm *
     
     alarm->enabled = false;
 
-    Cyg_Scheduler::unlock();            
 }
 
 //==========================================================================
@@ -709,6 +725,8 @@ void Cyg_Alarm::initialize(
 {
     CYG_REPORT_FUNCTION();
 
+    Cyg_Scheduler::lock();
+    
     // If already enabled, remove from counter
     
     if( enabled ) counter->rem_alarm(this);
@@ -725,6 +743,8 @@ void Cyg_Alarm::initialize(
     interval = i;
 
     counter->add_alarm(this);
+
+    Cyg_Scheduler::unlock();    
 }
 
 // -------------------------------------------------------------------------
@@ -759,6 +779,8 @@ Cyg_Alarm::synchronize( void )
 
 void Cyg_Alarm::enable()
 {
+    Cyg_Scheduler::lock();
+    
     if( !enabled )
     {
         // ensure the alarm time is in our future:
@@ -766,6 +788,20 @@ void Cyg_Alarm::enable()
         enabled = true;
         counter->add_alarm(this);
     }
+
+    Cyg_Scheduler::unlock();    
+}
+
+// -------------------------------------------------------------------------
+// Ensure alarm disabled
+
+void Cyg_Alarm::disable()
+{
+    Cyg_Scheduler::lock();
+
+    if( enabled ) counter->rem_alarm(this);
+
+    Cyg_Scheduler::unlock();
 }
 
 // -------------------------------------------------------------------------

new file mode 100644
--- /dev/null
+++ b/packages/kernel/current/tests/kalarm0.c
@@ -0,0 +1,191 @@
+//==========================================================================
+//
+//        kalarm0
+//
+//        Alarm functionality test
+//
+//==========================================================================
+//####ECOSGPLCOPYRIGHTBEGIN####
+// -------------------------------------------
+// This file is part of eCos, the Embedded Configurable Operating System.
+// Copyright (C) 2003 Nick Garnett <nickg@calivar.com>
+//
+// eCos is free software; you can redistribute it and/or modify it under
+// the terms of the GNU General Public License as published by the Free
+// Software Foundation; either version 2 or (at your option) any later version.
+//
+// eCos is distributed in the hope that it will be useful, but WITHOUT ANY
+// WARRANTY; without even the implied warranty of MERCHANTABILITY or
+// FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+// for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with eCos; if not, write to the Free Software Foundation, Inc.,
+// 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+//
+// As a special exception, if other files instantiate templates or use macros
+// or inline functions from this file, or you compile this file and link it
+// with other works to produce a work based on this file, this file does not
+// by itself cause the resulting work to be covered by the GNU General Public
+// License. However the source code for this file must still be made available
+// in accordance with section (3) of the GNU General Public License.
+//
+// This exception does not invalidate any other reasons why a work based on
+// this file might be covered by the GNU General Public License.
+//
+// Alternative licenses for eCos may be arranged by contacting the
+// copyright holders.
+// -------------------------------------------
+//####ECOSGPLCOPYRIGHTEND####
+//==========================================================================
+//#####DESCRIPTIONBEGIN####
+//
+// Author(s):     nickg
+// Contributors:  nickg
+// Date:          2003-06-25
+// Description:   Tests the ability of alarms to be added and removed by the
+//                alarm functions of other alarms.
+//
+//####DESCRIPTIONEND####
+//==========================================================================
+
+#include <cyg/kernel/kapi.h>
+
+#include <cyg/infra/testcase.h>
+
+//==========================================================================
+
+#if defined(CYGVAR_KERNEL_COUNTERS_CLOCK) && \
+    defined(CYGFUN_KERNEL_API_C)
+
+#include "testaux.h"
+
+//==========================================================================
+
+//#define db_printf diag_printf
+#define db_printf( fmt, ... )
+
+//==========================================================================
+
+static cyg_counter counter_obj;
+static cyg_handle_t counter;
+static cyg_alarm alarm_obj[3];
+static cyg_handle_t alarm[3];
+
+static cyg_uint32 alarmfn_called[3];
+
+//==========================================================================
+
+void alarmfn0(cyg_handle_t alarmh, cyg_addrword_t data)
+{
+    db_printf("%s: %d\n",__PRETTY_FUNCTION__,cyg_counter_current_value( counter ));
+
+    // alarmfn0 just counts how many times it has been called
+    
+    alarmfn_called[0]++;
+}
+
+//==========================================================================
+
+void alarmfn1(cyg_handle_t alarmh, cyg_addrword_t data)
+{
+    db_printf("%s: %d\n",__PRETTY_FUNCTION__,cyg_counter_current_value( counter ));
+
+    alarmfn_called[1]++;
+
+    // Reschedule alarm[0] to run every tick until alarm[2] next runs.
+    
+    cyg_alarm_initialize( alarm[0], cyg_counter_current_value( counter )+1, 1 );
+
+}
+
+//==========================================================================
+
+void alarmfn2(cyg_handle_t alarmh, cyg_addrword_t data)
+{
+    db_printf("%s: %d\n",__PRETTY_FUNCTION__,cyg_counter_current_value( counter ));
+    
+    alarmfn_called[2]++;
+
+    // Reschedule alarm[0] to run every 2 ticks until alarm[1] next runs.
+
+    cyg_alarm_initialize( alarm[0], cyg_counter_current_value( counter )+1, 2 );
+
+    // Reschedule alarm[1] to run every 3 ticks starting in 6 ticks time.
+    
+    cyg_alarm_initialize( alarm[1], cyg_counter_current_value( counter )+6, 3 );
+}
+
+//==========================================================================
+
+void alarm0_main(void)
+{
+    int i;
+    
+    CYG_TEST_INIT();
+
+    // Create the counter
+    cyg_counter_create( &counter, &counter_obj );
+
+    // Create the alarms
+    cyg_alarm_create( counter,
+                      alarmfn0,
+                      0,
+                      &alarm[0],
+                      &alarm_obj[0]);
+
+
+    cyg_alarm_create( counter,
+                      alarmfn1,
+                      1,
+                      &alarm[1],
+                      &alarm_obj[1]);
+
+    cyg_alarm_create( counter,
+                      alarmfn2,
+                      2,
+                      &alarm[2],
+                      &alarm_obj[2]);
+
+
+    // Kick it all off by starting alarm[2]
+    cyg_alarm_initialize( alarm[2], 0, 10 );
+
+    // Run the whole thing for 10000 ticks
+    for( i = 0; i < 10000; i++ )
+        cyg_counter_tick( counter );
+
+    db_printf("alarmfn_called: %d %d %d\n",
+                alarmfn_called[0],alarmfn_called[1],alarmfn_called[2]);
+
+    CYG_TEST_CHECK( alarmfn_called[0]==5000, "alarmfn0 not called 5000 times\n");
+    CYG_TEST_CHECK( alarmfn_called[1]==2000, "alarmfn1 not called 2000 times\n");
+    CYG_TEST_CHECK( alarmfn_called[2]==1001, "alarmfn2 not called 1001 times\n");
+    
+    CYG_TEST_PASS_FINISH("KAlarm0");
+}
+
+//==========================================================================
+
+externC void
+cyg_start( void )
+{
+    alarm0_main();
+}
+
+//==========================================================================
+
+#else
+
+externC void
+cyg_start( void )
+{
+    CYG_TEST_INIT();
+    CYG_TEST_NA( "This test needs CYGVAR_KERNEL_COUNTERS_CLOCK "
+                 "and CYGFUN_KERNEL_API_C" );
+}
+
+#endif
+
+//==========================================================================
+// End of kalarm0.c