C/C++100个典型的Bugs

俄罗斯OOO Program Verification Systems公司用自己的静态源码分析产品PVS-Studio对一些知名的C/C++开源项目，诸如Apache Http Server、Chromium、Clang、CMake、MySQL等的源码进行了分析，找出了100个典型的Bugs。个人觉得这份列表对C/C++ 程序员有一定参考意义。与其说事后用静态工具分析，倒不如在编码时就提高自知自觉，避免这份列表上的错误发生在你的代码中，因此这里将部分摘录一些Bugs（Bug编号这里不连续，为的是对应原文的编号）并做简要说明。原文将这份Bug列表分为了几类，这里也将沿用这个思路。

一、数组和字符串处理错误

数组和字符串处理错误是C/C++程序中最多的一类缺陷类型。这也可以看作是我们为拥有高效地底层内存操作能力而付出的代价。

[#1] Wolfenstein 3D项目 -"只有部分对象被clear了"

void CG_RegisterItemVisuals( int itemNum ) {
    …
    itemInfo_t *itemInfo;
    …
    memset( itemInfo, 0, sizeof( &itemInfo ) );
    …
}

这里的Bug出现在memset那一行。代码的真实意图是clear iteminfo这块内存，但调用memset时，第三个参数传入的却是sizeof(&iteminfo)，要知道 sizeof(&itemInfo) != sizeof(itemInfo_t)，前者只是一个指针的大小罢了。正确的写法是：

memset(itemInfo, 0, sizeof(itemInfo_t)); 或memset(itemInfo, 0, sizeof(*itemInfo));

[#2] Wolfenstein 3D项目 -"只有部分Matrix被clear了"

ID_INLINE mat3_t::mat3_t( float src[ 3 ][ 3 ] ) {
    memcpy( mat, src, sizeof( src ) );
}

这里的Bug出现在memcpy一行。程序的原意是将clear src[3][3]这个二维数组。但这里有个坑：那就是作为函数形式参数的数组名已经退化为指针了，对其sizeof只能得到一个指针的长度，因此这里的 memcpy只是copy了一个指针的长度，没有copy全。这里的代码是C++代码，原文中给出了正确的改正方法 – 传reference：

ID_INLINE mat3_t::mat3_t( float (&src)[3][3] )
{
    memcpy( mat, src, sizeof( src ) );
}

[#4] ReactOS项目 – "错误地计算一个字符串的长度"

static const PCHAR Nv11Board = "NV11 (GeForce2) Board";
static const PCHAR Nv11Chip = "Chip Rev B2";
static const PCHAR Nv11Vendor = "NVidia Corporation";

BOOLEAN
IsVesaBiosOk(…)
{
    …
    if (!(strncmp(Vendor, Nv11Vendor, sizeof(Nv11Vendor))) &&
            !(strncmp(Product, Nv11Board, sizeof(Nv11Board))) &&
            !(strncmp(Revision, Nv11Chip, sizeof(Nv11Chip))) &&
            (OemRevision == 0×311))
    …
}

Bug处在IsVesaBiosOK中那一串strncmp调用中，代码将一个指针的size传入strncmp作为第三个参数，导致 strncmp实际只是比较了字符串的前4 or 8个字节，而不是字符串的全部内容。

[#6] CPU Identifying Tool项目 – 数组越界

#define FINDBUFFLEN 64  // Max buffer find/replace size
…
int WINAPI Sticky (…)
{
    …
    static char findWhat[FINDBUFFLEN] = {'\0'};
    …
    findWhat[FINDBUFFLEN] = '\0';
    …
}

bug出在"findWhat[FINDBUFFLEN] = ‘\0′;”这一行。数组的最大长度为FINDBUFFLEN，但下标的最大值应该是FINDBUFFLEN-1，而不是FINDBUFFLEN。因此这 行代码显然应该改为findWhat[FINDBUFFLEN-1] = '\0';

[#7] Wolfenstein 3D项目 – 数组越界

typedef struct bot_state_s
{
    …
    char teamleader[32]; //netname of the team leader
    …
}  bot_state_t;

void BotTeamAI( bot_state_t *bs ) {
    …
    bs->teamleader[sizeof( bs->teamleader )] = '\0';
    …
}

"sizeof( bs->teamleader )]"这行的结果值已经超出了数组的最大边界，正确的代码是：

bs->teamleader[
  sizeof(bs->teamleader) / sizeof(bs->teamleader[0]) – 1
  ] = '\0';

[#8] Miranda IM项目 – 只Copy了部分字符串

struct _textrangew
{
    CHARRANGE chrg;
    LPWSTR lpstrText;
} TEXTRANGEW;

const wchar_t* Utils::extractURLFromRichEdit(…)
{
    …
    ::CopyMemory(tr.lpstrText, L"mailto:", 7);
    …
}

这里的bug在于L"mailto:"是宽字符串，宽字符串中的每个字符占2或4个字节（依Compiler使用的字符集编码而定），因此这里只 copy 7个字节显然是不够的，应该是7 * sizeof(wchar_t)。

[#9] CMake项目 – 循环內的数组越界

static const struct {
    DWORD   winerr;
    int     doserr;
} doserrors[] =
{
    …
};

static void
la_dosmaperr(unsigned long e)
{
    …
    for (i = 0; i < sizeof(doserrors); i++)
    {
        if (doserrors[i].winerr == e)
        {
            errno = doserrors[i].doserr;
            return;
        }
    }
    …
}

作者原本意图la_dosmaperr中for循环的次数等于数组的元素个数，但sizeof(doserrors)返回的却是数组占用的字节个数，这远远大于数组元素个数，因此造成数组越界。正确的写法：

for (i = 0; i < sizeof(doserrors) / sizeof(*doserrors); i++)

[#10] CPU Identifying Tool项目 – 打印到自身的字符串

char * OSDetection ()
{
    …
    sprintf(szOperatingSystem,
                    "%sversion %d.%d %s (Build %d)",
                    szOperatingSystem,
                    osvi.dwMajorVersion,
                    osvi.dwMinorVersion,
                    osvi.szCSDVersion,
                    osvi.dwBuildNumber & 0xFFFF);
    …
    sprintf (szOperatingSystem, "%s%s(Build %d)",
                      szOperatingSystem, osvi.szCSDVersion,
                      osvi.dwBuildNumber & 0xFFFF);
    …
}

通过sprintf，szOperatingSystem字符串将自己打印到自己里面，这是十分危险的，将导致无法预知的错误结果，可能会导致栈溢出等严重问题。

[#12] Notepad++项目 – 数组局部clear