代码审查九句真言.docx
《代码审查九句真言.docx》由会员分享，可在线阅读，更多相关《代码审查九句真言.docx（19页珍藏版）》请在冰豆网上搜索。
代码审查九句真言
看见了If，就想Else。
看见malloc，就去找Free。
函数调用要小心，需要看看返回值。
看到for循环，就找边界值。
看见return要注意，要去前面找资源。
看见数组把神提，问题往往在下标。
不要小看字符串，长度是个大问题。
得到函数不要急，看看变量初始化，各种路径要小心。
赋值函数最危险，变量没有初始化。
九句句真言不孤立，相互结合显神威。
真言详解
1.看见If，就想Else。
看到if语句，就要想到else语句。
如果没有else语句，就要分析是不需要，还是异常情况没有处理，如果是异常情况没有处理，可以提单。
2.看见malloc，就去找Free。
看到malloc语句分配了内存，立即停下正常走读，看malloc代码之后，是否在所有程序的返回分支中都有释放语句。
典型案例：
U32DEV_IfSetSectionEnable（DEV_IF_T*pIfIns）
{
CHAR*pSectionName=NULL;
ULONGulMsg[4];
/*向配置文件发送消息DEV_IF_READ_SECTION,进行配置下载*/
pSectionName=VOS_Malloc（MOD_DEV,MAX_INTERFACE_NAME_LEN+1）;
if（pSectionName!
=NULL）
{
VOS_strcpy（pSectionName,pIfIns->ifName）;
ulMsg[0]=MID_DEV;
ulMsg[1]=MID_CFM;
ulMsg[2]=DEV_CFM_ENABLE_SECTION;
ulMsg[3]=（ULONG）pSectionName;
VOS_Que_Write（ulVRPQID_CFM,ulMsg,VOS_NO_WAIT,0）;
returnSUCCESS;
}
returnDEV_ERR_NOMEMORY;
}
该函数有了VOS_Malloc，但没有看到free。
于是在下面的代码中寻找，发现VOS_Que_Write中自动实现free功能。
代码看似没有问题，但我们发现VOS_Que_Write的返回值没有判断，如果VOS_Que_Write返回失败，free语句就没有被执行到。
这样可以确认该函数存在内存泄露隐患。
该代码最终修改：
U32DEV_IfSetSectionEnable（DEV_IF_T*pIfIns）
{
CHAR*pSectionName=NULL;
ULONGulMsg[4];
ULONGrc;
/*向配置文件发送消息DEV_IF_READ_SECTION,进行配置下载*/
if（pIfIns==NULL||（VOS_strlen（pIfIns->ifName）>（MAX_INTERFACE_NAME_LEN+1）））
{
returnDEV_ERR_GEN;
}
pSectionName=VOS_Malloc（MOD_DEV,MAX_INTERFACE_NAME_LEN+1）;
if（pSectionName!
=NULL）
{
VOS_strcpy（pSectionName,pIfIns->ifName）;
ulMsg[0]=MID_DEV;
ulMsg[1]=MID_CFM;
ulMsg[2]=DEV_CFM_ENABLE_SECTION;
ulMsg[3]=（ULONG）pSectionName;
rc=VOS_Que_Write（ulVRPQID_CFM,ulMsg,VOS_NO_WAIT,0）;
if（rc!
=SUCCESS）
{
rc=VOS_Free（pSectionName）;
VOS_DBGASSERT（rc==VOS_OK）;
returnDEV_ERR_GEN;
}
returnSUCCESS;
}
returnDEV_ERR_NOMEMORY;
}
3.函数调用要小心，需要看看返回值。
看到函数调用，要养成习惯，进入函数内部瞄一眼。
看看函数的正常值和异常值都是什么。
看看返回值需不需要判断。
看看有没有参数理解不一致的地方。
例如：
if（VOS_strnicmp（szFullName,DEV_ATM_NAME,DEV_ATM_NAMELEN）==0）
{
ulIfType=DEV_GetIfTypeFromIfName（szFullName）;
if（ulIfType==-1）
{
EXEC_OutString（ulExecID,"\r\nUnknowninterfacetype"）;
returnVOS_ERR;
}
/*得到端口的索引*/
ulRet=DEV_GetIfIndexFromIfName（szFullName,&ulIfIndex）;
if（SUCCESS!
=ulRet）
{
EXEC_OutString（ulExecID,"\r\nUnknowninterfacenumber"）;
returnVOS_ERR;
}
/*判断端口是否已经存在*/
pIfIns=DEV_GetIfFromIndex（ulIfIndex）;
if（NULL==pIfIns）
{
rc=DEV_Cnsl_CreateIf（ulExecID,ulIfType,ulIfIndex,ulSubType）;
if（SUCCESS!
=rc）
{
returnSUCCESS;
}
pIfIns=DEV_GetIfFromIndex（ulIfIndex）;
if（NULL==pIfIns）
{
COUT_OUTPUT_DIAG（MOD_DEV,COUT_LEVEL_WARNING,"pIfIns=NULLisinvalid%s.%d"
__FILE__,__LINE__）;
returnDEV_ERR_GEN;
}
函数使用-1作为非法值，而在DEV_GetIfTypeFromIfName函数中：
U32DEV_GetIfTypeFromIfName（CHAR*ifName）
{
CHARszIfType[20];//接口的类型字符串
U32ulIfType;
U32strLen;
U32i;
if（NULL==ifName）
{
COUT_OUTPUT_DIAG（MOD_DEV,COUT_LEVEL_WARNING,"ifName=NULLinGetIfTypeFromIfName%s.%d",__FILE__,__LINE__）;
returnDEV_ERR_VALUE;
}
strLen=VOS_strlen（ifName）;
if（0==strLen）
{
/*字符串为空，返回错误*/
COUT_OUTPUT_DIAG（MOD_DEV,COUT_LEVEL_WARNING,"strlen=0%s.%d",__FILE__,__LINE__）;
returnDEV_ERR_VALUE;
}
VOS_Mem_Set（szIfType,0,sizeof（szIfType））;
/*---------------------------------------------------------*/
/*从字符串的尾部向前查找，直到找到第一个不是数字的字符*/
/*---------------------------------------------------------*/
for（i=strLen-1;i>=0;i--）
{
/*字符不等于'.','/'或数字字符时循环结束*/
if（ifName[i]!
='.'&&ifName[i]!
='/'
&&（ifName[i]<'0'||ifName[i]>'9'））
break;
}
VOS_strncpy（szIfType,ifName,i+1）;
ulIfType=DEV_IfStringToType（szIfType）;
returnulIfType;
}
函数的一个错误返回值是DEV_ERR_VALUE，很显然，两边参数理解不一致。
4.看到for循环，就找边界值。
看到for循环，就要看看边界值是否合理。
如果循环变量是数组的下标，更加需要注意。
例如：
U8*DEV_IfTypeToString（U32ulIfType）
{
U32i;
//changetousertype
ulIfType=DEV_IfUserType（ulIfType）;
for（i=0;ulIfType!
=-1&&i&&UserIfTypes[i].ulIfType!
=-1;i++）
{
if（UserIfTypes[i].ulIfType==ulIfType）
returnUserIfTypes[i].pIfTypeString;
}
return"unknown-interface";
}
函数中：
#defineDEV_USERIFTYPE_NUM9//numberofUserIfTypes,NOTDEV_IFTYPE_MAX_NUM!
!
!
而数组：
staticIfTypeList_SUserIfTypes[]=
{
{DEV_IFTYPE_ATM,"Atm","ATMinterface",3,0,DEV_AATMIF_LOGIF_MAX_NUM},
#ifndefISN_MODIFIED
//Modifiedby:
liyanmin
//Data:
2001/1/3
//Description:
8850设备目前只管理ATM物理端口和ATM子接口?
{DEV_ATM_TYPE_VPT,CLI_CMD_TEMPLET_IF_ATMVPT,"ATMVPtunnel",4,DEV_AATMIF_LOGIF_MAX_NUM,0},
{DEV_ATM_TYPE_VPRT,CLI_CMD_TEMPLET_IF_ATMVPRT,"ATMVPringtunnel",4,DEV_AATMIF_LOGIF_MAX_NUM,0},
#endif//endISN_MODIFIED
{DEV_ATM_TYPE_ATMSUB,"atm-sub","ATMsub-interfaceforIP",4,DEV_AATMIF_LOGIF_MAX_NUM,0},
#ifndefISN_MODIFIED
{DEV_IFTYPE_SONET,CLI_CMD_TEMPLET_IF_POS,"IPPOSinterface",3,0,0},
{DEV_IFTYPE_FASTETHERNET,CLI_CMD_TEMPLET_IF_FASTETHERNET,"IP10M/100Methernetinterface",3,0,DEV_AFE_VLAN_MAX_NUM},
{DEV_IFTYPE_GIGABITETHERNET,CLI_CMD_TEMPLET_IF_GIGAETHERNET,"IP1000Methernetinterface",3,0,DEV_AGE_VLAN_MAX_NUM},
{DEV_IFTYPE_VT,"virtual-template","IPPPPvirtualtemplete",1,1,0},
{DEV_IFTYPE_VE,CLI_CMD_TEMPLET_VIRTUAL_ETHERNET,"IPvirtualethernetforPPPOE",3,1,0},
//no{DEV_IFTYPE_L3IPVLAN,"vlan","IPvirtualLAN",1,0},
#endif//endISN_MODIFIED
{-1,"","",0,0},
};
而数组定义中，去掉被注释掉的部分，数组长度只有3。
数组将严重越界。
5.看见return要注意，要去前面找资源。
看见return语句，尤其是函数中间的异常返回语句。
看到这种语句，就需要折回头去看看前面有没有分配资源。
前面分配的任何资源（包括内存，端口，等等），在异常返回处需要一并释放。
例如：
在PVC_vl.c中的ULONGAllocateVl（void*pMsgEnv）函数中,
if（pValue->rxTDscrIndex）
{
rc=TD_IncreaseTDRefer（pValue->rxTDscrIndex）;
if（rc）
{
#ifPVC_APS_ENABLE==YES
APS_DecrProtectConf（void）;
#endif
returnPVC_TRAFF_ERROR;
}
newVlEntry.rxIndex=pValue->rxTDscrIndex;
}
if（pValue->txTDscrIndex）
{
rc=TD_IncreaseTDRefer（pValue->txTDscrIndex）;
if（rc）
{
TD_DecreaseTDRefer（pValue->rxTDscrIndex）;
#ifPVC_APS_ENABLE==YES
APS_DecrProtectConf（void）;
#endif
returnPVC_TRAFF_ERROR;
}
newVlEntry.txIndex=pValue->txTDscrIndex;
}
if（newVlEntry.txIndex&&newVlEntry.rxIndex）
newVlEntry.direction=BI_DIRECTIONAL;
elseif（newVlEntry.rxIndex）
newVlEntry.direction=RECEIVE;
else
newVlEntry.direction=TRANSMIT;
/*checkvirtuallinkselfconsistency*/
rc=CheckVlSelfConsistency（&newVlEntry）;
if（rc）
{
#ifPVC_APS_ENABLE==YES
APS_DecrProtectConf（void）;
#endif
TD_DecreaseTDRefer（newVlEntry.rxIndex）;
TD_DecreaseTDRefer（newVlEntry.txIndex）;
returnrc;
}
/*createvirtuallink*/
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_NOT_ENDPT;
newVlEntry.status=ROW_STATUS_UNKNOWN;
newVlEntry.vlFlag=PVC_NONRING;
VOS_Tm_Get（&newVlEntry.date,&newVlEntry.time,&newVlEntry.msec）;
/*checkupcflagvalue*/
if（PVC_CHECK_UPCNPCFLAG（pValue->upc））
returnPVC_BAD_VALUE;
/*p2mpleaf禁止设置UPC*/
if（newVlEntry.atmCastType==P2MPLEAF&&pValue->upc==ENABLE）
returnPVC_STATE_ERROR;
else
newVlEntry.atmConnUpcNpcEnb=pValue->upc;
红色代码处分配了资源，而在兰色代码处异常返回时没有被释放。
6.看见数组把神提，问题往往在下标。
函数中一旦出现数组，就要提起精神。
数组越界可是个致命问题。
例如：
ULONGDEV_ExecNetSwitchShow（ULONGulUserID）
{
NM_VALUE_TsValue;
U32frmIndex;
U32slotIndex;
U32netIndex1,netIndex2;
U32netstatus1=DEV_OBJ_STATUS_INACTIVE;
U32netstatus2=DEV_OBJ_STATUS_INACTIVE;
DEV_FRAME_T*pFrame;
DEV_SLOT_T*pSlotIns;
DEV_SLOT_T*pNetIns1;
DEV_SLOT_T*pNetIns2;
U32rc;
U32i,j;//LoopVariable
U8pOutputStr[80];//outputinfobuffer
U32ulNet[DEV_SLOT_ATMUNIT_MAX_NUM];
U8*pOutputStatus;
U8*pOutputType;
U8*pOutputApcNet[DEV_ASLT_SUB_MAX_NUM-1];
U8xidx,yidx,tmpPort;
U32ulNetVer;
U32ulSeq;
U32ulLen;
void*pData;//pointertomsgreceived
U8szBuf[256];
DEV_NET_SWPTMAP_TsNetSwptMap;
//checknetboard
netIndex1=DEV_CONV_SLT2SLTINDEX（NET_CARD1）;
netIndex2=DEV_CONV_SLT2SLTINDEX（NET_CARD2）;
pNetIns1=DEV_GetSlotFromIndex（netIndex1）;
pNetIns2=DEV_GetSlotFromIndex（netIndex2）;
if（pNetIns1==NULL&&pNetIns2==NULL）
{
EXEC_OutString（ulUserID,"\r\nnetboardnotfound"）;
returnSUCCESS;
}
if（pNetIns1!
=NULL）
{
rc=DEV_SltGetSlotStatus（netIndex1,&sValue）;
if（rc）
{
//erroroutput
returnDEV_ERR_GEN;
}
if（sValue.v_num!
=DEV_OBJ_STATUS_INACTIVE）
netstatus1=DEV_OBJ_STATUS_ACTIVE;
}
if（pNetIns2!
=NULL）
{
rc=DEV_SltGetSlotStatus（netIndex2,&sValue）;
if（rc）
{
//erroroutput
returnDEV_ERR_GEN;
}
if（sValue.v_num!
=DEV_OBJ_STATUS_INACTIVE）
netstatus2=DEV_OBJ_STATUS_ACTIVE;
}
if（netstatus1==DEV_OBJ_STATUS_INACTIVE&&netstatus2==DEV_OBJ_STATUS_INACTIVE）
{
EXEC_OutString（ulUserID,"\r\nallthenetboardareinacitve"）;
returnSUCCESS;
}
//defaultframenumberis0
frmIndex=DEV_CONV_FRM2FRMINDEX（DEV_DEFAULT_FRAME）;
pFrame=DEV_GetFrmFromIndex（frmIndex）;
if（pFrame==NULL）//theframedoesn'texist!
{
COUT_OUTPUT_DIAG（MOD_DEV,COUT_LEVEL_WARNING,"pFrame=NULL%s.%d"
__FILE__,__LINE__）;
returnDEV_ERR_GEN;
}
EXEC_OutString（ulUserID,"\r\nSlotTypeStateApc0Apc1Apc2Apc3"）;
EXEC_OutString（ulUserID,"\r\n-----------------------------"）;
for（i=0;i{
slotIndex=pFrame->slotArrayIndex[i];
if（slotIndex==DEV_OBJINDEX_NOEXIST）
{
continue;
}
//getslotstatus
rc=DEV_SltGetSlotStatus（slotIndex,&sValue）;
if（rc）
{
//erroroutput
returnDEV_ERR_GEN;
}
pOutputStatus=slotstatus[sValue.v_num];
if（sValue.v_num!
=DEV_OBJ_STAT
升级会员 