Prevent double registration in device framework

The device framework code will log an error if more than one
function is registered for the same operation/access/target
combination.

Change-Id: Id5136c389250ed26d7b62ff3b71116bba54ceb89
RTC: 38760
Reviewed-on: http://gfw160.austin.ibm.com:8080/gerrit/805
Tested-by: Jenkins Server
Reviewed-by: A. Patrick Williams III <iawillia@us.ibm.com>

7 files changed, 219 insertions, 93 deletions

diff --git a/src/include/usr/devicefw/devfwreasoncodes.H b/src/include/usr/devicefw/devfwreasoncodes.H
index 2f471eeb5..6d3c26476 100644
--- a/src/include/usr/devicefw/devfwreasoncodes.H
+++ b/src/include/usr/devicefw/devfwreasoncodes.H
@@ -37,6 +37,7 @@ namespace DeviceFW
         DEVFW_RC_INVALID_REGISTRATION   = DEVFW_COMP_ID | 0x01,
         DEVFW_RC_NO_ROUTE_FOUND         = DEVFW_COMP_ID | 0x02,
         DEVFW_RC_NULL_TARGET            = DEVFW_COMP_ID | 0x03,
+        DEVFW_RC_DOUBLE_REGISTRATION    = DEVFW_COMP_ID | 0x04,
     };
 };
 
diff --git a/src/include/usr/devicefw/driverif.H b/src/include/usr/devicefw/driverif.H
index 939190459..74843e4f4 100644
--- a/src/include/usr/devicefw/driverif.H
+++ b/src/include/usr/devicefw/driverif.H
@@ -59,7 +59,8 @@ namespace DeviceFW
         READ = 0,
         WRITE,
 
-        LAST_OP_TYPE
+        LAST_OP_TYPE,
+        FIRST_OP_TYPE = READ
 
     };
 
diff --git a/src/usr/devicefw/assoccontain.H b/src/usr/devicefw/assoccontain.H
index 127057698..1d500e07a 100644
--- a/src/usr/devicefw/assoccontain.H
+++ b/src/usr/devicefw/assoccontain.H
@@ -41,7 +41,7 @@ namespace DeviceFW
      *  @brief  Data about a particular association.
      *
      *  Typically this is a offset (pointer) to another association and an
-     *  optional flag for the associatior to use.  This allows a user to
+     *  optional flag for the associator to use.  This allows a user to
      *  construct a light-weight multi-dimensional sparse array.
      *
      *  The purpose of the flag field is left to the user.
diff --git a/src/usr/devicefw/associator.C b/src/usr/devicefw/associator.C
index 60a0895e0..7bc3ac28b 100644
--- a/src/usr/devicefw/associator.C
+++ b/src/usr/devicefw/associator.C
@@ -50,10 +50,10 @@ namespace DeviceFW
         TRACFCOMP(g_traceBuffer, EXIT_MRK "Associator::~Associator");        
     }
 
-    void Associator::registerRoute(int64_t i_opType,
-                                   int64_t i_accType,
-                                   int64_t i_targetType,
-                                   deviceOp_t i_regRoute)
+    errlHndl_t Associator::registerRoute(int64_t i_opType,
+                                         int64_t i_accType,
+                                         int64_t i_targetType,
+                                         deviceOp_t i_regRoute)
     {
         TRACFCOMP(g_traceBuffer, "Device route registered for (%d, %d, %d)",
                   i_opType, i_accType, i_targetType);
@@ -63,6 +63,43 @@ namespace DeviceFW
         // No assert-checks will be done here.
 
         mutex_lock(&iv_mutex);
+
+        // Make sure we aren't doing a double registration
+        for( OperationType optype = FIRST_OP_TYPE;
+             optype < LAST_OP_TYPE;
+             optype = static_cast<OperationType>(optype+1) )
+        {
+            if( (WILDCARD == i_opType) || (optype == i_opType) )
+            {
+                deviceOp_t l_devRoute = findDeviceRoute( optype,
+                                    static_cast<TARGETING::TYPE>(i_targetType),
+                                    i_accType );
+                if( l_devRoute != NULL )
+                {
+                    TRACFCOMP(g_traceBuffer, "Double registration attempted : i_opType=%d, i_accType=%d, i_targetType=0x%X, existing function=%p", i_opType, i_accType, i_targetType, l_devRoute );
+                    /*@ 
+                     *  @errortype
+                     *  @moduleid         DEVFW_MOD_ASSOCIATOR
+                     *  @reasoncode       DEVFW_RC_DOUBLE_REGISTRATION
+                     *  @userdata1[0:31]  OpType
+                     *  @userdata1[32:63] AccessType
+                     *  @userdata2        TargetType
+                     *
+                     *  @devdesc         A double registration was attempted 
+                     *                   with the routing framework.
+                     */
+                    errlHndl_t l_errl = 
+                      new ErrlEntry(ERRL_SEV_INFORMATIONAL,
+                                    DEVFW_MOD_ASSOCIATOR,
+                                    DEVFW_RC_DOUBLE_REGISTRATION,
+                                    TWO_UINT32_TO_UINT64(i_opType, i_accType),
+                                    TO_UINT64(i_targetType)
+                                    );
+                    mutex_unlock(&iv_mutex);
+                    return l_errl;
+                }
+            }
+        }
         
         size_t ops = 0;
         AssociationData targets = AssociationData();
@@ -71,7 +108,7 @@ namespace DeviceFW
         ops = iv_associations[iv_routeMap][i_accType].offset;
         if (0 == ops)
         {
-                // space for LAST_OP_TYPE plus WILDCARD(-1).
+            // space for LAST_OP_TYPE plus WILDCARD(-1).
             ops = iv_associations.allocate(LAST_OP_TYPE + 1) + 1;
             iv_associations[iv_routeMap][i_accType].offset = ops;
         }
@@ -93,31 +130,6 @@ namespace DeviceFW
             }
             iv_associations[ops][i_opType] = targets;
         }
-            // Ensure the right block size was allocated previously for this
-            // target-type (wildcard vs non-wildcard).
-        if(((targets.flag) && (i_targetType != WILDCARD)) ||
-           ((!targets.flag) && (i_targetType == WILDCARD)))
-        {
-	    TRACFCOMP(g_traceBuffer, "Invalid registration type was given to register a device : i_opType=%d, i_accType=%d, i_targetType=%d", i_opType, i_accType, i_targetType );
-            /*@ 
-             *  @errortype
-             *  @moduleid       DEVFW_MOD_ASSOCIATOR
-             *  @reasoncode     DEVFW_RC_INVALID_REGISTRATION
-             *  @userdata1      (OpType << 32) | (AccessType)
-             *  @userdata2      TargetType
-             *
-             *  @devdesc        An invalid registration type was given to 
-             *                  register a device with the routing framework.
-             */
-            errlHndl_t l_errl = 
-                new ErrlEntry(ERRL_SEV_INFORMATIONAL,
-                              DEVFW_MOD_ASSOCIATOR,
-                              DEVFW_RC_INVALID_REGISTRATION,
-                              TWO_UINT32_TO_UINT64(i_opType, i_accType),
-                              TO_UINT64(i_targetType)
-                             );
-            errlCommit(l_errl,DEVFW_COMP_ID);
-        }
 
         // Index offset to proper target type.  This is now lowest level of map.
         targets.offset += (i_targetType == WILDCARD ? 0 : i_targetType);
@@ -134,14 +146,14 @@ namespace DeviceFW
             opLocation = iv_operations.end() - 1;
         }
 
-        // TODO: Implement std::distance algorithm and change to:
-        //      std::distance(iv_operations.begin(), opLocation);
-        size_t opLoc = opLocation - iv_operations.begin();
+        size_t opLoc = std::distance(iv_operations.begin(), opLocation);
 
         // Set function offset into map.  True flag indicates valid.
         (*iv_associations[targets.offset]) = AssociationData(true, opLoc);
 
         mutex_unlock(&iv_mutex);
+
+        return NULL;
     }
 
     errlHndl_t Associator::performOp(OperationType i_opType,
@@ -179,15 +191,60 @@ namespace DeviceFW
         TRACDCOMP(g_traceBuffer, "Device op requested for (%d, %d, %d)",
                   i_opType, i_accessType, l_devType);
 
+        mutex_lock(&iv_mutex);
+
+        // Function pointer found for this route request.
+        deviceOp_t l_devRoute = findDeviceRoute( i_opType,
+                                                 l_devType,
+                                                 i_accessType );
+
+        mutex_unlock(&iv_mutex);        
+        
+        // Call function if one was found, create error otherwise.
+        if (NULL == l_devRoute)
+        {
+	    TRACFCOMP(g_traceBuffer, "A device driver operation was attempted for which no driver has been registered : i_opType=%d, i_accessType=%d, l_devType=%d", i_opType, i_accessType, l_devType );
+            /*@
+             *  @errortype
+             *  @moduleid       DEVFW_MOD_ASSOCIATOR
+             *  @reasoncode     DEVFW_RC_NO_ROUTE_FOUND
+             *  @userdata1      (OpType << 32) | (AccessType)
+             *  @userdata2      TargetType
+             *
+             *  @devdesc        A device driver operation was attempted for
+             *                  which no driver has been registered.
+             */
+            l_errl = new ErrlEntry(ERRL_SEV_INFORMATIONAL,
+                                   DEVFW_MOD_ASSOCIATOR,
+                                   DEVFW_RC_NO_ROUTE_FOUND,
+                                   TWO_UINT32_TO_UINT64(i_opType, i_accessType),
+                                   TO_UINT64(l_devType)
+                                  );
+        }
+        else
+        {
+            l_errl = (*l_devRoute)(i_opType, i_target, 
+                                   io_buffer, io_buflen,
+                                   i_accessType, i_addr);
+        }
+
+        return l_errl;
+    }
+
+    
+    deviceOp_t Associator::findDeviceRoute( OperationType i_opType,
+                                            TARGETING::TYPE i_devType,
+                                            int64_t i_accessType )
+    {
         // The ranges of the parameters should all be verified by the
-        // compiler due to the template specializations in driverif.H.  
+        // compiler due to the template specializations in driverif.H.
+        // e.g. i_accessType can never be negative
         // No assert-checks will be done here.
-        
-        mutex_lock(&iv_mutex);
 
-            // Function pointer found for this route request.
+        // Function pointer found for this route request.
         deviceOp_t l_devRoute = NULL;
-            // Pointer to root of the map.
+
+        // Pointer to root of the map.
         const AssociationData* routeMap = iv_associations[iv_routeMap];
 
         do
@@ -199,7 +256,7 @@ namespace DeviceFW
             }
 
             const AssociationData* ops = 
-                iv_associations[routeMap[i_accessType].offset];
+              iv_associations[routeMap[i_accessType].offset];
 
             // Check op type = WILDCARD registrations.
             if (0 != ops[WILDCARD].offset)
@@ -208,23 +265,23 @@ namespace DeviceFW
                 if (ops[WILDCARD].flag)
                 {
                     l_devRoute = 
-                        iv_operations[
+                      iv_operations[
                             iv_associations[ops[WILDCARD].offset]->offset];
                     break;
                 }
-                
+
                 // Check access type = i_target->type registrations.
                 const AssociationData* targets =
-                    iv_associations[ops[WILDCARD].offset];
-                if (targets[l_devType].flag)
+                  iv_associations[ops[WILDCARD].offset];
+                if (targets[i_devType].flag)
                 {
                     l_devRoute =
-                        iv_operations[
-                            targets[l_devType].offset];
+                      iv_operations[
+                                    targets[i_devType].offset];
                     break;
                 }
             }
-            
+
             // Check op type = i_opType registrations.
             if (0 != ops[i_opType].offset)
             {
@@ -232,55 +289,25 @@ namespace DeviceFW
                 if(ops[i_opType].flag)
                 {
                     l_devRoute =
-                        iv_operations[
+                      iv_operations[
                             iv_associations[ops[i_opType].offset]->offset];
                     break;
                 }
-                
+
                 // Check access type = i_target->type registrations.
                 const AssociationData* targets =
-                    iv_associations[ops[i_opType].offset];
-                if (targets[l_devType].flag)
+                  iv_associations[ops[i_opType].offset];
+                if (targets[i_devType].flag)
                 {
                     l_devRoute =
-                        iv_operations[
-                            targets[l_devType].offset];
+                      iv_operations[
+                                    targets[i_devType].offset];
                     break;
                 }
             }
         } while(0);
 
-        mutex_unlock(&iv_mutex);        
-        
-        // Call function if one was found, create error otherwise.
-        if (NULL == l_devRoute)
-        {
-	    TRACFCOMP(g_traceBuffer, "A device driver operation was attempted for which no driver has been registered : i_opType=%d, i_accessType=%d, l_devType=%d", i_opType, i_accessType, l_devType );
-            /*@
-             *  @errortype
-             *  @moduleid       DEVFW_MOD_ASSOCIATOR
-             *  @reasoncode     DEVFW_RC_NO_ROUTE_FOUND
-             *  @userdata1      (OpType << 32) | (AccessType)
-             *  @userdata2      TargetType
-             *
-             *  @devdesc        A device driver operation was attempted for
-             *                  which no driver has been registered.
-             */
-            l_errl = new ErrlEntry(ERRL_SEV_INFORMATIONAL,
-                                   DEVFW_MOD_ASSOCIATOR,
-                                   DEVFW_RC_NO_ROUTE_FOUND,
-                                   TWO_UINT32_TO_UINT64(i_opType, i_accessType),
-                                   TO_UINT64(l_devType)
-                                  );
-        }
-        else
-        {
-            l_errl = (*l_devRoute)(i_opType, i_target, 
-                                   io_buffer, io_buflen,
-                                   i_accessType, i_addr);
-        }
-
-        return l_errl;
+        return l_devRoute;
     }
 }
 
diff --git a/src/usr/devicefw/associator.H b/src/usr/devicefw/associator.H
index f803926c2..7d462e537 100644
--- a/src/usr/devicefw/associator.H
+++ b/src/usr/devicefw/associator.H
@@ -66,16 +66,31 @@ namespace DeviceFW
             ~Associator();
             
                 /** Register routing interface.  See deviceRegisterRoute. */
-            void registerRoute(int64_t i_opType,
-                               int64_t i_accType,
-                               int64_t i_targetType, 
-                               deviceOp_t i_regRoute);
+            errlHndl_t registerRoute(int64_t i_opType,
+                                     int64_t i_accType,
+                                     int64_t i_targetType, 
+                                     deviceOp_t i_regRoute);
                 
                 /** Perform routing.  See deviceOp. */
             errlHndl_t performOp(OperationType i_opType,
                                  TARGETING::Target* i_target,
                                  void* io_buffer, size_t& io_buflen,
                                  int64_t i_accessType, va_list i_addr);
+
+        private:
+            /**
+             * @brief Find an associated function for the given operation
+             *
+             * @param[in] i_opType  Enumeration specifying the operation type
+             * @param[in] i_accessType  Enumeration specifying the access type
+             * @param[in] i_devType  Enumeration specifying the target type
+             *
+             * @return NULL if none found, else a function pointer
+             */
+            deviceOp_t findDeviceRoute( OperationType i_opType,
+                                        TARGETING::TYPE i_devType,
+                                        int64_t i_accessType );
+
         private:
             typedef std::vector<deviceOp_t> opVector_t;
             
diff --git a/src/usr/devicefw/driverif.C b/src/usr/devicefw/driverif.C
index 89aeb4ca0..2536bddf0 100644
--- a/src/usr/devicefw/driverif.C
+++ b/src/usr/devicefw/driverif.C
@@ -25,6 +25,7 @@
  */
 #include <devicefw/driverif.H>
 #include <util/singleton.H>
+#include <errl/errlmanager.H>
 
 #include "associator.H"
 
@@ -42,10 +43,15 @@ namespace DeviceFW
                                       int64_t i_targetType,
                                       deviceOp_t i_regRoute)
     {
-        Singleton<Associator>::instance().registerRoute(i_opType,
-                                                        i_accessType,
-                                                        i_targetType,
-                                                        i_regRoute);
+        errlHndl_t errhdl =
+          Singleton<Associator>::instance().registerRoute(i_opType,
+                                                          i_accessType,
+                                                          i_targetType,
+                                                          i_regRoute);
+        if( errhdl )
+        {
+            errlCommit(errhdl,DEVFW_COMP_ID);
+        }
     }
 
     // deviceRegisterRoute:
diff --git a/src/usr/devicefw/test/associatortest.H b/src/usr/devicefw/test/associatortest.H
index c3ed5ab30..f54734858 100644
--- a/src/usr/devicefw/test/associatortest.H
+++ b/src/usr/devicefw/test/associatortest.H
@@ -113,6 +113,82 @@ public:
     }
 
     /**
+     * @test Verify we catch double registration.
+     */
+    void testDoubleRegistration()
+    {
+        errlHndl_t l_errl = NULL;
+
+        // Register non-sensical SCOM-READ to Nodes
+        Associator as;
+        l_errl = as.registerRoute(READ,
+                                  SCOM,
+                                  TYPE_NODE,
+                                  &performOperation);
+        if (l_errl)
+        {
+            TS_FAIL("testDoubleRegistration> Error received from registerRoute (1).");
+        }
+
+        // Register the exact same thing again
+        l_errl = as.registerRoute(READ,
+                                  SCOM,
+                                  TYPE_NODE,
+                                  &performOperation);
+
+        if (l_errl)
+        {
+            // error log is expected case, delete it
+            delete l_errl;
+        }
+        else
+        {
+            TS_FAIL("testDoubleRegistration> No error from duplicate registration.");
+        }
+
+        // Register a wildcard that overlaps
+        l_errl = as.registerRoute(WILDCARD,
+                                  SCOM,
+                                  TYPE_NODE,
+                                  &performOperation);
+
+        if (l_errl)
+        {
+            // error log is expected case, delete it
+            delete l_errl;
+        }
+        else
+        {
+            TS_FAIL("testDoubleRegistration> No error from wildcard registration.");
+        }
+
+        // Reverse the wildcard test
+        l_errl = as.registerRoute(WILDCARD,
+                                  PNOR,
+                                  TYPE_NODE,
+                                  &performOperation);
+        if (l_errl)
+        {
+            TS_FAIL("testDoubleRegistration> Error received from registerRoute (2).");
+        }
+        l_errl = as.registerRoute(WRITE,
+                                  PNOR,
+                                  TYPE_NODE,
+                                  &performOperation);
+        if (l_errl)
+        {
+            // error log is expected case, delete it
+            delete l_errl;
+        }
+        else
+        {
+            TS_FAIL("testDoubleRegistration> No error from registration after wildcard.");
+        }
+
+
+    }
+
+    /**
      * @test Verify registration with an operator as a WILDCARD.
      */
     void testOpWildcard()