LONDATEEX __stdcall GetDataDate( USHORT sMarket);
BOOL __stdcall GetDataDate( USHORT sMarket, LONDATEEX* pDate);
LONDATEEX __stdcall GetDataDate( USHORT sMarket);
BOOL __stdcall GetDataDate( USHORT sMarket, LONDATEEX* pDate);
class A
{
……
virtual FunA() = 0;
virtual FunB() = 0;
……
};
class A
{
……
virtual FunB(int J) = 0;
virtual FunA() = 0;
virtual FunB() = 0;
……
};
class A
{
……
virtual FunB(int J) = 0;
virtual FunB() = 0;
virtual FunA() = 0;
……
};
十一长假之后第一天上班,继续调试那位还在婚假当中的同事留下的代码。上午查到的一个Bug让我相当无语。不过鉴于自己如果思想在开小差的话也很可能会犯类似错误,故记录下来警醒自己。
看看这段代码:
#define ST_ERR -1
……
BYTE result = ST_ERR;
……
if (result == ST_ERR)
return errmsg;
事实上,这个if语句根本就没得到执行。即使result在赋值了ST_ERR之后从未改变,代码在执行时仍然跳过了判断,继续执行下面的代码。也就是说,result明明赋值了ST_ERR,但却不等于ST_ERR。
难道遇到鬼了,result中间被篡改了?溢出过?
好吧,其实result的确不是-1,而是255。看看BYTE的定义就清楚了。BYTE并不是C/C++内建的数据类型。在Win32平台上,它通常都被定义为unsigned char。
到这里就明白了,unsigned当然不可能有负数。而ST_ERR因为用了宏定义,当做常量使用的时候并不会自动转成非负的unsigned形式,这样当然不可能相等了。
说穿了,不是负数惹的祸,而是【宏】这个东西惹的祸。如果用const常量,因为常量的定义上有类型信息存在,就不会有这个问题了。
当然,这里无意评价宏与const常量的好与坏,单纯指出问题而已。目前这个公司的代码,宏用得不少,难保会有菜鸟程序员出这种错,隐患啊!
刚花了大半天的时间解决掉一个Bug,再次证明了变量初始化的重要性。照例,还是先来看一小段代码:
map<string, TEOBJ> telist; // 全局对象 …… TEOBJ spte(3); telist["debug"] = spte;
很简单吧?按道理这点代码应该没有什么问题。不过在最后一句中却抛出了异常,发生了崩溃。负责这部分代码的人昨天就请了假回家了,节后还有婚假,一走要近二十天,只好自己动手找原因。
这里的TEOBJ,是一个自己定义的类。在我的这个案例中,它是一个底层库里面定义的数据对象类,用在了很多地方。基于这个前提,我首先判断这个Bug的故障点在赋值号的左边。
可是左边也是个std::map的相当标准的用法,STL出错的可能性恐怕比我们自己的底层库更低,而且这个用法我以前也用过很多,没有出过什么问题。怀疑的眼光于是又回到了赋值号的右边。
那么到底是哪一边呢?我也拿不准了,于是排除法上马。通过几轮替换,我终于把问题基本定性为「由TEOBJ引起,但需要在赋值到map的[]运算符时产生」。
既然说到赋值,那么就该看看TEOBJ的相关声明了:
class TEOBJ {
public:
int GroupID;
private:
int GroupCode;
public:
TEOBJ::TEOBJ() {};
TEOBJ::TEOBJ(int nGroupID) {
GroupID = nGroupID;
GroupCode = TDataStore::Groups[GroupID]->Code;
}
TEOBJ::TEOBJ(const TEOBJ& r) {
GroupID = r.GroupID;
GroupCode = TDataStore::Groups[GroupID]->Code;
}
const TEOBJ& TEOBJ::operator=(const TEOBJ& r) {
GroupID = r.GroupID;
GroupCode = TDataStore::Groups[GroupID]->Code;
}
};
这个TEOBJ重载了拷贝构造函数和赋值运算符,在其中通过查询一个数据字典确保GroupCode有准确的取值。
首先,赋值运算符重载这里的返回值有点小问题。const引用类型的返回值与习惯上的non-const引用类型的返回值有所区别。不过这个最多造成编译时语法检查上的问题,不至于引起指针错误。
其次,就是这个TDataStore::Groups[GroupID]的数组的使用值得怀疑了。
实际案例比这个情况要复杂一些,而且我对于BCB的使用还不算熟悉,调试基本靠OutputDebugString。问题就诡异在,往TEOBJ::operator=()里面只要任意加一行OutputDebugString,故障就消失了。搞得好像是野指针一样。
我在此浪费了很多时间,走了不少弯路。后来,一咬牙用上了windbg,配合map2dbg,好歹是得到了CallStack。但是map2dbg之后得到的符号名称又和我以前看到的有些不同,于是又错判了位置。
最后,我终于在windbg+排除法的协助下找到了问题。问题根本不在赋值运算符重载函数中。尽管这里代码中明显是用到了它,但它并没有出问题。抛出异常的是拷贝构造函数。这个就要从map::operator[]的实现说起了。
Borland C++ Builder 5里面的std::map,其实现代码在map.h里面,是一个inline函数。挖出来一看,倒也很简单:
mapped_type& operator[] (const key_type& k) {
value_type tmp(k,T());
return (*((insert(tmp)).first)).second;
}
可以看到,这里先生成了一个临时变量(其实本例中就是pair<string, TEOBJ>)。在生成这个临时变量的时候,采用了无参数的TEOBJ构造函数。此时TEOBJ::GroupID还是没有初始化的,在Release版本中它可能是任意值(TEOBJ所在的底层库就是Release编译)。
实际使用时,TEOBJ都通过TEOBJ::TEOBJ(int nGroupID)来初始化,因此不会产生这种未被正确初始化的实例。但在map::operator[]中,由于是「先插入空值,再传出引用用于赋值」,这个时候就使得拷贝构造函数被调用,于是产生了类似的访问野指针的效果,导致故障的出现。
通过在拷贝构造函数中用OutputDebugString输出GroupID的取值,确认了这一点。
至此,真相大白。总结经验教训如下:
先来看一段代码:
typedef map<string, string> Dict;
Dict eventmap;
for (Dict::iterator pIter = eventmap.begin(); pIter != end(); ++pIter) {
if (pIter->second == "no")
eventmap.erase(pIter);
}
熟悉STL的人都知道,这段代码是错的。出问题的可能性不是100%,但是相当大,而且可能会是那种不一定能稳定重现的问题,往往会搞得人很恍惚。
自己很早以前就吃过这个亏,那时候还不太会用STL。当定位到这一段代码后,凭直觉也觉得如果这样删除对象,那删除之后的pIter是不是正确的?可能很难讲。毕竟学过「数据结构」,大概能猜到各种容器里面的信息是如何组织的。
昨天又看见同事在这样写代码。可能他对STL也不熟。至少我目前对于erase这个单词已经产生了足够的警戒,一旦要用的时候就会想起吃过的亏来。
正确的代码应该是:
typedef map<string, string> Dict;
Dict eventmap;
for (Dict::iterator pIter = eventmap.begin(); pIter != end();) {
if (pIter->second == "no")
pIter = eventmap.erase(pIter);
else
++pIter;
}
另外,昨天发现Borland C++ Builder提供的STL和VC有点区别,map::erase没有返回值,因此不支持这种写法。那么还有另一种办法:
eventmap.erase(pIter++);
这样就可以了。相比之下,这种写法可能适应性更强一些。