Updated targeting assertion handling

- Use trace assert feature
- Move assertions out of 'if' checks

Change-Id: I185d8d22fbdb81e62b09f13055e08d1de8d487e2
Reviewed-on: http://gfw160.austin.ibm.com:8080/gerrit/235
Tested-by: Jenkins Server
Reviewed-by: Nicholas E. Bofferding <bofferdn@us.ibm.com>
Reviewed-by: Mark W. Wenning <wenning@us.ibm.com>
Reviewed-by: A. Patrick Williams III <iawillia@us.ibm.com>

3 files changed, 159 insertions, 193 deletions

diff --git a/src/include/usr/targeting/target.H b/src/include/usr/targeting/target.H
index f2764aca8..76e508e27 100644
--- a/src/include/usr/targeting/target.H
+++ b/src/include/usr/targeting/target.H
@@ -171,7 +171,10 @@ class Target
             typename AttributeTraits<A>::Type const& i_attrValue)
         {
             bool l_wrote = trySetAttr<A>(i_attrValue);
-            assert(l_wrote);
+            
+            //@TODO: Remove assert once release has stabilized
+            assert(l_wrote,"TARGETING::Target::setAttr<%d>: trySetAttr "
+                   "returned false",A);
         }
 
     private: // Private helper interfaces
@@ -282,7 +285,10 @@ typename AttributeTraits<A>::Type Target::getAttr() const
 {
     typename AttributeTraits<A>::Type l_attrValue;
     bool l_read = tryGetAttr<A>(l_attrValue);
-    assert(l_read);
+    
+    //@TODO Remove assert once release has stablized 
+    assert(l_read,"TARGETING::Target::getAttr<%d>: tryGetAttr "
+           "returned false",A);
     return l_attrValue;
 }
 
@@ -295,7 +301,10 @@ const char* Target::getAttrAsString() const
     if(AttributeTraits<A>::hasStringConversion) { }
     typename AttributeTraits<A>::Type l_attrValue;
     bool l_read = tryGetAttr<A>(l_attrValue);
-    assert(l_read);
+
+    //@TODO Remove assert once release has stabilized
+    assert(l_read,"TARGETING::Target::getAttrAsString<%d>: tryGetAttr "
+           "returned false",A);
     return attrToString<A>(l_attrValue);
 }
 
diff --git a/src/usr/targeting/entitypath.C b/src/usr/targeting/entitypath.C
index 024c563ee..b93aaaaba 100644
--- a/src/usr/targeting/entitypath.C
+++ b/src/usr/targeting/entitypath.C
@@ -25,6 +25,7 @@ namespace TARGETING
 
 #define TARG_NAMESPACE "TARGETING::"
 #define TARG_CLASS "EntityPath::"
+#define TARG_LOC TARG_NAMESPACE TARG_CLASS TARG_FN ": "
 
 extern trace_desc_t* g_trac_targeting;
 
@@ -76,13 +77,9 @@ EntityPath& EntityPath::removeLast()
 {
     #define TARG_FN "removeLast()"
 
-    if(size() < 1)
-    {
-        TARG_ERR("Entity path empty (%d); cannot remove any path elements",
-                 size());
-        assert(0);
-    }
-
+    assert(size() >= 1, TARG_LOC "EntityPath empty (size = %d); cannot remove "
+           "any path elements", size());
+    
     iv_pathElement[size() - 1].type = TYPE_NA;
     iv_pathElement[size() - 1].instance = 0;
     --iv_size;
@@ -116,12 +113,8 @@ EntityPath& EntityPath::addLast(
 {
     #define TARG_FN "addLast(...)"
 
-    if(size() >= MAX_PATH_ELEMENTS)
-    {
-        TARG_ERR("Entity path cannot store any more path elements with size %d",
-                 size());
-        assert(size() < MAX_PATH_ELEMENTS);
-    }
+    assert(size() < MAX_PATH_ELEMENTS, TARG_LOC "Entity path cannot "
+           "store any more path elements with size %d", size());
 
     iv_pathElement[size()].type = i_type;
     iv_pathElement[size()].instance = i_instance;
@@ -189,12 +182,9 @@ bool EntityPath::equals(
 {
     #define TARG_FN "equals(...)"
 
-    if(i_size > MAX_PATH_ELEMENTS)
-    {
-        TARG_ERR("Caller specified invalid entity path size of %d which "
-                 "is greater than MAX_PATH_ELEMENTS",i_size,size());
-        assert(i_size <= MAX_PATH_ELEMENTS);
-    }
+    assert(i_size <= MAX_PATH_ELEMENTS, TARG_LOC "Caller specified invalid "
+           "entity path size of %d which is greater than MAX_PATH_ELEMENTS of "
+           "%d",i_size,MAX_PATH_ELEMENTS);
 
     return (   (i_rhs.iv_type == iv_type)
             && (i_size <= i_rhs.size())
@@ -215,12 +205,9 @@ const EntityPath::PathElement& EntityPath::operator[](
 {
     #define TARG_FN "operator[](...)"
 
-    if(i_index >= size())
-    {
-        TARG_ERR("Caller specified invalid entity path subscript of %d when "
-                 "size is only %d",i_index,size());
-        assert(0);
-    }
+    assert(i_index < size(), TARG_LOC "Caller specified invalid entity path "
+           "subscript of %d when size is only %d",i_index,size());
+    
     return iv_pathElement[i_index];
 
     #undef TARG_FN
diff --git a/src/usr/targeting/targetservice.C b/src/usr/targeting/targetservice.C
index df713614a..4418358ef 100644
--- a/src/usr/targeting/targetservice.C
+++ b/src/usr/targeting/targetservice.C
@@ -34,6 +34,7 @@ namespace TARGETING
 
 #define TARG_NAMESPACE "TARGETING::"
 
+#define TARG_LOC TARG_NAMESPACE TARG_CLASS TARG_FN ": "
 
 //******************************************************************************
 // _start
@@ -168,18 +169,12 @@ void TargetService::getTopLevelTarget(
     Target*& o_targetHandle) const
 {
     #define TARG_FN "getTopLevelTarget(...)"
+    
+    assert(iv_initialized, TARG_LOC "TargetService not initialized");
 
-    if (iv_initialized)
-    {
-        EntityPath l_topLevelPhysicalPath(EntityPath::PATH_PHYSICAL);
-        l_topLevelPhysicalPath.addLast(TYPE_SYS, 0);
-        o_targetHandle = toTarget(l_topLevelPhysicalPath);
-    }
-    else
-    {
-        o_targetHandle = NULL;
-        assert(iv_initialized);
-    }
+    EntityPath l_topLevelPhysicalPath(EntityPath::PATH_PHYSICAL);
+    l_topLevelPhysicalPath.addLast(TYPE_SYS, 0);
+    o_targetHandle = toTarget(l_topLevelPhysicalPath);
 
     #undef TARG_FN
 }
@@ -195,20 +190,16 @@ void TargetService::exists(
     #define TARG_FN "exists(...)"
 
     bool l_found = false;
-    if (iv_initialized)
+
+    assert(iv_initialized, TARG_LOC "TargetService not initialized");
+
+    for (uint32_t i = 0; i < iv_maxTargets; ++i)
     {
-        for (uint32_t i = 0; i < iv_maxTargets; ++i)
+        if (i_entityPath == (*iv_targets)[i].getAttr<ATTR_PHYS_PATH> ())
         {
-            if (i_entityPath == (*iv_targets)[i].getAttr<ATTR_PHYS_PATH> ())
-            {
-                l_found = true;
-            }
+            l_found = true;
         }
     }
-    else
-    {
-        assert(iv_initialized);
-    }
 
     o_exists = l_found;
 
@@ -227,22 +218,18 @@ Target* TargetService::toTarget(
     // Used by -> operator on EntityPath for convenience (can be dangerous
     // though!)
     Target* l_pTarget = NULL;
-    if (iv_initialized)
+
+    assert(iv_initialized, TARG_LOC "TargetService not initialized");
+
+    for (uint32_t i = 0; i < iv_maxTargets; ++i)
     {
-        for (uint32_t i = 0; i < iv_maxTargets; ++i)
+        if (i_entityPath == (*iv_targets)[i].getAttr<ATTR_PHYS_PATH> ())
         {
-            if (i_entityPath == (*iv_targets)[i].getAttr<ATTR_PHYS_PATH> ())
-            {
-                l_pTarget = &(*iv_targets)[i];
-                break;
-            }
+            l_pTarget = &(*iv_targets)[i];
+            break;
         }
     }
-    else
-    {
-        assert(iv_initialized);
-    }
-
+    
     return l_pTarget;
 
     #undef TARG_FN
@@ -258,22 +245,18 @@ void TargetService::masterProcChipTargetHandle(
     #define TARG_FN "masterProcChipTargetHandle(...)"
 
     Target* l_pTarget = NULL;
-    if (iv_initialized)
-    {
-        //@TODO Need to query the actual hardware and cross check it with
-        // PNOR to determine the master chip
-        // target; for now, just always report sys0.n0.proc0
-        EntityPath l_masterProcChipEntityPath(EntityPath::PATH_PHYSICAL);
-        l_masterProcChipEntityPath.addLast(TYPE_SYS, 0).addLast(TYPE_NODE, 0)
-            .addLast(TYPE_PROC, 0);
-
-        l_pTarget = l_masterProcChipEntityPath.operator->();
-    }
-    else
-    {
-        assert(iv_initialized);
-    }
 
+    assert(iv_initialized, TARG_LOC "TargetService not initialized");
+    
+    //@TODO Need to query the actual hardware and cross check it with
+    // PNOR to determine the master chip
+    // target; for now, just always report sys0.n0.proc0
+    EntityPath l_masterProcChipEntityPath(EntityPath::PATH_PHYSICAL);
+    l_masterProcChipEntityPath.addLast(TYPE_SYS, 0).addLast(TYPE_NODE, 0)
+        .addLast(TYPE_PROC, 0);
+
+    l_pTarget = l_masterProcChipEntityPath.operator->();
+    
     o_masterProcChipTargetHandle = l_pTarget;
 
     #undef TARG_FN
@@ -291,30 +274,25 @@ bool TargetService::tryGetPath(
     #define TARG_FN "tryGetPath(...)"
 
     bool l_exist = false;
-    if (iv_initialized)
-    {
-        switch (i_attr)
-        {
-            case ATTR_PHYS_PATH:
-                l_exist = i_pTarget->tryGetAttr<ATTR_PHYS_PATH> (o_entityPath);
-                break;
-            case ATTR_AFFINITY_PATH:
-                l_exist = i_pTarget->tryGetAttr<ATTR_AFFINITY_PATH> (
-                    o_entityPath);
-                break;
-            case ATTR_POWER_PATH:
-                l_exist = i_pTarget->tryGetAttr<ATTR_POWER_PATH> (o_entityPath);
-                break;
-            default:
-                TARG_ERR("BUG; i_attr = 0x%08X does not map to an entity "
-                         "path");
-                assert(0);
-                break;
-        }
-    }
-    else
+
+    assert(iv_initialized, TARG_LOC "TargetService not initialized");
+
+    switch (i_attr)
     {
-        assert(iv_initialized);
+        case ATTR_PHYS_PATH:
+            l_exist = i_pTarget->tryGetAttr<ATTR_PHYS_PATH> (o_entityPath);
+            break;
+        case ATTR_AFFINITY_PATH:
+            l_exist = i_pTarget->tryGetAttr<ATTR_AFFINITY_PATH> (
+                o_entityPath);
+            break;
+        case ATTR_POWER_PATH:
+            l_exist = i_pTarget->tryGetAttr<ATTR_POWER_PATH> (o_entityPath);
+            break;
+        default:
+            assert(0, TARG_LOC "i_attr = 0x%08X does not map to an "
+                   "entity path",i_attr);
+            break;
     }
 
     return l_exist;
@@ -336,16 +314,13 @@ void TargetService::getAssociated(
 
     do {
 
-    assert(iv_initialized);
-
-    if(   (i_pTarget == NULL)
-       || (i_pTarget == MASTER_PROCESSOR_CHIP_TARGET_SENTINEL) )
-    {
-        TARG_ERR("BUG; caller tried to get association using a NULL target "
-                 "handle or the master processor chip target handle sentinel. "
-                 "i_pTarget = %p",i_pTarget);
-        assert(0);
-    }
+    assert(iv_initialized, TARG_LOC "TargetService not initialized");
+    
+    assert(   (i_pTarget != NULL) 
+           && (i_pTarget != MASTER_PROCESSOR_CHIP_TARGET_SENTINEL),
+           TARG_LOC "Caller tried to get association using a NULL target "
+           "handle or the master processor chip target handle sentinel. "
+           "i_pTarget = %p",i_pTarget);
 
     // Start with no elements
     o_list.clear();
@@ -375,10 +350,10 @@ void TargetService::getAssociated(
                 }
                 else
                 {
-                    TARG_ERR("BUG; iv_associationMappings[i].associationDir "
-                             "= 0x%X not supported",
-                             iv_associationMappings[i].associationDir);
-                    assert(0);
+                    assert(0, TARG_LOC 
+                           "iv_associationMappings[i].associationDir "
+                           "= 0x%X not supported",
+                           iv_associationMappings[i].associationDir);
                 }
             }
             break;
@@ -398,96 +373,91 @@ void TargetService::dump() const
 {
     #define TARG_FN "dump(...)"
 
-    if (iv_initialized)
-    {
-        TARG_INF("Targets (size=%d):",
-                 sizeof(Target)*iv_maxTargets);
+    assert(iv_initialized, TARG_LOC "TargetService not initialized");
 
-        for (uint32_t i = 0; i < iv_maxTargets; ++i)
-        {
-            TARG_INF(
-                "[Target %d] "
-                "Class = 0x%X, "
-                "Type = 0x%X, "
-                "Model = 0x%X",
-                i,
-                (*iv_targets)[i].getAttr<ATTR_CLASS>(),
-                (*iv_targets)[i].getAttr<ATTR_TYPE>(),
-                (*iv_targets)[i].getAttr<ATTR_MODEL>());
-            TARG_INF("Physical");
-            (*iv_targets)[i].getAttr<ATTR_PHYS_PATH>().dump();
+    TARG_INF("Targets (size=%d):",
+             sizeof(Target)*iv_maxTargets);
 
-            EntityPath l_entityPath;
-            if( (*iv_targets)[i].tryGetAttr<ATTR_AFFINITY_PATH>(l_entityPath) )
-            {
-                TARG_INF("Affinity");
-                l_entityPath.dump();
-            }
+    for (uint32_t i = 0; i < iv_maxTargets; ++i)
+    {
+        TARG_INF(
+            "[Target %d] "
+            "Class = 0x%X, "
+            "Type = 0x%X, "
+            "Model = 0x%X",
+            i,
+            (*iv_targets)[i].getAttr<ATTR_CLASS>(),
+            (*iv_targets)[i].getAttr<ATTR_TYPE>(),
+            (*iv_targets)[i].getAttr<ATTR_MODEL>());
+        TARG_INF("Physical");
+        (*iv_targets)[i].getAttr<ATTR_PHYS_PATH>().dump();
+
+        EntityPath l_entityPath;
+        if( (*iv_targets)[i].tryGetAttr<ATTR_AFFINITY_PATH>(l_entityPath) )
+        {
+            TARG_INF("Affinity");
+            l_entityPath.dump();
+        }
 
-            if( (*iv_targets)[i].tryGetAttr<ATTR_POWER_PATH>(l_entityPath) )
-            {
-                TARG_INF("Power");
-                l_entityPath.dump();
-            }
+        if( (*iv_targets)[i].tryGetAttr<ATTR_POWER_PATH>(l_entityPath) )
+        {
+            TARG_INF("Power");
+            l_entityPath.dump();
+        }
 
-            uint8_t l_dummyRw = 0;
-            if ((*iv_targets)[i].tryGetAttr<ATTR_DUMMY_RW> (l_dummyRw))
-            {
-                TARG_INF("Dummy = 0x%X",
-                    l_dummyRw);
-            }
+        uint8_t l_dummyRw = 0;
+        if ((*iv_targets)[i].tryGetAttr<ATTR_DUMMY_RW> (l_dummyRw))
+        {
+            TARG_INF("Dummy = 0x%X",
+                l_dummyRw);
+        }
 
-            TARG_INF("Supports FSI SCOM = %d",
-            (*iv_targets)[i].getAttr<ATTR_PRIMARY_CAPABILITIES>()
-                .supportsFsiScom);
-            TARG_INF("Supports XSCOM SCOM = %d",
-            (*iv_targets)[i].getAttr<ATTR_PRIMARY_CAPABILITIES>()
-                .supportsXscom);
-            TARG_INF("Supports Inband SCOM = %d",
-            (*iv_targets)[i].getAttr<ATTR_PRIMARY_CAPABILITIES>()
-                .supportsInbandScom);
-
-            ScomSwitches l_switches = {0};
-            if ( (*iv_targets)[i].tryGetAttr<ATTR_SCOM_SWITCHES>(l_switches) )
-            {
-                TARG_INF("Use FSI SCOM = %d",l_switches.useFsiScom);
-                TARG_INF("Use XSCOM = %d",l_switches.useXscom);
-                TARG_INF("Use inband SCOM = %d",l_switches.useInbandScom);
-            }
+        TARG_INF("Supports FSI SCOM = %d",
+        (*iv_targets)[i].getAttr<ATTR_PRIMARY_CAPABILITIES>()
+            .supportsFsiScom);
+        TARG_INF("Supports XSCOM SCOM = %d",
+        (*iv_targets)[i].getAttr<ATTR_PRIMARY_CAPABILITIES>()
+            .supportsXscom);
+        TARG_INF("Supports Inband SCOM = %d",
+        (*iv_targets)[i].getAttr<ATTR_PRIMARY_CAPABILITIES>()
+            .supportsInbandScom);
+
+        ScomSwitches l_switches = {0};
+        if ( (*iv_targets)[i].tryGetAttr<ATTR_SCOM_SWITCHES>(l_switches) )
+        {
+            TARG_INF("Use FSI SCOM = %d",l_switches.useFsiScom);
+            TARG_INF("Use XSCOM = %d",l_switches.useXscom);
+            TARG_INF("Use inband SCOM = %d",l_switches.useInbandScom);
+        }
 
-            uint64_t l_xscomBaseAddr = 0;
-            if ( (*iv_targets)[i].tryGetAttr<ATTR_XSCOM_BASE_ADDRESS>(
-                l_xscomBaseAddr) )
-            {
-                TARG_INF("XSCOM Base Address = 0x%X",l_xscomBaseAddr);
-            }
+        uint64_t l_xscomBaseAddr = 0;
+        if ( (*iv_targets)[i].tryGetAttr<ATTR_XSCOM_BASE_ADDRESS>(
+            l_xscomBaseAddr) )
+        {
+            TARG_INF("XSCOM Base Address = 0x%X",l_xscomBaseAddr);
+        }
 
-            XscomChipInfo l_xscomChipInfo = {0};
-            if ( (*iv_targets)[i].tryGetAttr<ATTR_XSCOM_CHIP_INFO>(
-                l_xscomChipInfo) )
-            {
-                TARG_INF("XSCOM Node ID = 0x%X",l_xscomChipInfo.nodeId);
-                TARG_INF("XSCOM Chip ID = 0x%X",l_xscomChipInfo.chipId);
-            }
+        XscomChipInfo l_xscomChipInfo = {0};
+        if ( (*iv_targets)[i].tryGetAttr<ATTR_XSCOM_CHIP_INFO>(
+            l_xscomChipInfo) )
+        {
+            TARG_INF("XSCOM Node ID = 0x%X",l_xscomChipInfo.nodeId);
+            TARG_INF("XSCOM Chip ID = 0x%X",l_xscomChipInfo.chipId);
+        }
 
-            I2cChipInfo l_i2cChipInfo = {0};
-            if( (*iv_targets)[i].tryGetAttr<ATTR_I2C_CHIP_INFO>( l_i2cChipInfo ) )
-            {
-                TARG_INF( "I2C Bus Speed = 0x%X",
-                          l_i2cChipInfo.busSpeed );
-                TARG_INF( "I2C Device Address = 0x%X",
-                          l_i2cChipInfo.deviceAddr );
-                TARG_INF( "I2C Device Port = 0x%X",
-                          l_i2cChipInfo.devicePort );
-                TARG_INF( "I2C Master Engine = 0x%X",
-                          l_i2cChipInfo.deviceMasterEng );
-            }
+        I2cChipInfo l_i2cChipInfo = {0};
+        if( (*iv_targets)[i].tryGetAttr<ATTR_I2C_CHIP_INFO>( l_i2cChipInfo ) )
+        {
+            TARG_INF( "I2C Bus Speed = 0x%X",
+                      l_i2cChipInfo.busSpeed );
+            TARG_INF( "I2C Device Address = 0x%X",
+                      l_i2cChipInfo.deviceAddr );
+            TARG_INF( "I2C Device Port = 0x%X",
+                      l_i2cChipInfo.devicePort );
+            TARG_INF( "I2C Master Engine = 0x%X",
+                      l_i2cChipInfo.deviceMasterEng );
         }
     }
-    else
-    {
-        assert(iv_initialized);
-    }
 
     return;