华为代码review准则.ppt

Page 1,检视方法,触发式检视 分类专项检视 交叉检视 每日新合入检视 版本差异检视 其他检视方法,分类专项检视1,内存使用类 1.内存申请后，是否有是否申请成功? 2.函数退出，特别是异常分支退出时，是否释放了内存? 3.内存是否被重复释放？ 4.操作内存时是否存在溢出使用。 5.申请与释放的接口是否一致？ 6.跨函数释放尤其要关注，写代码时尽量不要设计成跨函数释放。 7.释放后要对指针置空。,Page 2,分类专项检视2,指针使用类 1.是否对函数的指针参数做了合法性检查？ 2.是否使用了空指针、已经释放的指针？ 3.对指针的偏移计算是否正确？ 4.指针合法性检查后是否有正确的处理？ 5.判断到指针为空，应清理已申请的全部资源，并退出正常流程 6.多个指针变量同时指向同一块内存地址，用其中一个指针释放该内存后，其它指针是否置空或重新赋值；(如果对其它指针进行置空，则需要在使用这些指针的地方进行判断是否为空的保护),Page 3,越界访问类 1.字符串拷贝时，是否保证字符串的长度合法？ 2.字符串初始化、拷贝、连接的入参是否正确？ 3.必须有一个地方保证内存不溢出，这个保证要么是编写代码时的静态保证（定义的内存足够大），要么是运行时的动态检查，由函数提供者或调用者进行检查。建议由函数提供者进行检查，或用strncpy等方式避免拷贝过多的内存提示：使用strncpy后注意在结尾处增加0。 4.典型错误是字符串越界1字节；,Page 4,分类专项检视3,分类专项检视4,数组溢出类 1.使用数组时，数组下标是否在数组范围之内？ 2.尤其是当循环语句和数组一起使用时，一定要注意边界值不能溢出。 重点检查：边界值处理不正确，最大、最小值处理不正确，重点关注(for/while)循环变量的上下限检测； 错误案例说明： 1、(ULONG型变量 = 0)恒真循环；(恒真的原因：ULONG型变量为零时减1产生数值翻转，变为0xFFFFFFFFUL)； 2、循环上下限是由外部函数传入，没有对传入值进行合法性检查导致死循环等问题；,Page 5,3、多层循环时，禁止使用同一个循环变量，否则很容易导致死循环； 4、循环变量做为数组下标时，要防止数组下标越界； 5、在循环体内修改循环变量时要慎重；如：对循环变量同时有加减操作时，很可能会出现死循环，要特别关注；,Page 6,分类专项检视4,资源泄漏类 1.在过程/函数退出之前是否释放了所有需要释放的资源(内存/文件句柄等)？ 2.信号量、waitlist等申请后，是否有进行释放？ 3. 跨函数的释放，如果不是产品中早已明确的通用资源管理机制，则要在申请内存的地方写作注释 4.调用可能会返回失败的函数，一定要判断返回值并做资源回收。 异常处理类 1.异常分支是否进行了处理？是否处理正确？ 2.如switch语句一定要包含default分支； 3.如有if分支就应该包含else分支（对于else分支中不需要特殊处理的情况，使用注释说明）,Page 7,分类专项检视5,资源保护类 1.可重入函数使用全局变量时，应通过关中断、信号量（即P、V操作）等手段对其加以保护 2.只对简单全局变量（如整型）进行读操作的函数可以不申请互斥保护，但操作复杂全局变量（如链表，树）时，即使仅仅读取，也必须申请互斥保护，因为存在中途被其它任务破坏数据结构的可能性 3.任务切换需重点关注 数据溢出类 1.强制的数据类型转换，是否导致了数据的丢失？ 2.是否数据精度不足，导致可能发生数据溢出或翻转？ 3.如对计数变量，一般是进行定时读取，要保证在定时器超时前数据不能溢出。,Page 8,分类专项检视6,差1错误 1.此类错误一般是由于把“=”误写成“”等造成的 2.如：VlanID有效值为14095，通常会定义宏，比如MAX_VLAN_NUMBER为4096，这时需要注意在参数合法性判断时，需要用“= MAX_VLAN_NUMBER”，不要误用“ MAX_VLAN_NUMBER”。 返回值类 1.函数返回值的类型需要和函数声明的类型一致 2.在异常情况下，需要返回不同的错误码，而不是返回OK 3.函数返回值return ok、error、true、false是否用混？,Page 9,分类专项检视7,代码审查九字真言,看见了If，就想Else。 看见malloc，就去找Free。 函数调用要小心，需要看看返回值。 看到for循环，就找边界值。 看见return要注意，要去前面找资源。 看见数组把神提，问题往往在下标。 不要小看字符串，长度是个大问题。 得到函数不要急，看看变量初始化，各种路径要小心。 赋值函数最危险，变量没有初始化。 九句句真言不孤立，相互结合显神威。,Page 10,真言详解1,看见If，就想Else 看到if语句，就要想到else语句。如果没有else语句，就要分析是不需要，还是异常情况没有处理，如果是异常情况没有处理，可以提单。 看见malloc，就去找Free 看到malloc语句分配了内存，立即停下正常走读，看malloc代码之后，是否在所有程序的返回分支中都有释放语句。 函数调用要小心，需要看看返回值 到函数调用，要养成习惯，进入函数内部瞄一眼。看看函数的正常值和异常值都是什么。看看返回值需不需要判断。看看有没有参数理解不一致的地方。,Page 11,看到for循环，就找边界值 看到for循环，就要看看边界值是否合理。如果循环变量是数组的下标，更加需要注意。 看见return要注意，要去前面找资源 看见return语句，尤其是函数中间的异常返回语句。看到这种语句，就需要折回头去看看前面有没有分配资源。前面分配的任何资源（包括内存，端口，等等），在异常返回处需要一并释放。 看见数组把神提，问题往往在下标 函数中一旦出现数组，就要提起精神。数组越界可是个致命问题。,Page 12,真言详解2,不要小看字符串，长度是个大问题 字符串往往是代码审查中不被重视的问题。而字符串由于各个模块对其他模块的不了解，经常出现随便定义一个字符串长度的现象。 得到函数不要急，看看变量初始化，各种路径要小心 在拿到函数之后，要习惯性的看看所有的局部变量是在函数一开始就被初始化了如果有变量没有被初始化，就要小心了。如果发现变量是在if，for，while，等语句中被初始化的，问题可能就来了。 赋值函数最危险，变量没有初始化 在C代码中，经常使用将变量的指针作为参数，在被调用函数中对变量进行赋值的做法。这种程序的写法是标准的C语言的用法，无可厚非。但这种做法隐藏着极大的危险。,Page 13,真言详解3,触发式检视1,VOS_Mem_Zero ( 后继判断p是否为空指针了吗？ 空指针问题 #define AAA_AUTH_NONE 0x07 #define AAA_AUTH_PPP 0x10 注意这是16进制数 没有特殊的情况，建议用枚举来定义,Page 14,触发式检视2,ulIndex = pMsg-ulIndex; p = 之前判断ulIndex是否越界了吗？ 内存越界问题 while ( i+ ”与”=”等 #define MAX_NUM ( TOTAL_LEN HEAD_LEN / ITEM_LEN ) 似乎少了对括号？ 注意运算顺序，要加括号,Page 15,触发式检视3,while ( i pNext ; 断言的作用？ 断言并不能保你不死。经常看到有人这样写,Page 16,触发式检视4,while (pNext != NULL) continue; pNext = pNext-pNext; 所有continue的地方pNext都有取下一个吧？ 死循环问题，强烈建议用for来做循环消除此类问题 p = VOS_Malloc ( MID_XXX, ulSize ); return 所有return的地方都释放内存了吗? 内存泄漏问题,Page 17,触发式检视5,if ( ulIndex MAX_NUM) return VOS_ERR 应该是“&”还是“| |”？ 条件判断错误,Page 18,触发式检视6,switch ( a ) case: break; Switch语句有default分支吗？每个case都有break吗? if ( 0 = ucCount | 256 pNext = pDel-pNext ; pDel = NULL ; pDel指向的内存释放了吗？ 双向链表及其它类似情况都要考虑,Page 19,触发式检视7,vos_printf ( szString, “%d”, ulIndex) ; ulIndex是无符号数，你确信 用“%d”而不是“u”？ 会打印出负值 if (ulItemCount 0) ulItemCount = ulItemCount 2； 统计量有没有做翻转保护 ? 数值翻转问题。同样加的时候也要注意 UCHAR szStringMAX_LEN; UCHAR szNameMAX_LEN; VOS_sprintf ( szString, “user-%s”, szName ); 打印后变长了，内存会不会越界？ 为了省事而把用同一个宏定义字符串,Page 20,触发式检视8,if ( ulMtu = 9018 ) . 9018为何没有定义为宏？ 用宏或枚举来代替魔鬼数字 /*是否有用户在线？*/ if ( VOS_OK = = AAA_IsUserOnline ( ) ) . 确信函数返回的是VOS_OK而不是VOS_YES？ 误导人的的函数名：函数应该返回“是/否”还是 “成功/失败”呢？,Page 21,触发式检视9,typedef struct tagAaaUser UCHAR ucFlag; ULONG ulCid; AAA_USER_S; 结构4字节对齐了吗?,Page 22,触发式检视10,extern AAA_GetUserCid (USHORT *pusCid ); AAA_GetUserInfo ( (USHORT *) 参数类型不匹配，PC-LINT不通过，我只好做个强制转换，没问题吧？ 内存越界问题 if ( ulCount = 0 ) “”应该是”吧？ 建议把常量放在前面,Page 23,触发式检视11,#define AAA_XXX(a, b) ( ( a ) = ( a ) * (b ) ) 所有的参数都加括号了吗? 宏展开之后的错误 if ( !a & b + c * d f | g) 没有括号，你能记住运算符的优先级？ 养成良好的编程习惯，要求作者加上括号,Page 24,触发式检视12,UCHAR szOutString2048; for (i = 0; i MAX_NUM; i+) VOS_StrCat(szOutString); 会打印越界吗？ 2048虽然很大，但不能保证不会越界,Page 25,触发式检视13,UCHAR szOutString204800; for (i = 0; i MAX_NUM; i+) VOS_sprintf(szOutString); 204800够大了吧？ 太大的局部变量导致任务堆栈被破坏；建议申请动态内存,Page 26,触发式检视14,if ( ulItemFlag ucIndex+) 循环退出的条件可以满足吗？ 死循环问题,Page 27,触发式检视15,UCHAR ucEncryptedPasswd 128 ; UCHAR ucMacAddr 6 ; vos_printf ( ucEncryptedPasswd ); vos_printf ( ucMacAddr ); . 变量中存放的并不是字符串，这样输出的有不可预知的后果，重则死机 这种情况要用16进制打印ASCII码值,Page 28,触发式检视16,VOS_StrCpy ( szDest, szSrc) ; VOS_StrNCpy ( szDest, szSrc, ulLen) ; VOS_MemCpy (pDest, pSrc, ulLen) ; 字符串或内存拷贝没有越界吧？ 另外要注意把Src和Dest写反的情况 . ulPacketLen = pstIp- ip_usLen ; 报文处理做字节序转换了吗？ 从报文中取出后要做NTOH转换,填写报文时要做HTON转换,Page 29,触发式检视17,For (i = 0 ; i 1000000000 ; i+) ./very complex VOS_TaskDelay(100) ; 长时间的循环中，有主动释放CPU的操作吗？ 我们是实时系统，这个不能忽略。时间太久系统会自动复位 ! if (.) else if () else if () . 最后有else分支吗？ 相当于switch语句中的default,Page 30,触发式检视19,/*如果用户认证不成功*/ if (VOS_ERR = = AAA_UserAuth (ulCid ) ) /error; 应该是” VOS_OK != ”吧？ 注意这两种写法并不是等价的，因为可能还有其它的错误类型 VOS_sprintf ( szOutString, “%s”, szTemp ); VOS_sprintf ( szOutString, szTemp ); 这两种写法有区别吗？ 第1种写法是安全的，第2种写法时，如果szTemp含有转义字符(如”%s”)，则极度危险，严重至死机,Page 31,触发式检视20,#d