代码审查九句真言.docx

1、代码审查九句真言看见了If，就想Else。看见malloc，就去找Free。函数调用要小心，需要看看返回值。看到for循环，就找边界值。看见return要注意，要去前面找资源。看见数组把神提，问题往往在下标。不要小看字符串，长度是个大问题。得到函数不要急，看看变量初始化，各种路径要小心。赋值函数最危险，变量没有初始化。九句句真言不孤立，相互结合显神威。真言详解1. 看见If，就想Else。看到if语句，就要想到else语句。如果没有else语句，就要分析是不需要，还是异常情况没有处理，如果是异常情况没有处理，可以提单。2. 看见malloc，就去找Free。看到malloc语句分配了内存，立即

2、停下正常走读，看malloc代码之后，是否在所有程序的返回分支中都有释放语句。典型案例：U32 DEV_IfSetSectionEnable(DEV_IF_T *pIfIns) CHAR * pSectionName = NULL; ULONG ulMsg4; /* 向配置文件发送消息DEV_IF_READ_SECTION, 进行配置下载 */ pSectionName = VOS_Malloc(MOD_DEV, MAX_INTERFACE_NAME_LEN+1); if ( pSectionName != NULL ) VOS_strcpy(pSectionName,pIfIns-ifNam

3、e); ulMsg0 = MID_DEV; ulMsg1 = MID_CFM; ulMsg2 = DEV_CFM_ENABLE_SECTION; ulMsg3 = (ULONG)pSectionName; VOS_Que_Write(ulVRPQID_CFM , ulMsg, VOS_NO_WAIT, 0); return SUCCESS; return DEV_ERR_NOMEMORY; 该函数有了VOS_Malloc，但没有看到free。于是在下面的代码中寻找，发现VOS_Que_Write中自动实现free功能。代码看似没有问题，但我们发现VOS_Que_Write的返回值没有判断，如果

4、VOS_Que_Write返回失败，free语句就没有被执行到。这样可以确认该函数存在内存泄露隐患。该代码最终修改：U32 DEV_IfSetSectionEnable(DEV_IF_T *pIfIns) CHAR * pSectionName = NULL; ULONG ulMsg4; ULONG rc; /* 向配置文件发送消息DEV_IF_READ_SECTION, 进行配置下载 */ if(pIfIns = NULL | (VOS_strlen( pIfIns-ifName) (MAX_INTERFACE_NAME_LEN+1) return DEV_ERR_GEN; pSection

5、Name = VOS_Malloc(MOD_DEV, MAX_INTERFACE_NAME_LEN+1); if ( pSectionName != NULL ) VOS_strcpy(pSectionName,pIfIns-ifName); ulMsg0 = MID_DEV; ulMsg1 = MID_CFM; ulMsg2 = DEV_CFM_ENABLE_SECTION; ulMsg3 = (ULONG)pSectionName; rc = VOS_Que_Write(ulVRPQID_CFM , ulMsg, VOS_NO_WAIT, 0); if(rc != SUCCESS) rc

6、= VOS_Free(pSectionName); VOS_DBGASSERT(rc = VOS_OK); return DEV_ERR_GEN; return SUCCESS; return DEV_ERR_NOMEMORY; 3. 函数调用要小心，需要看看返回值。看到函数调用，要养成习惯，进入函数内部瞄一眼。看看函数的正常值和异常值都是什么。看看返回值需不需要判断。看看有没有参数理解不一致的地方。例如： if ( VOS_strnicmp(szFullName, DEV_ATM_NAME , DEV_ATM_NAMELEN) = 0 ) ulIfType = DEV_GetIfTypeFr

7、omIfName( szFullName ); if ( ulIfType = -1 ) EXEC_OutString( ulExecID, rnUnknown interface type ); return VOS_ERR; /*得到端口的索引*/ ulRet = DEV_GetIfIndexFromIfName( szFullName, &ulIfIndex); if (SUCCESS != ulRet) EXEC_OutString( ulExecID, rnUnknown interface number ); return VOS_ERR; /*判断端口是否已经存在*/ pIfIn

8、s = DEV_GetIfFromIndex(ulIfIndex); if(NULL = pIfIns) rc = DEV_Cnsl_CreateIf(ulExecID, ulIfType, ulIfIndex, ulSubType); if(SUCCESS != rc) return SUCCESS; pIfIns = DEV_GetIfFromIndex(ulIfIndex); if(NULL = pIfIns) COUT_OUTPUT_DIAG(MOD_DEV, COUT_LEVEL_WARNING, pIfIns = NULL is invalid %s.%d , _FILE_, _L

9、INE_); return DEV_ERR_GEN; 函数使用-1作为非法值，而在DEV_GetIfTypeFromIfName函数中：U32 DEV_GetIfTypeFromIfName(CHAR *ifName) CHAR szIfType20; /接口的类型字符串 U32 ulIfType; U32 strLen; U32 i; if(NULL =ifName) COUT_OUTPUT_DIAG(MOD_DEV, COUT_LEVEL_WARNING, ifName = NULL in GetIfTypeFromIfName %s.%d, _FILE_, _LINE_); return

10、 DEV_ERR_VALUE; strLen = VOS_strlen(ifName); if(0 = strLen) /*字符串为空，返回错误*/ COUT_OUTPUT_DIAG(MOD_DEV, COUT_LEVEL_WARNING, strlen = 0 %s.%d, _FILE_, _LINE_); return DEV_ERR_VALUE; VOS_Mem_Set(szIfType, 0, sizeof(szIfType); /*-*/ /*从字符串的尾部向前查找，直到找到第一个不是数字的字符 */ /*-*/ for(i = strLen-1; i = 0; i-) /*字符不等

11、于., /或数字字符时循环结束*/ if(ifNamei != . & ifNamei != / & (ifNamei 9) break; VOS_strncpy(szIfType, ifName, i+1); ulIfType = DEV_IfStringToType( szIfType ); return ulIfType;函数的一个错误返回值是DEV_ERR_VALUE，很显然，两边参数理解不一致。4. 看到for循环，就找边界值。看到for循环，就要看看边界值是否合理。如果循环变量是数组的下标，更加需要注意。例如：U8* DEV_IfTypeToString(U32 ulIfType)

12、 U32 i; / change to user type ulIfType = DEV_IfUserType(ulIfType); for(i=0;ulIfType != -1 & irxTDscrIndex) rc = TD_IncreaseTDRefer(pValue-rxTDscrIndex); if (rc) #if PVC_APS_ENABLE = YES APS_DecrProtectConf(void);#endif return PVC_TRAFF_ERROR; newVlEntry.rxIndex = pValue-rxTDscrIndex; if ( pValue-txT

13、DscrIndex) rc = TD_IncreaseTDRefer(pValue-txTDscrIndex); if (rc) TD_DecreaseTDRefer(pValue-rxTDscrIndex);#if PVC_APS_ENABLE = YES APS_DecrProtectConf(void); #endif return PVC_TRAFF_ERROR; newVlEntry.txIndex = pValue-txTDscrIndex; if (newVlEntry.txIndex & newVlEntry.rxIndex) newVlEntry.direction = BI

14、_DIRECTIONAL; else if (newVlEntry.rxIndex) newVlEntry.direction = RECEIVE; else newVlEntry.direction = TRANSMIT; /* check virtual link self consistency */ rc = CheckVlSelfConsistency(&newVlEntry); if (rc) #if PVC_APS_ENABLE = YES APS_DecrProtectConf(void); #endif TD_DecreaseTDRefer(newVlEntry.rxInde

15、x); TD_DecreaseTDRefer(newVlEntry.txIndex); return rc; /* create virtual link*/ newVlEntry.port = port; newVlEntry.vpi = vpi; newVlEntry.vci = vci; newVlEntry.atmOperStatus = PVC_DOWN; newVlEntry.atmAlarmStatus = PVC_NOALARM; newVlEntry.oamSegEndPoint = PVC_NOT_ENDPT; newVlEntry.oamEndEndPoint = PVC

16、_NOT_ENDPT; newVlEntry.status = ROW_STATUS_UNKNOWN; newVlEntry.vlFlag = PVC_NONRING; VOS_Tm_Get(&newVlEntry.date, &newVlEntry.time, &newVlEntry.msec); /* check upc flag value */ if ( PVC_CHECK_UPCNPCFLAG(pValue-upc) ) return PVC_BAD_VALUE; /*p2mpleaf禁止设置UPC*/ if ( newVlEntry.atmCastType = P2MPLEAF &

17、 pValue-upc = ENABLE) return PVC_STATE_ERROR; else newVlEntry.atmConnUpcNpcEnb = pValue-upc; 红色代码处分配了资源，而在兰色代码处异常返回时没有被释放。6. 看见数组把神提，问题往往在下标。函数中一旦出现数组，就要提起精神。数组越界可是个致命问题。例如：ULONG DEV_ExecNetSwitchShow(ULONG ulUserID) NM_VALUE_T sValue; U32 frmIndex; U32 slotIndex; U32 netIndex1, netIndex2; U32 netst

18、atus1 = DEV_OBJ_STATUS_INACTIVE; U32 netstatus2 = DEV_OBJ_STATUS_INACTIVE; DEV_FRAME_T* pFrame; DEV_SLOT_T* pSlotIns; DEV_SLOT_T* pNetIns1; DEV_SLOT_T* pNetIns2; U32 rc; U32 i, j; / Loop Variable U8 pOutputStr80; / output info buffer U32 ulNetDEV_SLOT_ATMUNIT_MAX_NUM; U8* pOutputStatus; U8* pOutputT

19、ype; U8* pOutputApcNetDEV_ASLT_SUB_MAX_NUM - 1; U8 xidx, yidx, tmpPort; U32 ulNetVer; U32 ulSeq; U32 ulLen; void* pData; / pointer to msg received U8 szBuf256; DEV_NET_SWPTMAP_T sNetSwptMap; / check netboard netIndex1 = DEV_CONV_SLT2SLTINDEX(NET_CARD1); netIndex2 = DEV_CONV_SLT2SLTINDEX(NET_CARD2);

20、pNetIns1 = DEV_GetSlotFromIndex(netIndex1); pNetIns2 = DEV_GetSlotFromIndex(netIndex2); if (pNetIns1 = NULL & pNetIns2 = NULL) EXEC_OutString(ulUserID, rnnet board not found); return SUCCESS; if (pNetIns1 != NULL) rc = DEV_SltGetSlotStatus(netIndex1, &sValue); if (rc) /error output return DEV_ERR_GE

21、N; if (sValue.v_num != DEV_OBJ_STATUS_INACTIVE) netstatus1 = DEV_OBJ_STATUS_ACTIVE; if(pNetIns2 != NULL) rc = DEV_SltGetSlotStatus(netIndex2, &sValue); if (rc) /error output return DEV_ERR_GEN; if (sValue.v_num != DEV_OBJ_STATUS_INACTIVE) netstatus2 = DEV_OBJ_STATUS_ACTIVE; if (netstatus1 = DEV_OBJ_

22、STATUS_INACTIVE & netstatus2 = DEV_OBJ_STATUS_INACTIVE) EXEC_OutString(ulUserID, rnall the net board are inacitve); return SUCCESS; / default frame number is 0 frmIndex = DEV_CONV_FRM2FRMINDEX(DEV_DEFAULT_FRAME); pFrame = DEV_GetFrmFromIndex(frmIndex); if (pFrame = NULL) / the frame doesnt exist! CO

23、UT_OUTPUT_DIAG(MOD_DEV, COUT_LEVEL_WARNING, pFrame = NULL %s.%d , _FILE_, _LINE_); return DEV_ERR_GEN; EXEC_OutString(ulUserID, rnSlot Type State Apc0 Apc1 Apc2 Apc3); EXEC_OutString(ulUserID, rn- - - - - - -); for (i = 0; i slotArrayIndexi; if(slotIndex = DEV_OBJINDEX_NOEXIST) continue; / get slot status rc = DEV_SltGetSlotStatus(slotIndex, &sValue); if (rc) /error output return DEV_ERR_GEN; pOutputStatus = slotstatussValue.v_num; if (sValue.v_num != DEV_OBJ_STAT