代码审查技术烽火.pptx

代码审查技术学习目标掌握代码分析和评审技术掌握自动化代码分析工具的使用建筑工程师的第一堂课-关注细节关注并弄清楚桥梁修建细节，否则你建起来的桥就有可能坍塌。-安全编程 代码静态分析1940年Tacoma Narrows大桥600英尺的一段坍塌落入普吉特湾。代码审查对成本的节省黑盒测试的缺陷黑盒测试既不充分，而且效率也低。在系统完成之前，测试就无法开始，测试人员只有软件版本发布时才能拿到版本进行测试。Static white-box testingFind bugs earlyFind bugs that would be difficult to uncover or isolate with dynamic black-box testingCost effectiveSide benefit：give black-box tester ideas for test cases to apply when they receive the software for testing不懂开发怎么做代码审查？霍元甲霍元甲因为他本人身体虚弱，所以父亲从小不让练武功，而生长在那样的环境中，他天天可以看到兄弟们在练功，招式已经记忆在心里，但是苦在没有练功的机会，他利用体力劳动的过程中，改变劳动方式，趁机练功，后来发展到独创“迷踪拳”。代码静态分析静态分析是指在不执行的情况下对代码进行评估的过程。包括：-类型检查-风格检查-程序理解-BUG查找-安全审查静态分析-类型检查在Java中，下面的语句虽然符合类型检查规则，但是会在运行时失败，抛出一个ArrayStoreException异常:Object objs=new String1;objs0=new Object();/lint-w2/lint+e734#include stdafx.hint main()char ch=0;int n=0;/.ch=n;return 0;VC6编译通过，但是PCLint可以通过静态代码检查找出类型转换造成的精度丢失问题静态分析-风格检查常见工具C/C+：PC-LintJAVA：PMD.NET：StyleCop风格检查更加挑剔，也更加注重空格、缩进、命名、注释、程序结构这些表面的东西。风格检查程序所展示的错误往往都是影响代码的可读性和可维护性的问题。typedef enum red,green,blue Color;char*getColorString(Color c)char*ret=NULL;switch(c)case red:printf(red);return ret;gcc的“-Wall”选项将检查出其中的问题。typedef const char*CSTRING;CSTRING revere(int lights)CSTRING manner=by land;if(lights 0)if(lights=2)manner=by sea;else manner=;return manner;int main()printf(The British are coming%sn,revere(1);return 0;静态分析-程序理解程序理解工具能帮助我们搞懂代码库中的大量代码，洞察程序运转之道。集成开发环境（IDE）一般至少都包含某些程序理解功能，例如：“查找本方法的所有应用”。常用工具：-代码流程图：Code Visual to Flowchart-UML与源代码双向工程，例如FujabaFujiaba 能在UML视图和源代码之间来回转换静态分析-Bug查找BUG查找的目的不像风格检查那样抱怨格式方面的问题，而是根据“BUG惯用法”（规则）来描述代码中潜在的缺陷。常用工具：PMD、FindBugs、Coverity、KlocworkEmptyCatchBlock:Empty Catch Block finds instances where an exception is caught,but nothing is done.In most circumstances,this swallows an exception which should either be acted on or reported.Example:public void doSomething()try FileInputStream fis=new FileInputStream(/tmp/bugger);catch(IOException ioe)string str=;for(int i=0;i h_name,tHost,sizeof(tHost)trusted=true;elsetrusted=false;黑名单 vs.白名单黑名单法（black listing）：尝试枚举所有不能接受的输入值。白名单法（white listing）：通过已知正确值清单来进行检查。下面是摘自Tomcat5-1.31版本中的一段程序，采用了一个黑名单，假定在Web页面中只有4个特殊的字符输入才会导致程序的安全问题：for(int i=0;i content.length;i+)switch(contenti)case:resule.append(>);break;case&:resule.append(&);break;case:resule.append(");break;default:result.append(contenti);黑名单法只拒绝已知恶意的数据，而在给定环境中，恶意值的集合通常是难于枚举的（或者可能是无限的），所以黑名单法一般是不完善的。即使可能列出一个危险输入值的完整清单，也很可能随着时间的变迁而过时。黑名单法是不利于应用程序安全的。审查缓冲区溢出错误缓冲区溢出（buffer overflow）：将大量数据塞入一个较小的缓冲区中所导致的程序漏洞缓冲区溢出通常被攻击者用于重写内存中的数据。近10年来，缓冲区溢出漏洞的数量没有明显下降的趋势。void trouble()int a=32;char line128;gets(line);缓冲区溢出就是将长度超过缓冲区大小的数据写入程序的缓冲区，造成缓冲区的溢出，从而破坏程序的堆栈，使程序转而执行其他指令。当一个超长的数据进入到缓冲区时，超出部分就会被写入其他缓冲区，其他缓冲区存放的可能是数据、下一条指令的指针，或者是其他程序的输出内容，这些内容都被覆盖或者破坏掉。可见一小部分数据或者一套指令的溢出就可能导致一个程序或者操作系统崩溃。天生危险的函数gets、cin、scanf、strcpy、wcscpy、lstrcpysprintf、fprintf、swprintf禁用函数列表Linux和Windows平台上替换C类库的字符串解决方案GNU/LinuxBstrlib:http:/FireString:http:/firestuff.org/wordpress/projects/firestringGLib:http:/developer.gnome.org/doc/API/2.0/glib/index.htmlLibmib:http:/ CRT:http:/ overflow）。“回绕”unsigned long readamt;readamt=getstringsize();if(readamt 1024)return-1;readamt-;buf=malloc(readamt);如果getstringsize返回0，则readamt-1将等于4294967295（无符号32位整数的最大值），这个操作可能会因为内存不足而失败。检测整数溢出IntSafe数学函数对于C语言，微软开发了IntSafe类库，它可以帮助检测和防止整数操作中的漏洞IntSafe函数库包含200多个转换函数审查异常处理错误char buf10,cp_buf10;fgets(buf,10,stdin);strcpy(cp_buf,buf);如果发生I/O错误，fgets将不给buf添加NULL终止符。在buf中缺少NULL终止符将可能会导致在后续的strcpy调用时发生缓冲区溢出。Try catch finallyC+和Java都支持try/finally的语法。不论异常是否抛出，finally模块总是在try模块后执行。但是，如果finally模块包含一个return语句，将抑制异常抛出。审查资源泄漏问题未能及时释放资源（包括数据库对象、文件句柄和套接字）可能会导致严重的性能问题。在C程序中，在单个函数体中查找多个return语句，通常是分布式错误处理代码的标志，也是资源泄漏的滋生地。函数有多个出口时，没有在每个出口处对动态申请的内存进行释放。一般在异常处理时容易出现这种错误。下面的代码段就是这样的例子：.pRecord=new charpTable-GetRecordLength();assert(pRecord!=NULL);if(pTable-GoTop(FALSE)!=DBIERR_NONE)return;/如果从这里返回，pRecord将得不到释放.pTable-Close();delete pRecord;char*getBlock(int fd)char*buf=(char*)malloc(BLOCK_SIZE);if(!buf )return NULL;if(read(fd,buf,BLOCK_SIZE)!=BLOCK_SIZE)return NULL;return buf;如果read调用失败，将导致已经分配的buf内存泄漏！tryStatement stmt=conn.createStatement();ResultSet rs=stmt.executeQuery(CXN_SQL);harvestResult(rs);stmt.close();catch(SQLException e)logger.log(Level.ERROR,”error executing sql query”,e);如果在执行SQL或处理结果时出现了异常，那么stmt对象将不能被关闭。假如这种情况经常发生，数据库将耗尽可用的指针，不能执行任何其他的查询。C/C+代码审查C/C+语言编码规范C/C+常见代码问题检测循环语句的效率for(row=0;row100;row+)for(col=0;col5;col+)sum=sum+arowcol;在多重循环中，如果有可能，应当将最长的循环放在最内层，最短的循环放在最外层，以减少CPU 跨切循环层的次数：for(col=0;col5;col+)for(row=0;row100;row+)sum=sum+arowcol;指针的使用 _UC *puc_card_config_tab;.Get_Config_Table(AMP_CPM_CARD_CONFIG_TABLE,&ul_card_config_num,&puc_card_config_tab,use_which_data_area );.b_middle_data_ok =generate_trans_middle_data_from_original_data(puc_card_config_tab,Ul_card_config_num).分配资源是否正确释放void WarnSvr:SaveWarnData().for(int m=0;m Csn=BufferEVENT_ALARMm.Csn;item-Position=m+(RecordsInHistoryFile-RecordsInBufferEVENT_ALARM);/If a warn with a certain Csn is not in EventFilterIndex，it is not necessary to be added to HistoryFilterIndexint item_total=EventFilterIndex.GetItemsInContainer();BOOL find_flag=false;for(int k=0;k Csn=item-Csn)find_flag=true;break;if(find_flag)HistoryFilterIndex.Add(item);if(HistoryFilterIndex.IsFull()ClearIndexEntry();PC-LintLINT工具是一种软件质量保证工具，许多国外的大型专业软件公司，如微软公司，都把它作为程序检查工具，在程序合入正试版本或交付测试之前一定要保证通过了LINT检查，他们要求软件工程师在使用LINT时要打开所有的编译开关，如果一定要关闭某些开关，那么要给出关闭这些开关的正当理由。PC-LINT是GIMPEL SOFTWARE公司的产品，其中的内容是非常广泛的，光是选项就有30 0多个，涉及到程序编译及语法使用中的方方面面。练习：PC-Lint的安装和使用1:2:char*report(short m,short n,char*p)3:4:int result;5:char*temp;6:long nm;7:int i,k,kk;8:char name11=Joe Jakeson;9:10:nm=n*m;11:temp=p=?null:p;12:for(i=0;i 0)result=1;16:else if(kk 0)result=-1;17:if(m=result)return temp;18:else return name;19:/*Off-By-One Example*/#include int main()int i;int a=1,2,3;int n=sizeof(a)/sizeof(int);for(i=0;i=n;i+)printf(a%d=%dn,i,ai);return 0;#define NStu 3000#define NCrs 400extern int const StuCourseNStuNCrs;extern double const CostBookNCrs;double total_cost()int i,j;double cost=0;for(i=0;i NStu;i+)for(j=0;j NCrs;i+)cost+=StuCourseij*CostBookj;return cost;JAVA代码审查代码审查和语法错误检查是两个不同层次的概念。语法错误是低层次、强制性的检查，任何违反语法的程序都是无法通过编译的，也就是说可运行的程序必须是语法正确的。而代码审查是高级别，非强制性的检查，它对语法正确的程序施加了更高更严格的要求，从而提升程序的可读性、降低因变量命名、方法定义、程序逻辑的不完整性等问题而导致程序的潜在出错机率，增加程序的可维护性和健壮性。switch Statement Should Include a Default Case 1.switch(formatType)2.3.case 1:4.formatStr=yyyyMMddHHmmss;5.break;6.case 2:7.formatStr=yyyy-MM-dd HH:mm:ss;8.break;9.case 3:10.formatStr=yyyy.MM.dd HH:mm:ss;11.break;12.case 4:13.formatStr=yyyy年MM月dd HH:mm:ss;14.break;15.根据Sun的编码规范，每个switch流程控制语句都必须带一个default分支，以保证逻辑分支的完整性。如果没有第1516行的default代码，代码审查将给出警告。1.switch(formatType)2.3.case 1:4.formatStr=yyyyMMddHHmmss;5.break;6.case 2:7.formatStr=yyyy-MM-dd HH:mm:ss;8.break;9.case 3:10.formatStr=yyyy.MM.dd HH:mm:ss;11.break;12.case 4:13.formatStr=yyyy年MM月dd HH:mm:ss;14.break;15.default:16.formatStr=yyyy-MM-dd HH:mm:ss;17.Accessing Static Members by the Descendant Class Name1.public class ASMO12.3.void func()4.5.ASMO1 obj1=new ASMO1();6.ASMO2 obj2=new ASMO2();7.obj1.attr=10;8.obj2.attr=20;9.obj1.oper();10.obj2.oper();11.this.attr+;12.this.oper();13.14.15.static int attr;16.static void oper()17.18.19.20.class ASMO221.22.static int attr;23.static void oper()24.25.类中所有的静态方法或变量都应该通过类名来引用，如果通过类的实例来引用这些静态的成员将影响到程序的可读性可读性。如果通过类名来引用静态变量，将容易分辨出这些成员的静态属性。因为类静态成员变量在JVM中仅存在一份，而非每个对象实例各自一份，因此静态成员变量可以看成类的成员。Complex Assignment 1.int i=0;2.int j=0;3.int k=0;4.int l=0;5.i*=+j;6.7.k=j=10;8.9.l=j+=15;10.11.i=j+20;12.13.i=(j=25)+30;14.15.i=j+20;16.17.i=(j=25)+30;往往有些程序员热衷于将Java的语法发挥到极致，以资其对Java语法精通的凭据。如果是为了练习语法、理解语法，无可厚非。但如果在需要充分协作沟通的软件项目中，简洁明了，清晰易懂将会受到推崇，晦涩难懂的语句将会受到奚落。故此，大部分的软件公司的规范都对语句的精简明了精简明了提出了要求。Complex Initialization or Update Clauses in for Loops1.for(i=0,j=0,k=10,l=-1;i 10;i+,j+,k-,l+=2)2.3./do something4.由于for循环控制语句的高度灵活性，所以for()中的代码往往是复杂晦涩代码的乐园，在for语句中以逗号分隔的赋值语句最多不应超过3个：1.for(i=0,j=0,k=10,l=-1;i 10;i+,j+,k-,l+=2)2.3./do something4.5./应改写成下面的样式6./l=-1;7./for(i=0,j=0,k=10;i cnt;i+,j+,k-)8./9./do something10./l+=2;11./Use Abbreviated Assignment Operator 1.void oper()2.int i=0;3.i=i+20;4.i=30*i;5.在可能的情况下使用缩减赋值运行符（*=,/=,%=,+=,-=,=,&=,=,和|=），因为这些语句能够提高编码录入的速度，增强代码的简洁性简洁性，同时缩减赋值运行符使某些编辑器运行得更快。1.void oper()2.int i=0;3.i=i+20;4.i=30*i;5.6./应改写成应改写成7./void oper()8./9./int i=0;10./i+=20;11./i*=30;12./Coding Style：-Multiple Statements on One Line-Place Statement in Block-Use L instead of l at the End of Integer Constants 1.i+;j+;2 3.if(val=10)6.val/=10;7.8.void func()9.long var=0 x0001111l;10.不应将多行语句写在同一行代码中。代码块应以“”框起来，虽然增长了代码，但代码结构性结构性更强。声明长整型使用大写的“L”类型指定符，而非小写的“l”，因为后者和数字1相似。Hiding Names 1.public class HideName2.3.int index;4.void func()5.6.int index;7./do something8.9.void setIndex(int index)10.11.this.index=index;12.index+;13.14.类成员变量被局部变量隐藏：因类方法体中的局部变量和类成员变量具有相同的名字，而使成员变量被屏蔽 1.public class HideName2.3.int index;4.void func()5.6.int index;/隐藏了成员变量隐藏了成员变量index，应改成另一个名字，应改成另一个名字，如如int newIndex;7./do something8.9.void setIndex(int index)10.11.this.index=index;/该语句行中带this显式引用成员变量进行赋值，审查规则将不报警 12.index+;/该语句行没有该语句行没有this显式引用，审查规则将报警显式引用，审查规则将报警13.14.Hiding Inherited Field 1.class Window2.3.protected int style;4.5.6.class Button extends Window7.8.protected int style;9.子类成员变量隐藏父类成员变量：子类成员变量和可继承的父类成员变量名字相同。1.class Window2.3.protected int style;4.5.6.class Button extends Window7.8.protected int style;/具有和父类相同的成员变量，应改具有和父类相同的成员变量，应改为另一个名字，如为另一个名字，如anStyle9.Hiding Inherited Static Methods 1.class Animal2.3.static void oper1()4.static void oper2()5.6.7.class Elephant extends Animal8.9.static void oper1()10.static void oper2()11.子类覆盖父类静态方法：和非静态的方法覆盖不一样，静态的父类方法不应被子类覆盖。成员变量和局部变量的隐藏，常常会使开发人员张冠李戴，犯一些不经意的错误，而子类隐藏父类的成员和静态变量常常是由于没有注意到父类中已经具有相同的名字而引起的，由此而生产的程序Bug由于其隐身性强，是很难被发现。1.class Animal2.3.static void oper1()4.static void oper2()5.6.7.class Elephant extends Animal8.9.static void oper1()/隐藏了父类中的静态方法，应取另一个名字，如隐藏了父类中的静态方法，应取另一个名字，如anOper1()10.static void oper2()/隐藏了父类中的静态方法，应取另一个名字，如隐藏了父类中的静态方法，应取另一个名字，如anOper2()11.Use Conventional Variable Names 1.void method(double d)2.3.int i;4.Exception e;5.char s;6.Object f;7.String k;8.Object UK;9.Object COM;10.避免用过于简单的变量名避免用过于简单的变量名除了循环体中的临时变量，及一些没有特殊意义的常见数据类型，应该尽量避免使用一个字符作为变量。那些无特殊意义且常见的数据类型，所选取的单字符变量名必须按表1进行命名。为了减少潜在的冲突，避免不必要的混淆，不允许以大写域名或国家代码作变量名。1.void method(double d)2.3.int i;4.Exception e;5.char s;/应该改为应该改为c6.Object f;/应该改为应该改为o7.String k;/应该改为应该改为s8.Object UK;/和英国国家代码相同，应改为其他的名字，如和英国国家代码相同，应改为其他的名字，如ukObj9.Object COM;/和域名相同，应改为其他的名字，如和域名相同，应改为其他的名字，如obj_110.Break Statement is Missing before Case clause 1.switch(c)2.case+:3.4.break;5.case-:6.7.case n:8.9.case :case t:10.11.break;12.根据Sun编码惯例，程序入口点从一个case进入，直接到达下一个case代码段。前一个case没有对应的break语句时，在跨过的地方必须给出一个显示的注释，表示是特定流程控制的要求，而非无意遗漏。Non-Case Label in Switch statement 1.public class CaseLabel2.3./*点*/4.public static final int POINT=1;5./*线*/6.public static final int LINE=2;7./*多边形*/8.public static final int POLYGON=3;9.10.public String getFigureType(int kind)11.12.String tempName=null;13.switch(kind)14.15.case POINT:16.LINE:17.tempName=POINT and LINE;18.break;19.case POLYGON:20.tempName=POLYGON;21.break;22.default:23.tempName=UNDEFINE;24.25.return tempName;26.27.在switch中出现非case的标签：在Java语句中有两个标签，即case分支标签，另一个则是语句标签，如果case分支标签语句误删或遗漏了case关键字，则case分支标签将变成语句标签，而编译器无法识别这个错误。Suspicious Break/Continue 1.void scan(char arr)2.3.loop:4.for(int i=0;i arr.length;i+)5.6.switch(arri)7.8.case 0:case 1:case 2:case 3:case 4:9./56的数字10.case 5:case 6:case 7:case 8:case 9:11.12.if(processDigit(arri)13.14.continue loop;15.16.else17.18.break;19.20.21.case :case t:22.23.processWhitespace(arri);24.continue;25.26.default:27.processLetter(arri);28.break;29.30.有错误嫌疑的break和continue：break和continue用于switch和循环中的跳转控制，break用于提前结束循环以及从switch中退出，break的这种“多态性”使得在循环体中内嵌switch语句时，常会带来一些隐患。即开发者本希望退出外层循环，结果却只退出内层的switch语句而已。Comparing Floating-Point Values1.void calc(double limit)2.3.if(limit=0.0)4.5.System.out.println(the float-point number is exactly 0);6.7.避免对浮点值进行等值逻辑判断避免对浮点值进行等值逻辑判断浮点数都是一定精度的数据，由于内部表示的误差，往往字面上相同的两个浮点数，其内部表示也不完全相同。故此应避免对浮点值数进行等值逻辑判断，而应采用逻辑比较判断。1.void calc(double limit)2.3.if(limit=0.0)/应改为通过和较小值比较来判断，如应改为通过和较小值比较来判断，如if(Math.abs(limit)0.0000001)4.5.System.out.println(the float-point number is exactly 0);6.7.Mixing Logical Operators Without Parentheses 1.boolean a,b,c;2.3.if(a|b&c)4.5.6.添加添加()清晰化复杂的表达式清晰化复杂的表达式写复杂的表达式时不应过度依赖运算操作符的计算优先顺序，而应养成使用“()”的好习惯，当一个逻辑表达式由多个逻辑运算组成时，应该用“()”划分不同的部分。1.boolean a,b,c;2.3.if(a|b&c)/应该替换成应该替换成if(a|b)&c)4.5.6.Member is Not Used 1.public class Unuse2.3.private String name;4.public Object value;5.6.private Object getValue()7.8.return value;9.10.11.private void print()12.13.System.out.println(getValue()+=+value);14.15.类中private的成员方法和成员变量不可能在外部类中调用，如果发现private的成员变量或方法并没有在内部的protect或public方法中使用，即这个成员永远不会在运行期得到引用，而成为一个无用的成员变量和方法。代码清单中name变量，getValue()及print()方法都是无用的方法，因为不可能通过外面的类访问到这些成员，Unuse也没有提供调用这些成员的接口，所以这些成员都可以从类中清除。Comparison always produces the Same Result 1.void handleEvent(Event e)2.if(e!=null)3.4.if(e=null)5.6.7.8.9.10.void putChar(char c,boolean isLetter,boolean isDigit)11.if(isDigit)12.boolean isLetterOrDigit=isLetter|isDigit;13.14.15.无作为的表达式无作为的表达式比较表达式总是返回相同的值。1.void handleEvent(Event e)2.if(e!=null)3.4.if(e=null)/该表达式的值永远都是该表达式的值永远都是false，因为进入这个代码段，因为进入这个代码段的的e恒为非空恒为非空5.6.7.8.9.10.void putChar(char c,boolean isLetter,boolean isDigit)11.if(isDigit)12.boolean isLetterOrDigit=isLetter|isDigit;/该表达式的值永远都是该表达式的值永远都是true13.14.15.Operation has No Effect 1.public class NoEffect2.3.private String name;4.private int index;5.6.NoEffect(String n,int index)7.8.this.name=name;9.this.index=index;10.11.12.int getPosition()13.14.int base=0;15.return index+base;16.17.18.int getModule()19.20.int x=1,y=2;21.return x%y;22.23.无效的算术运算无效的算术运算，例如：当进行加法和减法运算时，有一个操作数是0。当进行乘法运算时，乘法或被被乘数为1。当进行除法运算时，除数为1。当进行取模运算时，左边的操作数的绝对值比右边操作数的绝对值小，此时xy=x。属性赋值时，将本身的值赋给自己。Statement is Unreachable 1.int arr=new intsize;2.if(arr=null)3.4.return null;5.流程控制中存在不可到达的语句流程控制中存在不可到达的语句有些流程控制由于测试条件恒为false，则流程中的程序无法到达。1.int arr=new intsize;2.if(arr=null)/由于由于arr不为空，则该测试逻辑不可能通不为空，则该测试逻辑不可能通过，程序无法进入该程序块中过，程序无法进入该程序块中3.4.return null;5.JBuilder的Code Audits功能的使用JBuilder根据Sun的编码规范及软件开发界总结出的一套行之有效的编码习惯，对Java开发中的编码风格、声明风格、Javadoc文档注释、EJB规范、命名风格、潜在错误、编码中的画蛇添足等诸多方面进行代码审查并给出警示，以便开发人员发现这些不足和隐患予以及时更正。C#代码审查C#编码规范C#常见代码问题检测避免不必要的强制转换public static void UnderPerforming(ArrayList list)foreach(object obj in list)if(obj is Control)Control aControl=obj as Control;/Use aControl.使用字符串长度属性测试是否为空字符串string s1=test;public void EqualsTest()if(s1=)Console.WriteLine(s1 equals empty string.);优先使用交错数组而非多维数组 int,multiDimArray=1,2,3,4,5,6,7,0,8,0,0,0,9,0,0,0 ;属性不应返回数组 public class Test string nameValues;public Test()nameValues=new string100;for(int i=0;i 100;i+)nameValuesi=Sample;public string Names get return(string)nameValues.Clone();public static void Main()Test t=new Test();for(int i=0;i t.Names.Length;i+)if(t.Namesi=(SomeName)/Perform some operation.public class PhotoLibraryprivate Photo photos=null;public Photo PhotosgetPhoto copyOfPhotos=new Photophotos.Length;for(int i=0;i photos.Length;i+)copyOfPhotosi=photosi.Clone();return copyOfPhotos;/使用Photos属性：for(int i=0;i library.Photosi.Height)Console.WriteLine(Landscape);else if(library.Photosi.Width library.Pho