华为代码review准则.ppt

Page1,检视方法,触发式检视分类专项检视交叉检视每日新合入检视版本差异检视其他检视方法,分类专项检视1,内存使用类1.内存申请后，是否有是否申请成功?2.函数退出，特别是异常分支退出时，是否释放了内存?3.内存是否被重复释放？4.操作内存时是否存在溢出使用。5.申请与释放的接口是否一致？6.跨函数释放尤其要关注，写代码时尽量不要设计成跨函数释放。7.释放后要对指针置空。,Page2,分类专项检视2,指针使用类1.是否对函数的指针参数做了合法性检查？2.是否使用了空指针、已经释放的指针？3.对指针的偏移计算是否正确？4.指针合法性检查后是否有正确的处理？5.判断到指针为空，应清理已申请的全部资源，并退出正常流程6.多个指针变量同时指向同一块内存地址，用其中一个指针释放该内存后，其它指针是否置空或重新赋值；(如果对其它指针进行置空，则需要在使用这些指针的地方进行判断是否为空的保护),Page3,越界访问类1.字符串拷贝时，是否保证字符串的长度合法？2.字符串初始化、拷贝、连接的入参是否正确？3.必须有一个地方保证内存不溢出，这个保证要么是编写代码时的静态保证（定义的内存足够大），要么是运行时的动态检查，由函数提供者或调用者进行检查。建议由函数提供者进行检查，或用strncpy等方式避免拷贝过多的内存提示：使用strncpy后注意在结尾处增加0。4.典型错误是字符串越界1字节；,Page4,分类专项检视3,分类专项检视4,数组溢出类1.使用数组时，数组下标是否在数组范围之内？2.尤其是当循环语句和数组一起使用时，一定要注意边界值不能溢出。重点检查：边界值处理不正确，最大、最小值处理不正确，重点关注(for/while)循环变量的上下限检测；错误案例说明：1、(ULONG型变量=0)恒真循环；(恒真的原因：ULONG型变量为零时减1产生数值翻转，变为0 xFFFFFFFFUL)；2、循环上下限是由外部函数传入，没有对传入值进行合法性检查导致死循环等问题；,Page5,3、多层循环时，禁止使用同一个循环变量，否则很容易导致死循环；4、循环变量做为数组下标时，要防止数组下标越界；5、在循环体内修改循环变量时要慎重；如：对循环变量同时有加减操作时，很可能会出现死循环，要特别关注；,Page6,分类专项检视4,资源泄漏类1.在过程/函数退出之前是否释放了所有需要释放的资源(内存/文件句柄等)？2.信号量、waitlist等申请后，是否有进行释放？3.跨函数的释放，如果不是产品中早已明确的通用资源管理机制，则要在申请内存的地方写作注释4.调用可能会返回失败的函数，一定要判断返回值并做资源回收。异常处理类1.异常分支是否进行了处理？是否处理正确？2.如switch语句一定要包含default分支；3.如有if分支就应该包含else分支（对于else分支中不需要特殊处理的情况，使用注释说明）,Page7,分类专项检视5,资源保护类1.可重入函数使用全局变量时，应通过关中断、信号量（即P、V操作）等手段对其加以保护2.只对简单全局变量（如整型）进行读操作的函数可以不申请互斥保护，但操作复杂全局变量（如链表，树）时，即使仅仅读取，也必须申请互斥保护，因为存在中途被其它任务破坏数据结构的可能性3.任务切换需重点关注数据溢出类1.强制的数据类型转换，是否导致了数据的丢失？2.是否数据精度不足，导致可能发生数据溢出或翻转？3.如对计数变量，一般是进行定时读取，要保证在定时器超时前数据不能溢出。,Page8,分类专项检视6,差1错误1.此类错误一般是由于把“=”误写成“”等造成的2.如：VlanID有效值为14095，通常会定义宏，比如MAX_VLAN_NUMBER为4096，这时需要注意在参数合法性判断时，需要用“=MAX_VLAN_NUMBER”，不要误用“MAX_VLAN_NUMBER”。返回值类1.函数返回值的类型需要和函数声明的类型一致2.在异常情况下，需要返回不同的错误码，而不是返回OK3.函数返回值returnok、error、true、false是否用混？,Page9,分类专项检视7,代码审查九字真言,看见了If，就想Else。看见malloc，就去找Free。函数调用要小心，需要看看返回值。看到for循环，就找边界值。看见return要注意，要去前面找资源。看见数组把神提，问题往往在下标。不要小看字符串，长度是个大问题。得到函数不要急，看看变量初始化，各种路径要小心。赋值函数最危险，变量没有初始化。九句句真言不孤立，相互结合显神威。,Page10,真言详解1,看见If，就想Else看到if语句，就要想到else语句。如果没有else语句，就要分析是不需要，还是异常情况没有处理，如果是异常情况没有处理，可以提单。看见malloc，就去找Free看到malloc语句分配了内存，立即停下正常走读，看malloc代码之后，是否在所有程序的返回分支中都有释放语句。函数调用要小心，需要看看返回值到函数调用，要养成习惯，进入函数内部瞄一眼。看看函数的正常值和异常值都是什么。看看返回值需不需要判断。看看有没有参数理解不一致的地方。,Page11,看到for循环，就找边界值看到for循环，就要看看边界值是否合理。如果循环变量是数组的下标，更加需要注意。看见return要注意，要去前面找资源看见return语句，尤其是函数中间的异常返回语句。看到这种语句，就需要折回头去看看前面有没有分配资源。前面分配的任何资源（包括内存，端口，等等），在异常返回处需要一并释放。看见数组把神提，问题往往在下标函数中一旦出现数组，就要提起精神。数组越界可是个致命问题。,Page12,真言详解2,不要小看字符串，长度是个大问题字符串往往是代码审查中不被重视的问题。而字符串由于各个模块对其他模块的不了解，经常出现随便定义一个字符串长度的现象。得到函数不要急，看看变量初始化，各种路径要小心在拿到函数之后，要习惯性的看看所有的局部变量是在函数一开始就被初始化了如果有变量没有被初始化，就要小心了。如果发现变量是在if，for，while，等语句中被初始化的，问题可能就来了。赋值函数最危险，变量没有初始化在C代码中，经常使用将变量的指针作为参数，在被调用函数中对变量进行赋值的做法。这种程序的写法是标准的C语言的用法，无可厚非。但这种做法隐藏着极大的危险。,Page13,真言详解3,触发式检视1,VOS_Mem_Zero(后继判断p是否为空指针了吗？空指针问题#defineAAA_AUTH_NONE0 x07#defineAAA_AUTH_PPP0 x10注意这是16进制数没有特殊的情况，建议用枚举来定义,Page14,触发式检视2,ulIndex=pMsg-ulIndex;p=之前判断ulIndex是否越界了吗？内存越界问题while(i+”与”=”等#defineMAX_NUM(TOTAL_LENHEAD_LEN/ITEM_LEN)似乎少了对括号？注意运算顺序，要加括号,Page15,触发式检视3,while(ipNext;断言的作用？断言并不能保你不死。经常看到有人这样写,Page16,触发式检视4,while(pNext!=NULL)continue;pNext=pNext-pNext;所有continue的地方pNext都有取下一个吧？死循环问题，强烈建议用for来做循环消除此类问题p=VOS_Malloc(MID_XXX,ulSize);return所有return的地方都释放内存了吗?内存泄漏问题,Page17,触发式检视5,if(ulIndexMAX_NUM)returnVOS_ERR应该是“Switch语句有default分支吗？每个case都有break吗?if(0=ucCount|256pNext=pDel-pNext;pDel=NULL;pDel指向的内存释放了吗？双向链表及其它类似情况都要考虑,Page19,触发式检视7,vos_printf(szString,“%d”,ulIndex);ulIndex是无符号数，你确信用“%d”而不是“u”？会打印出负值if(ulItemCount0)ulItemCount=ulItemCount2；统计量有没有做翻转保护?数值翻转问题。同样加的时候也要注意UCHARszStringMAX_LEN;UCHARszNameMAX_LEN;VOS_sprintf(szString,“user-%s”,szName);打印后变长了，内存会不会越界？为了省事而把用同一个宏定义字符串,Page20,触发式检视8,if(ulMtu=9018).9018为何没有定义为宏？用宏或枚举来代替魔鬼数字/*是否有用户在线？*/if(VOS_OK=AAA_IsUserOnline().确信函数返回的是VOS_OK而不是VOS_YES？误导人的的函数名：函数应该返回“是/否”还是“成功/失败”呢？,Page21,触发式检视9,typedefstructtagAaaUserUCHARucFlag;ULONGulCid;AAA_USER_S;结构4字节对齐了吗?,Page22,触发式检视10,externAAA_GetUserCid(USHORT*pusCid);AAA_GetUserInfo(USHORT*)参数类型不匹配，PC-LINT不通过，我只好做个强制转换，没问题吧？内存越界问题if(ulCount=0)“”应该是”吧？建议把常量放在前面,Page23,触发式检视11,#defineAAA_XXX(a,b)(a)=(a)*(b)所有的参数都加括号了吗?宏展开之后的错误if(!afor(i=0;iMAX_NUM;i+)VOS_StrCat(szOutString);会打印越界吗？2048虽然很大，但不能保证不会越界,Page25,触发式检视13,UCHARszOutString204800;for(i=0;iip_usLen;.报文处理做字节序转换了吗？从报文中取出后要做NTOH转换,填写报文时要做HTON转换,Page29,触发式检视17,For(i=0;i1000000000;i+)./verycomplexVOS_TaskDelay(100);长时间的循环中，有主动释放CPU的操作吗？我们是实时系统，这个不能忽略。时间太久系统会自动复位!if(.)elseif()elseif().最后有else分支吗？相当于switch语句中的default,Page30,触发式检视19,/*如果用户认证不成功*/if(VOS_ERR=AAA_UserAuth(ulCid)/error;应该是”VOS_OK!=”吧？注意这两种写法并不是等价的，因为可能还有其它的错误类型VOS_sprintf(szOutString,“%s”,szTemp);VOS_sprintf(szOutString,szTemp);这两种写法有区别吗？第1种写法是安全的，第2种写法时，如果szTemp含有转义字符(如”%s”)，则极度危险，严重至死机,Page31,触发式检视20,#defineMAX_NAME_LEN64UCHARszUserNameMAX_NAME_LEN；64这个长度包不包括最后的0呢？那就是说名字最长是63咯？这样定义通常会让人迷惑到底是多长，建议的做法是定义成(64+1)。参考aaa_pub.hpMsg=VOS_Malloc();if(VOS_OK!=AAA_SendMsg(pMsg)else发送失败情况下pMsg在哪里释放的？发送成功情况下pMsg在哪里释放的？内存泄漏问题,Page32,触发式检视21,if(g_stVar.ulTimerId!=0)VOS_Timer_Delete(g_stVar.ulTimerId);删除定