12个C语言的程序问题

下面的12个C语言的语句是我从实际的应用系统中摘录和简化来的。这些语句都被我列入丑陋代码的类别中。对此，我给出了相应的改善程序和说明。

1.
if ( strlen(s) == 0 )
if ( strlen(s) )
if ( strcmp(s, “”) == 0 )

改善：
# define isNotEmpeyStr(s) ( (s)[0] != ‘/0’ )
# define isEmptyStr(s) ( (s)[0] == ‘/0’ )

这样可以用上面两个宏来判断字符串是否为空，或者字符串是否不为空。
当然，也可以直接写 if ( s[0] == ‘/0’ ) 或者 if ( s[0] != ‘/0’ )，但使用宏，有更好的可读性。

2.
memcpy(s1, s2, 1);
对于一个字节的拷贝，竟然也调用memcpy。看来，C语言可以删除赋值的语法功能了。
改善：
s1[0] = s2[0];

3.
for ( i = 0; i < strlen(s); i++ )
每次循环的循环判断中都有strlen的函数调用，显然降低效率。
改善如下：
len = strlen(s);
for ( i = 0; i < len; i++ )

4.
printf("OK/n");
对于没有格式控制的输出，最好使用puts，fputs等替代。这样有更高的效率。
puts("OK");

5.
char  sbuf[1024];

memset(sbuf, 0, sizeof sbuf);
sprintf(sbuf, "select %s from %s where %s = ", col_name, table_name, key);
strcat(sbuf, value);

这里，memset实属多余。而strcat需要扫描 sbuf到有字符串的结尾，然后进行拷贝。
改善如下：
char sbuf[1024];
int  pos;

pos = sprintf(sbuf, "select %s from %s where %s = ", col_name, table_name, key);
strcpy(sbuf + pos, value);

6.
str[strlen(str)] = '/0';
写这个语句的程序员大概是被字符串以'/0'结尾的问题搞昏头了。它忘记了一个常识：在有尾巴的地方设置一个尾巴，不是一个多余的行为吗？

7.
sprintf(curtime, "%02d:%02d:%02d:%03d", tt->tm_hour, tt->tm_min, tt->tm_sec, tb.millitm);
curtime[12] = 0;

sprintf函数本身就能在curtime 的后面添加0， 那句curtime[12] = 0的语句实属多余。另外，佩服写这个程序的人，还能算清楚那个字符的长度是12。

8.
if ( ch == 1 )   {
   ..........
}
if ( ch == 2 )    {
   ...........
}
if ( ch == 3 )    {
  ..........
}

改善如下：
if ( ch == 1 )   {
   ..........
} else
if ( ch == 2 )    {
   ...........
} else
if ( ch == 3 )    {
  ……………
}

如果，中间的处理不复杂的话，可以使用switch语句。

9.
nRet=lockWait(c_sem_id);
if(nRet!=0)
{
  usleep(100000);
  nRet=lockWait(c_sem_id);
  if (nRet!=0)
{
    usleep(100000);
    nRet=lockWait(c_sem_id);
    if(nRet!=0)
   {
      logger->fatal("lock Virtual teller config failed”);
      nRet=unlockWait(c_sem_id);
      return -1;
    }
  }
}

这段程序可真快把人雷倒了。大概这个程序员还不知道程序语言的循环语句。不知道如果要求调用lockWait过程是100次的话，他是不是会一直这样写下去。 改善后的代码如下：

const int trytimes = 3;
int try = 0;

while( ( (nret = lockWait(c_sem_id)) != 0 ) && (try++ < trytimes) )
    usleep(1e5);

if ( nret < 0 ) {
  unlockWait(c_sem_id);
  logerr->fatal("lock Virtual teller config failed");
  return -1;
}

10

#define SBUF_DBLMIN (-1e20) /* 最小double值 */

char sbuf[16];
double double_val;

………………接收一字符串到sbuf中。
double_val = atof(sbuf);
if ( double_val == SBUF_DBLMIN )
  return -1;

这个程序员实在没有搞清楚atof出错的返回问题，使用浮点数与自己定义的所谓最小值比较，更是异想天开的想当然。
关于这个问题，我在另一篇文章中 有更详细的论述。

11
下面是我从一个复杂的输出一系列二进制数据为16进制字符的一段简化的程序，其中c为简化的这个二进制数据的一个字节。

char sbuf[3];
char c;
c已经存放了一个字节的二进制数。
sprintf(sbuf, “%.2x”, c);

我们考虑一个情况，当c的最高位为1的时候，将会出现什么情况。比如： c的值为 0xff。那么它输出到 sbuf里的字串是 ‘ff”吗？那你就和程序的作者犯同样的错误了。
我们知道，在sprintf中，c是作为整型数传给sprintf函数的。而对于char c的变量，当它的值为 0xff时，其实相当于十进制数的-1。想想看，
sprintf(sbuf, ‘%2x”, -1); 会产生什么结果。
也许你现在该明白了，-1的整型数为 0xffffffff，这是假设int是四个字节的情况。那么它输出到sbuf，应该是字串: “ffffffff”。一共是8个字节的长度。而sbuf却只定义有3个字节的空间。显然，会产生溢出。

这个问题，可以修改sprintf调用如下：
sprintf(sbuf, “%.2x”, (unsigned char)c);

12
void rtrim( char* str )
{
  while( strlen(str) && str[strlen(str)-1] == ' ')
    str[strlen(str)-1] = '/0';
}

void ltrim( char* str )
{
  while( str[0] == ' ' )
    strcpy( str, str+1 );
}

这两个函数看起来显得整洁清晰。甚至程序的作者还引以为自豪，感觉写得很漂亮。但是，他忘记了C语言不是普通的高级语言，它是面向程序员的高效率的语言。对于这样的基础实用函数的写法，正可以看出对于C语言掌握的功底。
在rtrim中，循环的使用strlen调用，严重影响了程序的效率。
在ltrim中，循环体中的strcpy便将这段程序打落到丑陋之列。

下面是我写的程序，看起来比原来的还长。

void rtrim(char *str)
{
  char *ps = str;

  /* find <str> tail */
  while( *ps != '/0' )
    ps++;

  ps—;
  while( (ps >= str) && (*ps == ' ') )
    ps—;

  *(++ps) = '/0';
}

void ltrim(char *str)
{
  char *pd = str;

  while( *str == ' ' )
    str++;

  while( (*pd++ = *str++) != '/0' );
}