Updated Ada Hardware Interrupt Patch

Arnaud Charlet charlet@adacore.com
Wed Jun 4 14:34:00 GMT 2008


> Then here it is again.

Thanks. Unfortunately this does not seem to take into account recent changes
made in s-interr-vxworks.adb, so needs to be updated (in particular some
comments have been removed/updated, and use of raise...with instead of
Raise_Exception).

Also, I'm not sure the following change is good:
<<
    procedure Notify_Interrupt (Param : System.Address) is
       Interrupt : constant Interrupt_ID := Interrupt_ID (Param);

-      Id : constant SEM_ID := Semaphore_ID_Map (Interrupt);
-
-      Discard_Result : STATUS;
-      pragma Unreferenced (Discard_Result);
-
+      Status : int;
+      pragma Unreferenced (Status);
+      --  ??? shouldn't we test Stat at least in a pragma Assert?
    begin
-      if Id /= 0 then
-         Discard_Result := semGive (Id);
-      end if;
+      Status := Binary_Semaphore_Release (Semaphore_ID_Map (Interrupt));
    end Notify_Interrupt;
>>

The following seems also related:
<<
@@ -1085,18 +1130,13 @@ package body System.Interrupts is
             
             POP.Write_Lock (Self_Id);

-            --  Unassociate the interrupt handler.
-
-            Semaphore_ID_Map (Interrupt) := 0;
-            
             --  Delete the associated semaphore

-            S := semDelete (Int_Sema);
-
-            pragma Assert (S = 0);
+            Status := Binary_Semaphore_Delete (Int_Sema);
             
             --  Set status for the Interrupt_Manager

+            Semaphore_ID_Map (Interrupt) := 0;
             Server_ID (Interrupt) := Null_Task;
             POP.Unlock (Self_Id);
>>

Note sure why you removed the previous comment and moved code around.
I also suspect a missed merge. You also removed a pragma Assert here.

Could you confirm that the fact that you no longer check for Id /= 0 is
an intentional change you did, or an artefact of an incomplete merge ?

There are also comments referencing a non exiting "Stat" variable (should
probably be "Status" instead).

Could you also explain the following change:
<<
       procedure Unbind_Handler (Interrupt : Interrupt_ID) is
-         S : STATUS;
-         use type STATUS;
-
+         Status : int;
+         pragma Unreferenced (Status);
+         --  ??? shouldn't we test Stat at least in a pragma Assert?
       begin
+         --  Hardware interrupt
+
+         Install_Default_Action (HW_Interrupt (Interrupt));
 
          --  Flush server task off semaphore, allowing it to terminate
 
-         S := semFlush (Semaphore_ID_Map (Interrupt));
-         pragma Assert (S = 0);
+         Status := Binary_Semaphore_Flush (Semaphore_ID_Map (Interrupt));
       end Unbind_Handler;
>>

In particular the new call to Install_Default_Action ?

and also, you've removed assertions:

<<
-         S := semTake (Int_Sema, WAIT_FOREVER);
-         pragma Assert (S = 0);
+         Status := Binary_Semaphore_Obtain (Int_Sema);
>>

which does not seem like a good idea to me (and indeed you've added comments
such as: --  ??? shouldn't we test Stat at least in a pragma Assert?)
My answer is "yes".

Arno



More information about the Gcc-patches mailing list