行进中开火

C++夜未眠

MFC中一个危险的Bug

 

上次说日本海啸警报的时候,程序出错。在解析代码的时候,发现了MFC中的一个Bug。

一。问题的产生。

这个程序,用来处理日本各种天气预报数据,包括灾害的预报。如果地震,台风之类的自然灾害到来,程序会把预报数据进行处理,生成相应的警报信息,并在电视上面显示滚动的字幕来提示。程序本身,是几年前公司的其他人写的。里面有涉及到文件读写的地方,有很多地方,用了MFC中自带的文件读写类CStdioFile。

CStdioFile这个文件读写类,估计大家都不陌生。这个类的父类,是CFile类。CStdioFile类本身的功能也很简单。CStdioFile类有一个成员函数是ReadString,函数的定义如下:

    virtual LPTSTR ReadString(__out_ecount_z(nMax) LPTSTR lpsz, __in UINT nMax);
    virtual BOOL ReadString(CString& rString);
MSDN定义如下http://msdn.microsoft.com/library/x5t0zfyf(VS.80).aspx
BOOL ReadString(CString& rString);
throw( CFileException );
Return Value
A pointer to the buffer containing the text data. NULL if end-of-file was reached without reading any data; or if boolean, FALSE if end-of-file was reached without reading any data.

ReadString函数能直接读取文本中的一行数据到CString中,很方便。读到文件结尾,没有读出任何数据的时候,返回FALSE。很简单的函数,但恰恰是这个函数有Bug。

程序在处理数据的时候,会生成一些临时文件,然后会读取这些临时文件中的数据,读取操作,正是用的CStdioFile的ReadString函数。读取流程很简单:

while(dFile.ReadString(Str_temp))
{
    doSomething();
}

当时的现象为,读取到最后一行,总是直接返回FALSE,怎么也读不出最后一行来。看了看文件的最后一行,包含2176个字符的数据,没有换行符。没有任何异常啊。当时没想到是MFC的Bug,因为以前有这样那样的毛病,多数是预报数据本身有问题,所以这次也是先分析数据了。分析来分析去,没发现这次的数据有什么异常。后来发现如果最后一行的文件不是2176个字符,就能正常读出来。奇了怪了,2176也不是什么特殊长度啊。实验了几次后,觉的是在不对劲。莫非是MFC的Bug?

二。发现问题所在

决定看看MFC的代码再说。做了个简单的测试程序,跟到MFC代码里一看,果然是MFC的问题!测试代码如下:

    CStdioFile  dFile;
    dFile.Open("text.txt",CFile::modeRead);
    CString str;
    while (dFile.ReadString(str) != FALSE )
    {
        printf("%s", str);
    }
    dFile.Close();

测试代码很简单,读text.txt文件中的每一行,然后打印出来。还是2176个字符就不行。确定了不是数据的问题,就是MFC代码本身的Bug。

MFC的ReadString代码如下:(中文是我加的注释)

BOOL CStdioFile::ReadString(CString& rString)
{
    ASSERT_VALID(this);
    rString = &afxChNil;    // empty string without deallocating
    const int nMaxSize = 128;  //临时字符串的长度
    LPTSTR lpsz = rString.GetBuffer(nMaxSize);  //保存每次读取到的字符串到CString中
    LPTSTR lpszResult;  //指向每次读到的字符串
    int nLen = 0;
    for (;;)
    {
        lpszResult = _fgetts(lpsz, nMaxSize+1, m_pStream); //读取操作
        rString.ReleaseBuffer();
        // handle error/eof case
        if (lpszResult == NULL && !feof(m_pStream))
        {
            clearerr(m_pStream);
            AfxThrowFileException(CFileException::generic, _doserrno,
                m_strFileName);
        }
        // if string is read completely or EOF
        if (lpszResult == NULL ||
            (nLen = lstrlen(lpsz)) < nMaxSize ||
            lpsz[nLen-1] == '\n')
            break;
        nLen = rString.GetLength();
        lpsz = rString.GetBuffer(nMaxSize + nLen) + nLen; //位置后移
    }
    // remove '\n' from end of string if present
    lpsz = rString.GetBuffer(0);
    nLen = rString.GetLength();
    if (nLen != 0 && lpsz[nLen-1] == '\n') // 最后结果中,去掉回车符
        rString.GetBufferSetLength(nLen-1); 
    return lpszResult != NULL;  // 这里就是Bug的关键。返回值不对!
}

可以看到,ReadString的底层,是用fgets来读取文件的。在内部,每次读取128个字符到CString中,然后位置后移,反复读取128个字符,直到遇到回车符或者文件结束。最后把回车符去掉,返回一个CString。其中,lpszResult也指向每次读出的字符串。

这里就看出问题所在了,2176个字符,正好是128的17倍!也就是说,只要文件最后一行是128倍数个字符,就一定会返回FALSE。

为什么会这样呢,因为ReadString在每次读取128个字符的时候,用lpszResult指向读取到的字符串。如果读满了128个字符,就继续读,如果读到的字符不够128个,那么就结束读取。

当一行数据正好为128的倍数,又没有回车符的时候,会发生什么呢?比如最后一行数据是128个,那么,读一次128个字符,会继续读下一次,但是下一次的读取,什么也没有读到,lpszResult就指向NULL,最后的返回值,是return lpszResult != NULL; 所以返回FALSE。

但之前读到的128个字符,已经在CString里面了。也就是说实际上读取已经成功了,但还是返回了FALSE。返回值不恰当!

Bug的描述:当文件的最后一行数据,正好是128的倍数个字符的时候,用ReadString读取,一定会返回FALSE。但实际上读取是成功的,返回的CString中的数据是正确的!(VC6.0中存在这个Bug,VS2005中,没有这个Bug)

这个Bug,只会影响到最后一行数据。因为如果有换行符的存在,lpszResult就不会为NULL。

三。解决方法

要解决这个问题,也简单,修改一下判断ReadString成功与否的语句:

while (dFile.ReadString(str) != FALSE || str.GetLength() != 0)

在返回FALSE的情况下,CString的长度不为0,就不算读取失败。或者这样:

if(!dFile.ReadString(str) && str.GetLength() == 0)

在返回FALSE并且CString的长度为0,则算读取失败,否则就是读取成功。

这个程序,是用VC6.0做的,我有看了看VC2005中的代码,发现这个Bug被修复了,代码如下:

BOOL CStdioFile::ReadString(CString& rString)
{
    ASSERT_VALID(this);
    rString = _T("");    // empty string without deallocating
    const int nMaxSize = 128;
    LPTSTR lpsz = rString.GetBuffer(nMaxSize);
    LPTSTR lpszResult;
    int nLen = 0;
    for (;;)
    {
        lpszResult = _fgetts(lpsz, nMaxSize+1, m_pStream);
        rString.ReleaseBuffer();
        // handle error/eof case
        if (lpszResult == NULL && !feof(m_pStream))
        {
            Afx_clearerr_s(m_pStream);
            AfxThrowFileException(CFileException::genericException, _doserrno,
                m_strFileName);
        }
        // if string is read completely or EOF
        if (lpszResult == NULL ||
            (nLen = (int)lstrlen(lpsz)) < nMaxSize ||
            lpsz[nLen-1] == '\n')
            break;
        nLen = rString.GetLength();
        lpsz = rString.GetBuffer(nMaxSize + nLen) + nLen;
    }
    // remove '\n' from end of string if present
    lpsz = rString.GetBuffer(0);
    nLen = rString.GetLength();
    if (nLen != 0 && lpsz[nLen-1] == '\n')
        rString.GetBufferSetLength(nLen-1);
    return nLen != 0; //返回值变了!
}

我们看到,VC2005中,读取部分的代码与VC6.0中的代码完全一样。不一样的地方只是返回值的部分。VC2005的ReadString中,返回值为

return nLen != 0;

也就是说,只要读出的CString的长度不为0就为读取成功。与我修改后的方法完全一致。就这样向客户解释,然后修改了。悲剧的是,几年前所有程序中所有使用ReadString函数的地方,都要进行修改。。。

MFC的这个Bug比较隐蔽,平常不容易发现,但一旦遇到特殊长度的数据,就会表现异常。所以,在用VC6.0开发的时候,尽量避免使用ReadString,或者在使用中,多判断一步读取出来的CString长度。避开这个Bug。

posted on 2010-03-26 22:15 Jakcie 阅读(1853) 评论(4)  编辑 收藏 引用 所属分类: C++ & CWindows & MFC

评论

# re: MFC中一个危险的Bug 2010-03-27 22:16 陈梓瀚(vczh)

解决方法应该是避免使用VC6进行开发。  回复  更多评论   

# re: MFC中一个危险的Bug 2010-03-27 23:47 Jakcie

的确应该避免用VC6开发。但使用VC6的人,还是不少啊。  回复  更多评论   

# re: MFC中一个危险的Bug 2010-03-28 11:17 唐风

1. 遗留代码不可替代
2. 升级开发环境的预算和必要性
3. 有决策权开发人员的“技术惯性”
使得VC6到现在还这么有“生命”力。感觉就像IE6一样,哈哈,有种种的不好,可偏还是有那么多用户。

反正我是等VS2010出来就准备XX的,嘿嘿。

  回复  更多评论   

# re: MFC中一个危险的Bug 2010-03-28 16:08 Jakcie

是啊。尤其是像这种遗留代码,的确是没有办法。

好在现在公司的环境,基本都是VS2005. VS2010,估计短时间内用不上。  回复  更多评论   


只有注册用户登录后才能发表评论。
【推荐】超50万行VC++源码: 大型组态工控、电力仿真CAD与GIS源码库
网站导航: 博客园   IT新闻   BlogJava   知识库   博问   管理