From aaacd79d799b9b99ab66505c23fc65b898550874 Mon Sep 17 00:00:00 2001 From: Zane Shelley Date: Tue, 7 Feb 2017 12:56:34 -0600 Subject: [PATCH] PRD: cleaned error handling for getConnectedParent() functions The functions will now assert if no parent is found. No need to check for nullptr anymore. Change-Id: I40da83f801ab2b47f2e98b7438211d68bea0dcc7 RTC: 168856 Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/36052 Reviewed-by: Benjamin J. Weisenbeck Tested-by: Jenkins Server Reviewed-by: Brian J. Stegmiller Reviewed-by: Zane C. Shelley Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/36079 Tested-by: FSP CI Jenkins Tested-by: Jenkins OP Build CI --- .../prdf/common/plat/mem/prdfMemEccAnalysis.C | 3 +- .../diag/prdf/common/plat/mem/prdfMemoryMru.C | 25 ++------- .../prdf/common/plat/p9/prdfP9ProcDomain.C | 9 +--- .../prdf/common/plat/prdfTargetServices.C | 51 ++++--------------- .../prdf/common/plat/prdfTargetServices.H | 4 +- .../diag/prdf/plat/mem/prdfMemScrubUtils.C | 4 +- src/usr/diag/prdf/plat/mem/prdfP9Mca.C | 3 +- src/usr/diag/prdf/plat/prdfPlatServices.C | 2 - src/usr/diag/prdf/plat/prdfPlatServices_ipl.C | 3 +- 9 files changed, 22 insertions(+), 82 deletions(-) diff --git a/src/usr/diag/prdf/common/plat/mem/prdfMemEccAnalysis.C b/src/usr/diag/prdf/common/plat/mem/prdfMemEccAnalysis.C index 8e809c4ca52..3c6b757fb16 100644 --- a/src/usr/diag/prdf/common/plat/mem/prdfMemEccAnalysis.C +++ b/src/usr/diag/prdf/common/plat/mem/prdfMemEccAnalysis.C @@ -5,7 +5,7 @@ /* */ /* OpenPOWER HostBoot Project */ /* */ -/* Contributors Listed Below - COPYRIGHT 2016 */ +/* Contributors Listed Below - COPYRIGHT 2016,2017 */ /* [+] International Business Machines Corp. */ /* */ /* */ @@ -64,7 +64,6 @@ uint32_t __addVcmEvent( ExtensibleChip * i_chip, PRDF_ASSERT( TYPE_MCA == i_chip->getType() ); ExtensibleChip * mcbChip = getConnectedParent( i_chip, TYPE_MCBIST ); - PRDF_ASSERT( nullptr != mcbChip ); // definitely a bug McbistDataBundle * mcbdb = getMcbistDataBundle( mcbChip ); diff --git a/src/usr/diag/prdf/common/plat/mem/prdfMemoryMru.C b/src/usr/diag/prdf/common/plat/mem/prdfMemoryMru.C index 3acf6e233f7..2441d4b95eb 100755 --- a/src/usr/diag/prdf/common/plat/mem/prdfMemoryMru.C +++ b/src/usr/diag/prdf/common/plat/mem/prdfMemoryMru.C @@ -5,7 +5,7 @@ /* */ /* OpenPOWER HostBoot Project */ /* */ -/* Contributors Listed Below - COPYRIGHT 2013,2016 */ +/* Contributors Listed Below - COPYRIGHT 2013,2017 */ /* [+] International Business Machines Corp. */ /* */ /* */ @@ -293,33 +293,14 @@ void MemoryMru::getCommonVars() PRDF_ASSERT( TYPE_MCA == trgtType || TYPE_MBA == trgtType ); TargetHandle_t proc = getConnectedParent( iv_target, TYPE_PROC ); - if ( NULL == proc ) - { - PRDF_ERR( PRDF_FUNC "Could not find proc attached to target 0x%08x", - getHuid(iv_target) ); - PRDF_ASSERT( false ); - } - TargetHandle_t node = getConnectedParent( proc, TYPE_NODE ); - if ( NULL == node ) - { - PRDF_ERR( PRDF_FUNC "Could not find node attached to target 0x%08x", - getHuid(proc) ); - PRDF_ASSERT( false ); - } // If our target is an MBA, get the chnlPos from the membuf and the // mbaPos from the target if ( TYPE_MBA == getTargetType(iv_target) ) { - TargetHandle_t membuf = getConnectedParent( iv_target, - TYPE_MEMBUF ); - if ( NULL == membuf ) - { - PRDF_ERR( PRDF_FUNC "Could not find membuf attached to 0x%08x", - getHuid(iv_target) ); - PRDF_ASSERT( false ); - } + TargetHandle_t membuf = getConnectedParent( iv_target, TYPE_MEMBUF ); + iv_memMruMeld.s.isMca = 0; iv_memMruMeld.s.chnlPos = getTargetPosition( membuf ); iv_memMruMeld.s.mbaPos = getTargetPosition( iv_target ); diff --git a/src/usr/diag/prdf/common/plat/p9/prdfP9ProcDomain.C b/src/usr/diag/prdf/common/plat/p9/prdfP9ProcDomain.C index daaa33836c5..23ea0fade74 100644 --- a/src/usr/diag/prdf/common/plat/p9/prdfP9ProcDomain.C +++ b/src/usr/diag/prdf/common/plat/p9/prdfP9ProcDomain.C @@ -5,7 +5,7 @@ /* */ /* OpenPOWER HostBoot Project */ /* */ -/* Contributors Listed Below - COPYRIGHT 2016 */ +/* Contributors Listed Below - COPYRIGHT 2016,2017 */ /* [+] International Business Machines Corp. */ /* */ /* */ @@ -146,13 +146,6 @@ void ProcDomain::SortForXstop() TargetHandle_t proc = sysDebug.getTargetWithAttn( TYPE_PROC, MACHINE_CHECK); node = getConnectedParent( proc, TYPE_NODE); - if( NULL == node ) - { - // We should never reach here. Even if we reach here, as this is - // XSTOP, we would like to go ahead with analysis to have as much - // data as possible. So just print a trace. - PRDF_ERR("[ProcDomain::SortForXstop] Node is Null"); - } } // Get internal setting and external driver list for each chip. diff --git a/src/usr/diag/prdf/common/plat/prdfTargetServices.C b/src/usr/diag/prdf/common/plat/prdfTargetServices.C index f08276ecdd8..c7459e20a66 100755 --- a/src/usr/diag/prdf/common/plat/prdfTargetServices.C +++ b/src/usr/diag/prdf/common/plat/prdfTargetServices.C @@ -624,8 +624,6 @@ TargetHandle_t getConnectedParent( TargetHandle_t i_target, TYPE i_connType ) PRDF_ASSERT( nullptr != i_target ); - TargetHandle_t o_parent = NULL; - // Get the association type, must be PARENT_BY_AFFINITY. TargetService::ASSOCIATION_TYPE assocType = getAssociationType( i_target, i_connType); @@ -636,21 +634,16 @@ TargetHandle_t getConnectedParent( TargetHandle_t i_target, TYPE i_connType ) PRDF_ASSERT(false); } - do + // Get the connected parent, should be one and only one parent + TargetHandleList list = getConnAssoc( i_target, i_connType, assocType ); + if ( 1 != list.size() || nullptr == list[0] ) { - TargetHandleList list = getConnAssoc( i_target, i_connType, assocType ); - if ( 1 != list.size() ) // Should be one and only one parent - { - PRDF_ERR( PRDF_FUNC "Could not find parent: i_target=0x%08x " - "i_connType=%d", getHuid(i_target), i_connType ); - break; - } - - o_parent = list[0]; - - } while(0); + PRDF_ERR( PRDF_FUNC "Could not find parent: i_target=0x%08x " + "i_connType=%d", getHuid(i_target), i_connType ); + PRDF_ASSERT(false); + } - return o_parent; + return list[0]; #undef PRDF_FUNC } @@ -849,16 +842,10 @@ ExtensibleChip * getConnectedParent( ExtensibleChip * i_child, { PRDF_ASSERT( nullptr != i_child ); - ExtensibleChip * o_parent = nullptr; - TargetHandle_t trgt = getConnectedParent( i_child->getTrgt(), i_parentType ); - if ( nullptr != trgt ) - { - o_parent = (ExtensibleChip *)systemPtr->GetChip( trgt ); - } - return o_parent; + return (ExtensibleChip *)systemPtr->GetChip( trgt ); } //------------------------------------------------------------------------------ @@ -1192,18 +1179,9 @@ uint32_t getMemChnl( TARGETING::TargetHandle_t i_memTarget ) // INVALID_POSITION_BOUND for call // from getTargetPosition(). - do - { - TargetHandle_t mcsTarget = getConnectedParent( i_memTarget, TYPE_MCS ); - if ( NULL == mcsTarget ) - { - PRDF_ERR( PRDF_FUNC "getConnectedParent() failed" ); - break; - } - - o_chnl = getTargetPosition( mcsTarget ); + TargetHandle_t mcsTarget = getConnectedParent( i_memTarget, TYPE_MCS ); - } while (0); + o_chnl = getTargetPosition( mcsTarget ); if ( MAX_MCS_PER_PROC <= o_chnl ) // Real MCS position check. { @@ -1597,13 +1575,6 @@ TARGETING::TargetHandle_t getClockId(TARGETING::TargetHandle_t if(TYPE_MEMBUF == getTargetType(i_pGivenTarget)) { l_target = getConnectedParent(i_pGivenTarget, TYPE_PROC); - if(NULL == l_target) - { - PRDF_ERR(PRDF_FUNC "failed to get proc target " - "connected to membuf 0x%.8X", - getHuid(l_target)); - break; - } } PredicateIsFunctional l_funcFilter; diff --git a/src/usr/diag/prdf/common/plat/prdfTargetServices.H b/src/usr/diag/prdf/common/plat/prdfTargetServices.H index 62d487c3b09..60b93013643 100755 --- a/src/usr/diag/prdf/common/plat/prdfTargetServices.H +++ b/src/usr/diag/prdf/common/plat/prdfTargetServices.H @@ -5,7 +5,7 @@ /* */ /* OpenPOWER HostBoot Project */ /* */ -/* Contributors Listed Below - COPYRIGHT 2016 */ +/* Contributors Listed Below - COPYRIGHT 2016,2017 */ /* [+] International Business Machines Corp. */ /* */ /* */ @@ -183,6 +183,7 @@ TARGETING::TargetHandleList getConnected( TARGETING::TargetHandle_t i_target, * @param i_connType Type of target(s) return in list * @note If the given target is the same type as the given type, the given * target is returned. + * @note This function will assert if no valid parent is found. * @return The requested parent target, NULL if something failed. */ TARGETING::TargetHandle_t getConnectedParent(TARGETING::TargetHandle_t i_target, @@ -217,6 +218,7 @@ ExtensibleChipList getConnected( ExtensibleChip * i_chip, * @param i_parentType The targeting type of the parent. * @note If the child is the same type as the parent type, the child is * returned. + * @note This function will assert if no valid parent is found. * @return The connected parent, nullptr if not found. */ ExtensibleChip * getConnectedParent( ExtensibleChip * i_child, diff --git a/src/usr/diag/prdf/plat/mem/prdfMemScrubUtils.C b/src/usr/diag/prdf/plat/mem/prdfMemScrubUtils.C index b1843bc360d..ef429cfa827 100644 --- a/src/usr/diag/prdf/plat/mem/prdfMemScrubUtils.C +++ b/src/usr/diag/prdf/plat/mem/prdfMemScrubUtils.C @@ -5,7 +5,7 @@ /* */ /* OpenPOWER HostBoot Project */ /* */ -/* Contributors Listed Below - COPYRIGHT 2016 */ +/* Contributors Listed Below - COPYRIGHT 2016,2017 */ /* [+] International Business Machines Corp. */ /* */ /* */ @@ -265,7 +265,6 @@ uint32_t checkEccFirs( ExtensibleChip * i_chip, PRDF_ASSERT( TYPE_MCA == i_chip->getType() ); ExtensibleChip * mcbChip = getConnectedParent( i_chip, TYPE_MCBIST ); - PRDF_ASSERT( nullptr != mcbChip ); SCAN_COMM_REGISTER_CLASS * mcaeccfir = i_chip->getRegister( "MCAECCFIR" ); SCAN_COMM_REGISTER_CLASS * mcbistfir = mcbChip->getRegister( "MCBISTFIR" ); @@ -328,7 +327,6 @@ uint32_t checkEccFirs( ExtensibleChip * i_chip, PRDF_ASSERT( TYPE_MBA == i_chip->getType() ); ExtensibleChip * membChip = getConnectedParent( i_chip, TYPE_MEMBUF ); - PRDF_ASSERT( nullptr != membChip ); const char * reg = (0 == i_chip->getPos()) ? "MBA0_MBSECCFIR" : "MBA1_MBSECCFIR"; diff --git a/src/usr/diag/prdf/plat/mem/prdfP9Mca.C b/src/usr/diag/prdf/plat/mem/prdfP9Mca.C index 6bc3f755553..1a2f7792a0a 100644 --- a/src/usr/diag/prdf/plat/mem/prdfP9Mca.C +++ b/src/usr/diag/prdf/plat/mem/prdfP9Mca.C @@ -5,7 +5,7 @@ /* */ /* OpenPOWER HostBoot Project */ /* */ -/* Contributors Listed Below - COPYRIGHT 2016 */ +/* Contributors Listed Below - COPYRIGHT 2016,2017 */ /* [+] International Business Machines Corp. */ /* */ /* */ @@ -75,7 +75,6 @@ int32_t RcdParityError( ExtensibleChip * i_mcaChip, if ( io_sc.service_data->IsAtThreshold() ) { ExtensibleChip * mcbChip = getConnectedParent( i_mcaChip, TYPE_MCBIST ); - PRDF_ASSERT( nullptr != mcbChip ); McbistDataBundle * mcbdb = getMcbistDataBundle( mcbChip ); diff --git a/src/usr/diag/prdf/plat/prdfPlatServices.C b/src/usr/diag/prdf/plat/prdfPlatServices.C index 7d7fa744279..3bee25dce85 100644 --- a/src/usr/diag/prdf/plat/prdfPlatServices.C +++ b/src/usr/diag/prdf/plat/prdfPlatServices.C @@ -309,7 +309,6 @@ uint32_t startBgScrub( ExtensibleChip * i_mcaChip, // Get the MCBIST fapi target ExtensibleChip * mcbChip = getConnectedParent( i_mcaChip, TYPE_MCBIST ); - PRDF_ASSERT( nullptr != mcbChip ); fapi2::Target fapiTrgt ( mcbChip->getTrgt() ); // Get the stop conditions. @@ -397,7 +396,6 @@ uint32_t __startTdScrub_mca( ExtensibleChip * i_mcaChip, // Get the MCBIST fapi target ExtensibleChip * mcbChip = getConnectedParent( i_mcaChip, TYPE_MCBIST ); - PRDF_ASSERT( nullptr != mcbChip ); fapi2::Target fapiTrgt ( mcbChip->getTrgt() ); do diff --git a/src/usr/diag/prdf/plat/prdfPlatServices_ipl.C b/src/usr/diag/prdf/plat/prdfPlatServices_ipl.C index b3674d26a38..67e1d6dae65 100644 --- a/src/usr/diag/prdf/plat/prdfPlatServices_ipl.C +++ b/src/usr/diag/prdf/plat/prdfPlatServices_ipl.C @@ -5,7 +5,7 @@ /* */ /* OpenPOWER HostBoot Project */ /* */ -/* Contributors Listed Below - COPYRIGHT 2016 */ +/* Contributors Listed Below - COPYRIGHT 2016,2017 */ /* [+] International Business Machines Corp. */ /* */ /* */ @@ -185,7 +185,6 @@ uint32_t startSfRead( ExtensibleChip * i_mcaChip, // Get the MCBIST fapi target ExtensibleChip * mcbChip = getConnectedParent( i_mcaChip, TYPE_MCBIST ); - PRDF_ASSERT( nullptr != mcbChip ); fapi2::Target fapiTrgt ( mcbChip->getTrgt() ); // Get the stop conditions.