最近在C++ Builder里使用jsoncpp 0.10.2(0.y.z分支)的过程中碰到一个bug,创建的数组,无法超过5个元素,测试代码如下:
int j = 0;
int count = 20;
Json::Value root;
Json::Value item;
for (int i = 0; i < count; i++)
{
root[i] = i;
j = root.size();
}
在我的实际项目中,如果数组只有1个是元素(该元素稍微有点大的JSON对象),也有可能出现这个元素的值错误的故障,超过5个肯定出错。
在github上创建了一个issue,作者响应很快,但由于开发环境不同,他测试各个分支的代码结果都正常,不能重现故障。我在几天里,陆续调试过几次后,终于逐渐发现了故障原因:
在jsoncpp的源码里进行Debug,发现实际创建的数组成员数量并没有少,但 value_.map_ 的键CZString的成员index_是 0,1,2,3,4,0,1,2,3,4, 0,1,2,3,4,0,1,2,3,4,按正确代码应该是从0一直递增到19。而数组的size()函数是按最后一对键值的index_加1来计算大小的:
case arrayValue: // size of the array is highest index + 1
if (!value_.map_->empty())
{
ObjectValues::const_iterator itLast = value_.map_->end();
--itLast;
return (*itLast).first.index() + 1;
}
return 0;
这里我不能理解作者不直接调用map的size()函数,虽然按最后一个元素的index_的值加1来计算数组大小在逻辑上无错误。不过后面经过思考,数组可能有为null的元素不在map里面,但map最后一个元素肯定不为null,作者的思路并没有错。发现这个环节的故障后,再经过几次调试和思考,发现了以下这个拷贝构造函数有问题:
Value::CZString::CZString(const CZString& other)
: cstr_(other.storage_.policy_ != noDuplication && other.cstr_ != 0 ? duplicateStringValue(other.cstr_, other.storage_.length_) : other.cstr_)
{
storage_.policy_ = (other.cstr_ ? (other.storage_.policy_ == noDuplication ? noDuplication : duplicate) : other.storage_.policy_);
storage_.length_ = other.storage_.length_;
}
刚开始看这个拷贝构造函数,以为没有处理index_这个成员变量。将此构造函数修改为以下版本,解决了故障:
Value::CZString::CZString(const CZString& other)
: cstr_(((other.storage_.policy_ != noDuplication) && (other.cstr_ != 0)) ? duplicateStringValue(other.cstr_, other.storage_.length_)
: other.cstr_)
{
if (other.cstr_ != 0)
{
storage_.policy_ = (other.storage_.policy_ == noDuplication) ? noDuplication : duplicate;
storage_.length_ = other.storage_.length_;
}
else
{
this->index_ = other.index_;
}
}
再次更新issue,作者确认是bug,同时对该bug未影响其他用户感到奇怪。我再次检查了这个函数,考虑到index_ 和 storage_ 在同一个union里,感觉按以前的拷贝构造函数的写法index_也应该被复制。继续研究,接着发现了一个的问题:
在子类CZString的定义中:
enum DuplicationPolicy
{
noDuplication = 0,
duplicate,
duplicateOnCopy
};
这个enum的值是从0到2的,接着定义了:
struct StringStorage
{
DuplicationPolicy policy_: 2;
unsigned length_: 30; // 1GB max
};
union
{
ArrayIndex index_;
StringStorage storage_;
};
很显然,作者希望StringStorage 刚好只使用32bit,刚好4个字节,但我在CB里用sizeof测试StringStorage其实是8个字节,因为2bit整数有1个是符号位,只能支持0、1、-1,DuplicationPolicy 的值从0到3,作为整数至少需要3bit,所以CB偷偷将其扩大了,超过32bi对齐后t就是8字节。将这个结构的定义改为:
struct StringStorage
{
unsigned policy_: 2;
unsigned length_: 30; // 1GB max
};
std::swap(cstr_, other.cstr_);
std::swap(index_, other.index_);
}