[Development] What am I not seeing in the QByteArray::capacity test failure?

[Thiago Macieira]

  FAIL!  : tst_QByteArray::reserve() 'qba.constData() != data' returned FALSE. 
  tst_qbytearray.cpp(1871) : failure location

http://testresults.qt-project.org/ci/QtBase_stable_Integration/build_03694/win32-msvc2010_Windows_7/log.txt.gz

Code around the location is:

  1858      qba.resize(capacity);
[...]
  1863      QCOMPARE(qba.capacity(), capacity);
[...]
  1866      copy = qba;
  1867      qba.reserve(capacity * 2);
  1868      QCOMPARE(qba.size(), capacity);
  1869      QCOMPARE(qba.capacity(), capacity * 2);
  1870      QCOMPARE(copy.capacity(), capacity);
  1871      QVERIFY(qba.constData() != data);

On line 1866, we make a copy, so qba's refcount should now be 2.

When we do reserve(capacity * 2) on line 1867, the byte array is supposed to 
detach and reallocate to a larger size:

   462  inline void QByteArray::reserve(int asize)
   463  {
   464      if (d->ref.isShared() || uint(asize) + 1u > d->alloc) {
   465          reallocData(qMax(uint(size()), uint(asize)) + 1u, d-
>detachFlags() | Data::CapacityReserved);
   466      } else {
   467          // cannot set unconditionally, since d could be the 
shared_null or
   468          // otherwise static
   469          d->capacityReserved = true;
   470      }
   471  }

We know it did not detach, since the test failed because the pointer value is 
still the same. It could be for one of two reasons:
 1) both conditions of the if on line 464 are false
 2) one of them is true, but reallocData() did not actually detach

We know that asize == capacity*2 is larger than d->alloc because of the test 
on line 1863 above: capacity() returns d->alloc - 1 and we know qba.capacity() 
== capacity == 100. That rules out condition #1: one of the branches of the if 
is true.

Note that we also expect d->ref.isShared() to also be true.

If it's case #2, then reallocData did not detach:

  1495  void QByteArray::reallocData(uint alloc, Data::AllocationOptions 
options)
  1496  {
  1497      if (d->ref.isShared() || IS_RAW_DATA(d)) {
  1498          Data *x = Data::allocate(alloc, options);
  1499          Q_CHECK_PTR(x);
  1500          x->size = qMin(int(alloc) - 1, d->size);
  1501          ::memcpy(x->data(), d->data(), x->size);
  1502          x->data()[x->size] = '\0';
  1503          if (!d->ref.deref())
  1504              Data::deallocate(d);
  1505          d = x;
  1506      } else {
  1507          if (options & Data::Grow)
  1508              alloc = qAllocMore(alloc, sizeof(Data));
  1509          Data *x = static_cast<Data *>(::realloc(d, sizeof(Data) + 
alloc));
  1510          Q_CHECK_PTR(x);
  1511          x->alloc = alloc;
  1512          x->capacityReserved = (options & Data::CapacityReserved) ? 1 : 
0;
  1513          d = x;
  1514      }
  1515  }

Now, for the test to be failing, we must have hit the realloc on line 1509. 
That leads me to conclude that d->ref.isShared() actually returned false, 
despite there being an implicitly shared copy.

Could there be a bug in operator=?

   937  QByteArray &QByteArray::operator=(const QByteArray & other)
   938  {
   939      other.d->ref.ref();
   940      if (!d->ref.deref())
   941          Data::deallocate(d);
   942      d = other.d;
   943      return *this;
   944  }

Note that due to construction of the testcase, at this point 
this->d == other.d. In any case, we do increment the refcount, so it should 
have gone up to 2 or 3.

QByteArray does have a move-assignment operator:

   202      inline QByteArray &operator=(QByteArray &&other)
   203      { qSwap(d, other.d); return *this; }

But even if MSVC 2010 did erroneously call this operator, the refcount already 
was 2.

Am I missing something here? Because I can't explain the issue. And I can't 
reproduce it on Linux either.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center