4.1 代码检视实例
看完上面的评审检视要点后，也许有些读者已经急切地想找一些代码来试验一下看能否通过上面的内容来提高自己的检视能力。下面就讲几个代码检视的实例，请读者在看这些实例时先不要看后面的分析，自己先拿张纸边看代码边把自己能够发现的问题记录下来，然后再和后面的分析进行比较。如果能够发现后现分析中没有提及的问题，说明已经将评审检视基本功学得非常好了。
4.1.1 日志文件函数的检视实例
日志文件函数的代码如下：
/** 写日志函数，实现将日志信息写入日志文件中的功能
@param char *pszLogFile - 日志文件名
@param char *pszLogMsg - 要写入的日志消息字符串
@return int - 失败返回-1, 成功返回0
*/
int WriteLog (char *pszLogFile, char *pszLogMsg)
{
FILE *fp;
fp = fopen(pszLogFile, "w");
fseek(fp, 0, SEEK_END);
if ( fwrite(pszLogMsg, sizeof(char), strlen(pszLogMsg), fp)
!= strlen(pszLogMsg) )
{
return -1;
}
fclose(fp);
return 0;
}
下面按照检视要点来对这个函数进行检视：
n 从输入校验方面的检视要点来看的话，输入参数没有校验。即没有判断输入的文件名是否合法或是否为空。文件名中有许多特殊字符是不能使用的，会导致打开文件失败。输入的日志消息字符串是否为空也没有判断。
n 从函数方面的检视要点可以发现打开文件函数的返回值没有进行校验，即没有判断打开文件是否成功，
n 从资源释放方面的检视要点来看，打开的文件在return -1;这行之前没有被关闭。
n 再从系统调用方面的检视要点来看，调用fopen使用不正确的参数，实际上第二个参数不能使用”w”，”w”参数会将文件内容清空掉，以前写的信息都会丢失，应该改成“a+b”。改掉这个参数后，fseek()那个调用就可以不要删除掉了。
n 除了上面的检视要点外，还需要使用元素分析法来进行检查，从日志文件这个元素进行分析，两条日志信息间应该有分隔符分开的，因此需要对pszLogMsg判断是否有分隔符，如果没有的话，程序中应该给它补上分隔符。
所以总共可以得到上面写日志函数的几个主要问题如下：
1) 输入参数pszLogFile, pszLogMsg没有进行校验
2) 打开文件后没有判断是否打开成功
3) fopen()第2个参数使用不正确，应该使用“a+b”作为参数
4) return -1;前没有将打开的文件关闭掉
5) 日志文件中的两条日志信息间的分隔符没有考虑进行处理
4.1.2 求和函数的检视实例
一个整数求和函数的代码如下：
/** 整数求和函数，计算0到指定正整数(包括指定整数)间所有整数的和
@param int num - 指定的整数求和的上界
@return int - 求得的整数之和
*/
int Sum( int num )
{
int i, sum;
for ( i =0; i < num; i++ )
{
sum += i;
}
return sum;
}
这个程序看起来很简单，寥寥两三行程序，初看起来不应该有多少问题，还是检视一下看看它到底存在多少问题。
n 先分析以上代码中的变量初始化问题，显然sum变量没有被初始化
n 进行一致性检查发现函数是求正整数之和，函数参数应该定义成unsigned int类型才更确切
n 根据计算方面的检视要点进行分析，发现当num较大时可能会造成整数上限溢出，要解决这个溢出问题，有三种方法，一是将参数的类型改成unsigned short或者将返回值改成64位整数类型，或者在程序中对输入参数进行上限校验，校验失败返回-1表示失败。
n 进行循环边界检查发现i<num应该为i<=num
因此检视出来的主要问题如下：
1) 变量sum没有初始化
2) 函数参数类型定义不准确
3) 当参数num的值大到一定程度时会造成整数上限溢出
4) 循环中的条件i<num写错，应为i<=num
4.1.3 字符串处理函数的检视实例
有一个字符串处理函数的代码如下，请检视出其中的问题。
/** 删除字符串最右边的特殊字符(包括空格、回车符、换行符、TAB字符)
直到遇到非特殊字符为止
源字符串不能修改，删除特殊字符后的字符串放到返回值中
@param char *str - 源字符串指针
@return char * - 返回删除特殊字符后的字符串，
失败或者删除特殊字符后字符串内容为空时返回NULL;
*/
char * StrTrimRight(char *str)
{
int len;
char *psz;
if ( str != NULL )
{
return NULL;
}
len = strlen(str);
psz = str + len;
while ( *psz == '/r' || *psz == '/n'
|| *psz == ' ' || *psz == '/t' )
{
psz--;
len--;
}
psz = (char *)malloc(len + 1);
strncpy(psz, str, len);
return psz;
}
n 先从校验的检视要点看，对输入的函数参数进行了校验，看上去好像函数输入参数校验方面没有什么问题。
n 从函数方面的检视要点来看，调用的malloc函数没有对返回值进行校验
n 从循环条件方面看，循环是否会中止呢？只有碰到不等于特殊字符中的字符才会结束，如果一个字符串中只有特殊字符的情况下，循环会执行到psz越过字符串str的头部，导致内存越界，将导致异常出现，或者异常没有出现而此时如果越界后的字符凑巧为特殊字符的话，将导致len变成负数，后果都将不堪设想，因此需要将循环的中止条件再加上对len是否小于等于0的判断，即加上 并且 len >0 的条件
n 从计算方面看，psz = str + len 使得psz 指向了str字符串尾部的’/0’字符，会导致后面的循环内的代码不被执行，将没有把特殊字符删除掉就复制到返回值去了。
n 由于是字符串处理函数，因此下面着重从字符串的检视要点进行检查，首先考虑字符串内容为空的情况，在这种情况下，按照注释里的说明，应该返回NULL的，所以漏掉了对输入参数内容为空的校验，需要增加一句 if ( *str == ‘/0’ ) return NULL;
n 从系统和库调用方面来看，strncpy函数不会在复制后的字符串尾部添加’/0’，因此最后还需要在复制完后添加上’/0’
n 从边界进行分析，变量len具有边界条件，它的下界是0，当len为0时，显然删除特殊字符后的字符串内容为空，此时需要返回NULL，不需要分配内存和调用strncpy操作，因此遗漏了删除特殊字符后字符串为空的情况的处理。
因此总共可以发现这个函数有以下一些主要问题：
1) psz = str + len; 导致psz指向字符尾部的’/0’字符，引起程序后面出错 应该为 psz = str + len - 1;
2) while 循环条件有问题，某些情况下会导致异常，应该加上 len 是否大于0的判断
3) 应加上对输入字符串内容是否为空（首字符是否为’/0’）的校验
4) malloc()函数分配内存后没有判断是否成功
5) strncpy没有给复制后的字符串尾部添加’/0’字符，需要另行添加
6) 循环完后没有判断删除后字符串的内容是否为空，应通过len是否大于0来进行判断
4.1.4 网络服务函数的检视实例
下面是一个初学socket编程的人找了一段样例代码改写成的一个简单的TCP服务器程序，服务器收到客户端连接请求后，和客户端建立连接，接收客户端发送来的一个字符串“Hello!”，然后发送回客户端一条表示成功的信息，并告诉客户端是服务器运行以来的第几个访问者。程序由两个函数组成，程序如下，为方便后面描述，给每行代码前加上了行号。
1、 void ClientTask(LPVOID args);
2、
3、 /** 创建服务器程序，如果服务器收到客户端发送的“Hello!"字符串
4、 服务器应该返回给客户端一串"Succeed. you are the %dth visitor."信息
5、 其中的%dth表示客户端是第几个访问者
6、
7、 @return int - 成功返回0，失败返回-1
8、 */
9、 int ServerCreate()
10、 {
11、 struct sockaddr_in local, from;
12、 SOCKET listen_socket, connect_socket;
13、 int from_len;
14、
15、 local.sin_family = AF_INET;
16、 local.sin_addr.s_addr = INADDR_ANY;
17、 local.sin_port = 1001;
18、
19、 listen_socket = socket(AF_INET, SOCK_DGRAM, 0);
20、
21、 bind(listen_socket, (struct sockaddr *)&local, sizeof(local));
22、
23、 for(;;)
24、 {
25、 from_len = sizeof(from);
26、 connect_socket = accept(listen_socket, (struct sockaddr *)&from,
27、 &from_len);
28、 if ( connect_socket != INVALID_SOCKET )
29、 {
30、 _beginthread(ClientTask, 0, &connect_socket);
31、 }
32、 }
33、 return 1;
34、 }
35、
36、 /** 服务器端处理客户端响应的任务处理函数
37、
38、 @param LPVOID args - 和客户端连接的socket的地址指针
39、 @return void - 无
40、 */
41、 void ClientTask(LPVOID args)
42、 {
43、 char szBuf[1024];
44、 static int number;
45、
46、 SOCKET s = *(SOCKET *)args;
47、
48、 recv(s, szBuf, sizeof(szBuf), 0);
49、
50、 if ( strcmp(szBuf, "Hello!") == 0 )
51、 {
52、 char szMsg[1024];
53、
54、 number++;
55、 sprintf(szMsg, "Succeed. you are the %dth visitor.", number);
56、
57、 send(s, szMsg, sizeof(szMsg), 0);
58、 }
59、 return;
60、 }
先检视ServerCreate()函数，
n 从函数方面的检视要点看， socket()和bind()都没有处理返回值，是否成功不知道
n 从常量方面的检视要点看，第17行使用了魔法数字1001，应使用宏定义如 #define SERVER_PROT 1001或者const unsigned short SERVER_PORT = 1001;
n 从网络功能方面的检视要点看，程序17行中端口要将主机顺序转换成网络字节顺序，应为htons(SERVER_PORT);
n 从系统调用方面的检视要点来分析， 第19行的socket()函数第二个参数错误，TCP服务器应该为SOCKET_STREAM，另外TCP服务需要调用listen()函数设置侦听队列大小，程序中遗漏掉了，应该在第21行和23行间补上
n 从注释的检视要点看，第7行注释描述的返回值和实际函数里不一致，实际函数在最后一行第33行返回了1，而注释描述的是成功返回0
n 从全局变量的检视要点进行分析，传入客户任务的变量connect_socket可以看成是一个对全体客户处理任务的全局变量，将connetc_socket的地址传入客户端处理任务中，如果第一个任务还没有运行到第46行，又有客户端进行连接时，第26行的赋值会将传入第1个任务的connect_socket覆盖掉，实际上不应该传入地址，只传值就够了，否则可能造成异常情况。
再检视ClientTask() 函数
n 从静态变量的检视要点看，变量number没有初始化为0
n 从计算的检视要点看，number变量存在累积增加问题，在32位系统中连接的数量超过231后就存在整数上限的溢出问题。如果以每分钟响应1万个客户端计算，运行到149天多一些时就会产生溢出。
n 从字符串的检视要点看，接收的数据放到szBuf缓冲区中，需要给字符串结尾添加'/0'字符
n 从网络方面的检视要点看，第48行中，如果客户端发送一个超过1024长度的字符串，会导致字符串尾部没有’/0’型的溢出，48行中第3个参数应该写成sizeof(szBuf)-1；接收数据时没有处理接受失败时的情况，如客户端连接成功后没有发送数据就关闭socket等情况会导致服务器端的接收失败。SOCKET是否会阻塞没有考虑，没有进行发送接收的超时处理。发送数据前要判断是否可以发送才能进行发送，否则可能造成阻塞，发送后也没有判断是否发送成功。
n 从资源释放的检视要点看，ClientTask处理完客户请求后没有将socket关闭
n 从全局变量与共享变量的检视要点看，number需要加锁保护
n 从初始化的检视要点看，number变量没有初始化
n 从函数的检视要点看，发送接收函数没有判断函数返回值，没有处理失败情况
n 从系统调用和库调用的检视要点看，第57行中发送函数的第3个参数错误，应该为strlen(szMsg)
n 从数组使用的检视要点看，第52行szMsg数组空间1024过大，远超过实际要使用的大小，实际数据大小不会超过64字节，定义成64或128大小的空间就足够了
这样可以得到ServerCreate()函数主要问题如下：
1) 第17行使用了魔法数字1001，应改成宏定义
2) 第17行的端口没有进行网络字节序转换
3) 第19行和21行的socket()和bind()函数的返回值没有处理
4) 第19行socket()函数第二个参数错误，应为SOCKET_STREAM
5) 在第21行和第23行间漏掉了listen()调用
6) 第30行_beginthread()函数第3个参数传入connect_socket变量地址有多任务读写问题，不应传地址，而应该传值
7) 返回值和注释中说明不一致
ClientTask()函数主要问题如下：
1) 第44行number变量没有初始化为0
2) number变量可能存在整数上限溢出问题
3) 第48行收到数据放到szBuf后，没有给字符串添加尾部字符’/0’
4) 第48行第3个参数sizeof(szBuf)应改为sizeof(szBuf)-1
5) 第48行接收数据函数没有处理接收失败的情况
6) 第52行szBuf空间过大，定义成64或128的大小就足够了
7) 第54行和第55行的number变量读写都存在重入问题，需要进行锁保护
8) 没有处理接收和发送数据超时的情况，存在着无限等待的问题
9) 收发数据前没有判断是否可以收发数据
10) 发送数据后没有判断是否发送成功，没有对返回值校验
11) 第57行发送函数的第3个参数错误，应改为strlen(szMsg)
12) 最后处理完后没有关闭连接的socket
以上发现的问题大部分都是一些常规性的问题，如果从安全性编程等角度去分析的话，还有更多的问题在里面。读者如果有兴趣可以再去试着发现一些更深入的问题。
from：http://blog.csdn.net/drzhouweiming/article/details/1519048