软件代码评审检查点--C-C++.doc

软件代码评审检查点-C-C+文件结构审查【检查点1 】文件名是否和实际内容相符？规范性说明文件名应该清晰说明出该文件的功能和作用。案例略【检查点2 】多个模块公用的定义和函数原型的说明是否放在“*.h”？说明一般来说，*.h文件是公用的头文件，文件中申明的宏、结构、函数原型等，一其他的模块需要引用。案例略【检查点3 】私有的申明和函数原型的说明是否放在“*.inc”？说明一般来说，*.inc文件是私有的头文件。文件中申明的宏、结构、函数原型等，是仅供本模块使用。案例略【检查点4 】头文件是否使用了#ifndef-#endif宏开关来防止重复引用？说明一般来说，头文件通过如下方法避免重复引用：#ifndef _XXX_H_#define _XXX_H_/*文件主体*/#endif 【检查点5 】函数原型是否没有明确申明返回值类型说明虽然C语言并不需要精确定义和声明函数返回类型，如果函数没有返回值，则定义为类型void。如果函数没有定义返回类型，编译器将认为其返回类型为int。在这种情况，很难判断函数是否应该有返回值。为了避免这种情况，函数返回类型应该被定义和声明。案例错误书写：SSfunction() ;正确书写：void SSfunction() ;预处理【检查点1 】宏定义是否缺少了“( )”符号？说明对于表达式的宏定义，注意要对变量和表达式本身使用“( )”，防止宏展开时出错。案例案例一注意下面的宏定义极易出现问题： #define ADD(x, y) x + y上面的宏在如下代码中出错： c = ADD(a, b) /2;宏展开后的形式为： c = a + b /2;而不是： c = (a + b) /2;案例二 #define MULTI(x, y) x*y在程序中代码如下： a = MULTI( b+c, 2);宏展开后为： a = b + c * 2 ;而不是： a = (b + c) * 2;【检查点2 】宏定义是否存在不期望或者意料之外的附加效应？说明宏是一个短行的文本,或者说文本模板,它可以被扩充为更长的文本.通常由宏产生的问题并不在宏定义本身,而往往由其下面的程序引起. 采用宏可以使代码简化,但它也可能隐藏重要的细节和关键的操作.案例 #define SQUARE ( x ) ( ( x ) * ( x ) ) . . . w = SQUARE ( + value ); 实际对宏SQUARE的引用将导致value被增加两次 , 因为这个语句将被C预处理器扩充以下形式: w = ( ( + + value ) * ( + + value ) );【检查点3 】是否存在通过定义宏来改变程序控制流程？说明通过定义宏来改变程序控制流程实际上是很糟糕的编程技巧案例 错误形式: #define FOR_ALL for ( i = 0; i cmd.req.req2, tmrc ) );正确形式: return ( & MLtisfail ( mltmsg - cmd.req.req2, tmrc ) );在如下这些情况下传送指针作为参数是更好的选择: - 需要传送大量的数据和大型的数据结构作为输入时, - 被调函数需要修改调用函数的数据时.【检查点4 】返回值的所携带的信息是否的确是调用函数所需要的? 说明略【检查点5 】函数入参是间接引用参数时候，当中是否漏掉一个层次？ 说明 案例 错误形式: DXal_fndnxt ( . . . , ( DXALMDATA * ) & data_ptr );正确形式: DXal_fndnxt ( . . . , ( DXALMDATA * * ) & data_ptr );【检查点6 】函数退出时是否有正确的返回值？说明看到函数调用，要养成习惯，进入函数内部瞄一眼。看看函数的正常值和异常值都是什么。看看返回值需不需要判断。看看有没有参数理解不一致的地方。案例错误形式:if ( VOS_strnicmp(szFullName, DEV_ATM_NAME , DEV_ATM_NAMELEN) = 0 ) ulIfType = DEV_GetIfTypeFromIfName( 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;/*判断端口是否已经存在*/ pIfIns = 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_, _LINE_); return DEV_ERR_GEN; 函数使用-1作为非法值，而在DEV_GetIfTypeFromIfName函数中：U32 DEV_GetIfTypeFromIfName(CHAR *ifName)CHAR szIfType20; /接口的类型字符串U32ulIfType;U32 strLen;U32 i;if(NULL =ifName)COUT_OUTPUT_DIAG(MOD_DEV, COUT_LEVEL_WARNING, ifName = NULL in GetIfTypeFromIfName %s.%d, _FILE_, _LINE_); return 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-) /*字符不等于., /或数字字符时循环结束*/ if(ifNamei != . & ifNamei != / & (ifNamei 9) break; VOS_strncpy(szIfType, ifName, i+1);ulIfType = DEV_IfStringToType( szIfType );return ulIfType;函数的一个错误返回值是DEV_ERR_VALUE，很显然，两边参数理解不一致。【检查点7 】调用其他函数时是否对返回值进行了判断？说明略【检查点8 】在函数调用前是否认真核实传参的数据类型？ 说明 案例 错误形式: (void) CCbinasc ( loop_ptr - annc_rtidx, . . . );正确形式: (void) CCbinasc ( ( DMUNLONG ) loop_ptr - annc_rtidx , . . . );【检查点9 】在多任务操作系统环境下是否考虑了函数的可重入性？说明在多任务的操作系统中，对于多任务共用的函数，如果该函数用到了全局变量或静态变量，需要注意到函数的可重入性。一般来说，如果函数只使用了内部的非静态局部变量，则该函数没用可重入的问题【检查点10 】当一个指针被传给一个函数,并且被调用的函数试图增加或减少这个指针所指示的内容时是否出现了常见错误？说明此时容易出现常见错误案例案例一这个例子中,尽管 + 和 * 操作符拥有相同操作优先级. 但它们是按从右到左的顺序执行的.错误形式: * numretry +;正确形式:(* numretry) +;案例二另一个常见的错误发生在当你试图调用一个函数并取其返回值,然后在一个条件判断表达式中测试这个返回值时.错误形式: if ( ( rtc = _ims_open ( NPRD_CH ) ! = _SUCCESS ) ) 正确形式: if ( ( rtc = _ims_open( NPRD_CH ) ) ! = _SUCCESS ) 【检查点11 】函数定义中增添了一个参数时，是否有在所有该函数调用的场合完成对应修改？ 说明(函数被调用而参数未被充分传递的错误也因之而生).如果新的参数是加到原来所定义的参数序列的开始或中间位置时问题将更加严重.发生这种问题时,所有从这一点起前面已经存在的参数都被错位了.下面这个例子说明这个问题. 案例函数调用传送了三个参数: DBnswch_mem( &omsg, &smtimr, grwsize ) ;但是函数定义了四个参数: DBnswch_mem( OSPID, *omsg, *timr, grwsize) ;【检查点13 】一组错误类型是否也归入这个子标题所表示的错误类型？说明调用 printf 及其相关的这组类型函数时传递了不恰当数量的参数数量.问题来自于可能传递了更多或更少于转义字符的变量参数给打印函数.案例错误形式: sprintf ( infile, /rclog/tmp.evl %d.5E3l );错误形式: sprintf ( corclog, /log/cpcorc, 0 ); 【检查点14 】fprintf ( ) 和 printf ( ) 是否被含糊理解？说明前者要求一个流指针作参数一而后者不需要.案例错误形式: fprintf( system error, cdbcom not found in relation table en );错误形式: printf( stdout, %s VPATH:en, Prompt);【检查点15 】函数定义前是否已经声明过？说明程序在调用函数和存取全局数据之前,必须先声明函数的返回值类型和全局数据的数据类型.通常,这些申明是头文件的一部分,并且必须根据函数调用/数据存取的范围予以定义.函数申明必须提供作为函数原型,也称之为函数模板.函数原型 列出函数每个参数的类型以及函数返回参数的类型.通过这些函数原型和关于数据定义的头文件确保被调函数/被存取的数据的正确定义.案例略如果被调函数/被存取的数据只是在某个特定的模块中定义和使用,则无需专门的头文件.如果被调函数/被存取的数据在某个特定模块中定义但在子系统中多余一个模块中使用,则对它们的申明(包括所用到的数据类型的定义)就应该用到局部于该子系统的本地头文件.如果被调函数/被存取的数据被其所定义的子系统之外的其他子系统所使用,那么其类型申明(包括所用到的数据类型的定义)就应该用到系统全局的头文件.【检查点16 】函数参数是否清晰而简洁？说明在接口上,简洁是非常可取的特点,因为函数的参数的数量将影响程序接口的复杂度. 参数数量巨大将使得接口无法管理,代码难以阅读和理解, 接口的可维护性也会因其易于插入诸如修改参数后造成参数类型不匹配的错误而受到损害. 数目7的法则(rule fo seven) 可能是函数调用时合理的最大的参数数目.一些接口的度量方法有益于管理函数的参数,例如: 1) 每个函数的变量数目(输入 + 输出), 2) 与函数相关联的函数的数目(调用和被调用).在一个结构( struct 或者 typedef struct )中传送所有或者大部分被调函数所需的参数数据是一个达到简洁效果的有力方法.案例通常形式: SSfunction ( code, value, type, DBYES, msg.numb, msg.size, msg.unit.side, msg.unit.mod, msg.reg_make, DBNO);传送指针: SSfunction ( code, value, type, DBYES, &msg, DBNO);传送结构: SSfunction ( code, value, type, DBYES, msg, DBNO);如果传送的是一个指向结构的指针,那么被调函数可以改变这个结构中元素的值.如果传送整个结构,并且结构包含的元素多于单独传递每个元素所需传递的元素总数时,程序设计者必须注意避免引入堆栈溢出问题.【检查点17 】函数是否有正确的返回类型？说明 尽管C语言不要求必须明确定义或申明函数的返回值类型, 但如果函数没有返回值则应该定义或者申明为void类型. 如果函数未被明确的定义或申明其返回类型, 编译器默认其返回类型为int . 这种情况下,可能就难以清楚知道函数是否返回了参数.因此为避免这种含糊,我们应该清楚的定义和申明所有函数的返回值类型.为避免对函数进一步使用时的含糊不清以及提高程序的可维护性,应该运用 void 数据类型通知编译器函数将不会返回任何参数. 因为编译器无需为返回参数产生目标代码,因此也节约了堆栈空间同时提高系统性能. 不能在表达式中使用被定义或申明为 void 类型的函数.函数中实际返回值的类型必须和函数类型相同或者必须强制转换为函数类型. 如果函数的返回值无需被检查或保留,那么函数返回值可以强制转换为 (void), 这也将节约堆栈空间.案例 案例一未明确定义的函数类型默认为 int 类型: 错误形式: SSfunction ( ) return; 正确形式: void SSfunction ( ) return; 案例二尽管函数 SSmodify ( ) 被申明为返回 void 类型,但它的返回值却被检查了。 错误形式: void SSfunction ( ) void SSmodify ( ); if ( SSmodify ( ) = = GLSUCCESS ) 案例三如果调用一个有返回值的函数,但是又无需检测和保留该返回值,则应该将其强制转换为 void 类型. 错误形式: RET_VAL SSfunction ( ) strncpy ( cost, GLctcst, 4); . . . 正确形式: RET_VAL SSfunction ( ) (void) strncpy ( cost, Glctcst, 4) ; . . . 案例四如果已定义为返回 void 类型的函数却返回了一个值, 将不会被编译器编译. 错误形式: void SSbad_func ( ) return ( 0 ); 【检查点19 】是否有防御型的编程技巧？说明函数调用时,程序应该检查参数传递是在所期待的取值范围之内.这项原则有利于防止不加区别的使用指针以致于其值无法担保其安全性.防御型检查的程度需要和实时性要求相互制约平衡.例如,当同一个参数传递到更低层次的函数时,层层检查的方法并不是绝对必要的.如果参数在最初的函数中进行了检查,就无需在每一次函数调用中再次检查了,除非在过程中参数被修改了.案例 案例一在使用全局指针前,程序员应该检查指针的安全性. 正确形式: /* 对头指针和尾指针进行边界检查 */ if ( CRrptr CRMAX | | CRfptr CRMAX ) CRqrecover ( ); /* recover the CRA queue */ AUASRTA ( AUFALSE, CRA_$_ASRT); return ( CRERROR ); 案例二进入函数时,程序必须确保参数的安全性. 正确形式: short DBopgparm (parid, new_buf, pcrid, operation ) DMSYSID parid; char * new_buf; unsigned char pcrid; DMDBMODE operation; /* Global parameter operation */ . . . . . . /* 对 parid 作范围检查 */ if ( ( parid = dbparmax ) ) return ( DBSYS_ERR ); 变量【检查点1 】是否使用了未初始化的变量？说明 该检查点要求检查在程序中使用的变量是否在使用前已经初始化，确定初始化的位置和第一次调用的位置。 该类错误绝大部分可以使用PC-LINT检查出来，但对一些隐藏的较深的情况使用PC-LINT无法查出，需要特别注意。例如，对于全局变量A，在模块B中初始化，在模块C中调用。如果C在B前调用，显然就会发生该类错误，而PC-LINT是无法发现的案例略变量初始化的法则是: 1)局部和全局的常量 ( constant ) 必须在定义的位置进行初始化. 2)非常量、非静态的局部变量必须在代码段中予以初始化,而不是在它们定义的位置 . 3)与进程有关的全局变量应该在每次进程被重建时调用特定的函数对它们进行初始化.【检查点2 】是否定义了变量但是没有使用？规范性说明略案例略【检查点3 】是否存在类型不匹配的赋值操作说明该检查点主要检查程序中是否有不同类型数据相互转换造成的错误。在程序中，开启定时器的正确书写为：long time;time = 10000;OSWAIT(10000, time);错误书写：char time;time = 10000;OSWAIT(10000, time);在这里，执行语句time = 10000;，然后作为第二个参数传递到函数OSWAIT()以产生10秒的等待。但是，由于变量time为单字节，所以10000被截取为16进制。因此，该代码只等待16毫秒，而不是10秒。【检查点4 】是否存在类型不匹配的比较操作说明在循环终止条件的判断中，不同类型变量的比较操作是容易造成死循环错误的地方，同时开发人员容易忽视的地方，在代码审查中值得多加留意。案例在下列循环中：while( ftell(fp)= 0) /*将出现下溢*/ . /* program code*/当size等于0时，再减1不会小于0，而是0xFF，故程序是一个死循环。应如下修改。char size; /* 从unsigned char 改为char*/while (size- = 0) . /* program code */【检查点7 】是否没有判断unsigned为最大值而直接加1（上溢）？说明unsigned为最大值时，加1则为0，产生上溢。【检查点8 】调用函数时传入参数和是否和函数原型申明不一致？说明略案例在函数INT DebugMsgProc(char byMsg0, char byMsg1)函数中，两个形参都是char型。如果实际传入的参数都是BYTE型（unsigned char），函数中的如下语句： PrintfE(PID_RED, %d ticks time out!,byMsg1);在byMsg1大于127时，输出错误的结果。【检查点9 】变量的计算和取值顺序是否清淅？说明有的编译器从右到左计算表达式而有的是从左到右, 取决与不同的方法。变量可能将相互偏离一个取值。无论以什么为基准,这样的代码是含糊不清的。案例错误形式:for ( idx = 0 ; idx 40; dispstring idx = COTsuccess idx+ ) ;正确形式:for ( idx = 0 ; idx cindex = (cindex + 1) & 0x7F;这里,值被0X7F做了采样(即采样7位)之后进行赋值操作. 因为 cindex 在数据结构中仅仅被分配了6比特的存储空间,赋值的数据被修剪了,导致了严重的域问题.注意决不可忽视变量赋值的过程.必须检查确保所有的变量不能赋以超过变量本身数据结构可以容纳的数值.【检查点11 】是否将局部变量在函数体外使用？说明局部变量只在函数体中有效案例案例一下面的函数对变量a的操作是错误的：int *foo (void) int a; /* code */ /*a 使局部变量，其地址位于堆栈中 ，返回a的指针是没有意义的。*/ return &a; 案例二在SendToOtherOMU（）函数中，局部数组变量 unsigned char ucpToMsg300 的地址被作为消息传送给MSG_QUEUE队列。由于局部变量的生存期仅在函数运行期内，当函数退出后，局部变量自动失效，导致队列中的消息不确定int SendToOtherOMU(unsigned char * ucpMsg)int err;unsigned char ucpToMsg300; /* 定义存放消息局部变量 */UcpToMsg0=MSG_SEND_DATA;MemCopy(ucpMsg,ucpToMsg+1,ucpMsg0+1); /* 形成消息内容 */sc_qpost(MSG_QUEUE,(char *)ucpToMsg,&err); /* 将数组首地址传递给函数 */If(err!=0)Return(NACK_SC_QPOST_ERR);Return(0);在上述代码中，sc_qpost(MSG_QUEUE,(char *)ucpToMsg,&err) 语句 将ucpToMsg 数组的首地址放入MSG_QUEUE队列，当函数退出后，局部变量失效，当队列中的消息不能及时取走时，可能出现 ucpToMsg 地址开始的内存被覆盖，导致消息数据发生变化。【检查点12 】在return语句前，占有的资源是否被释放？说明看见return语句，尤其是函数中间的异常返回语句。看到这种语句，就需要折回头去看看前面有没有分配资源。前面分配的任何资源（包括内存，端口，等等），在异常返回处需要一并释放。案例在ULONG AllocateVl(void *pMsgEnv)函数中, if ( pValue-rxTDscrIndex) rc = TD_IncreaseTDRefer(pValue-rxTDscrIndex); if (rc) #if PVC_APS_ENABLE = YES APS_DecrProtectConf(void);#endif return PVC_TRAFF_ERROR; newVlEntry.rxIndex = pValue-rxTDscrIndex; 指针【检查点1 】使用指针时是否先判断为空？ 说明指针在使用前应该初始化，或者赋值。【检查点2 】对数组amn是否正确使用“ai+j ”和“&ai+j ”指针？说明C语言中规定数组名代表数组的首地址。对任意的二维数组 amn， a表示本数组的第1个元素的地址，a + i 表示本数组第 i 行的首地址。 ai 代表 amn 第 i 行的首地址（即第 i 行第 0 列元素的地址），&ai 也表示第 i 行第 0 列元素的地址。 但ai与&ai 的含义却是不同的，是两种不同类型的指针，在指针运算时就会表现出不同，它们处理下一单元的方向是不一样的：&ai指向行，而ai指向列。对 &ai+j 和 ai+j 而言，只有当 j 为0 时，&ai+j和ai+j 的值才相等，即都指向第 i 行第 0 列的元素。 例如， 定义了一个二维数组a66， 当j = 0.5 时， ai + j和&ai+ j 产生的地址如下： ai + j ：按行产生地址，即 ai0、ai1 . ai5 的地址 &ai + j ：按列产生地址，即ai0、ai+10 . ai+j0的地址，也就是aij 数组第 i + j 行的首地址。 如对&ai进行强制转换，&ai和ai就等价了，即(_UC*)&ai也按行的方向计算地址。案例略【检查点3 】对指针加减法操作是否超出数据类型的边界？说明运用指针来提供记法上的便利并且提高程序的效率,同时也使得代码占用更少的内存、执行得更快.指针的递增和递减量规定为指针所指示的数据类型的数据空间大小.对指针加减法操作不要超出数据类型的边界.以下显示一个对指针的增减操作超出边界的例子.案例错误形式: short valueexmaxval; short *value_pointer; value_pointer = value; value_pointer += (exmaxval + 1 );【检查点3 】是否正确地使用数组函数参数的指针？说明没有下标的数组名是指向该数组的指针.需特别指明没有下标的数组名产生的是一个指向数组第一个元素的指针.将函数参数申明为数组并使用其名称,而不是指向数组的指针.如以下所示.内存操作【检查点1 】是否对数组进行了越界操作?说明内存操作主要是指对数组、指针、内存地址等的操作。内存操作越界是软件系统主要错误之一，后果往往非常严重，所以当我们进行这些操作时一定要仔细小心。案例案例一在C语言中，定义数组a10，则有效的数组元素为：a0, a1, ., a9，在程序中使用a10是错误的。【检查点2 】内存拷贝时是否判断了长度？【检查点3 】申请内存后是否判断了内存申请成功？【检查点4 】对异常情况的处理时是否直接return而忘记了释放内存?说明对于动态申请和释放的内存使用经常存在以下问题：1、异常情况的处理，直接调用了return，忘记了释放内存。2、switch语句没有相应的default处理或default处理中也遗漏了释放内存的操作。3、if语句没有相应的else分支，原理同上。4、连续申请两个内存块的时候，若第二个申请不成功而函数返回时，应释放第一个申请的内存块。实际上存在着第二个申请不成功就直接return的情况。案例案例一下面的代码段是一个异常情况内存没有释放的例子： . pRecord = new charpTable-GetRecordLength();assert(pRecord != NULL);if (pTable-GoTop(FALSE) != DBIERR_NONE)return; /* 如果从这里返回，pRecord将得不到释放*/ . pTable-Close();delete pRecord; 【检查点5 】终止一项任务时是否完全释放内存？说明略案例略循环for,while循环语句 【检查点1 】是否在for语句后误用了“;”【检查点2 】是否在while 语句后误用了“;”【检查点3 】控制循环的变量是否有递增（递减）操作？【检查点4 】是否在多层循环中使用同一个循环控制变量？【检查点5 】多层循环中的break对应关系是否正确？【检查点6 】循环边界是否正确说明该检查点需要检查程序中所有的循环边界是否设置正确。要求确保程序不会进入死循环。常犯的错误是对于for循环，循环的边界大于循环变量的取值范围，这样就会直接导致死循环。比如用unsigned char型变量作为循环变量，而循环边界大于unsigned char型变量的最大值255。对于使用while循环进行链表操作往往容易引起更隐蔽的死机错误。这种错误常常只有在很特殊的情况下才会重现，需要仔细检查。建议在使用while循环比较复杂的链表操作时，可以在while循环中增加循环变量判断或改写成for循环。这样即便出现死循环的条件，但是只要对循环变量进行控制，也能够退出循环。案例 案例一在如下对链表的操作代码： while( go != NO_JOB)go = JobTablego.NextJob;在实际环境中，由于其他异常的情况造成链表被改成了循环链表，从而在这里陷入了死循环。对于这种代码，可以通过改造加入保护，从而防止在这里陷入死循环。这是实际中改造后的代码： for( protect=0; (protectMAX_JOBS)&(go != NO_JOB); protect+ ) go = JobTablego.NextJob; 案例二尽管局部数组proname 的长度分配了正确的数量, 但是因为循环的边界限制设定得太高,数组最终被越界检索了.错误形式: char proname 10 ; . . . for (i = 0; i 14; i + ) pronamei = *argv 0 ; . . .案例三循环的边界测试可能持续的太远.在这个例子当中,变量 featent.curpos 只能测试小于,而不能是等于,NBISTRFTENT的情况.错误形式: for( ; ( featent.curpos = NBISTRMFTENT & .正确形式: for( ; ( featent.curpos NBISTRNFTNT & .为避免类似微妙的错误,所有的循环测试的取值范围应该仔细的检查.案例四U8* DEV_IfTypeToString(U32