华为代码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有效值为1 4095 通常会定义宏 比如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 LEN HEAD 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 应该是 还是 条件判断错误 Page18 触发式检视6 switch a case break Switch语句有default分支吗 每个case都有break吗 if 0 ucCount 256pNext pDel pNext pDel NULL pDel指向的内存释放了吗 双向链表及其它类似情况都要考虑 Page19 触发式检视7 vos printf szString d ulIndex ulIndex是无符号数 你确信用 d 而不是 u 会打印出负值if ulItemCount 0 ulItemCount ulItemCount 2 统计量有没有做翻转保护 数值翻转问题 同样加的时候也要注意UCHARszString MAX LEN UCHARszName MAX LEN VOS sprintf szString user s szName 打印后变长了 内存会不会越界 为了省事而把用同一个宏定义字符串 Page20 触发式检视8 if ulMtu 9018 9018为何没有定义为宏 用宏或枚举来代替魔鬼数字 是否有用户在线 if VOS OK AAA IsUserOnline 确信函数返回的是VOS OK而不是VOS YES 误导人的的函数名 函数应该返回 是 否 还是 成功 失败 呢 Page21 触发式检视9 typedefstructtagAaaUser UCHARucFlag 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 a b c d f g 没有括号 你能记住运算符的优先级 养成良好的编程习惯 要求作者加上括号 Page24 触发式检视12 UCHARszOutString 2048 for i 0 i MAX NUM i VOS StrCat szOutString 会打印越界吗 2048虽然很大 但不能保证不会越界 Page25 触发式检视13 UCHARszOutString 204800 for i 0 i MAX NUM i VOS sprintf szOutString 204800够大了吧 太大的局部变量导致任务堆栈被破坏 建议申请动态内存 Page26 触发式检视14 if ulItemFlagucIndex 循环退出的条件可以满足吗 死循环问题 Page27 触发式检视15 UCHARucEncryptedPasswd 128 UCHARucMacAddr 6 vos printf ucEncryptedPasswd vos printf ucMacAddr 变量中存放的并不是字符串 这样输出的有不可预知的后果 重则死机这种情况要用16进制打印ASCII码值 Page28 触发式检视16 VOS StrCpy szDest szSrc VOS StrNCpy szDest szSrc ulLen VOS MemCpy pDest pSrc ulLen 字符串或内存拷贝没有越界吧 另外要注意把Src和Dest写反的情况 ulPacketLen pstIp ip usLen 报文处理做字节序转换了吗 从报文中取出后要做NTOH转换 填写报文时要做HTON转换 Page29 触发式检视17 For i 0 i 1000000000 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 LEN64 UCHARszUserName MAX 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 删除定时器前判断合法性了吗 删除定时器后 控制句柄清0了吗 其它类型资源的控制句柄都要这样处理if pMsp NULL VOS Assert pMsg NULL returnVOS ERR 断言的 应该是 经常碰到这类错误 断言在条件为 假 的时候工作 Page33 触发式检视22 if g stVar ulMemPtr NULL VOS Free g stVar ulMemPtr 释放内存前判断指针合法性了吗 释放内存后把指针清空了吗 内存重复释放问题 空闲内存被改写问题等严重后果 Page34