0

I have a problem with stream operator>>. I'm trying to save and load on file a QList of custom objects. The save routine seems working fine but reading the file causes a crash. I prepared a very minimal example. First of all the custom class:

class CustomObject : public QObject
{
    Q_OBJECT
    Q_PROPERTY(QString name READ name WRITE setName)
public:
    explicit CustomObject(QObject *parent = 0);
    CustomObject(const CustomObject & copy, QObject *parent = 0);

    QString name() const;
    void setName( const QString & name);

private:
    QString m_name;
};

Q_DECLARE_METATYPE( CustomObject )

QDataStream& operator<<( QDataStream& dataStream, const CustomObject * item );
QDataStream& operator>>( QDataStream& dataStream, CustomObject * item );

I have implemented stream operators this way:

QDataStream &operator<<(QDataStream &dataStream, const CustomObject *item)
{
    for(int i = 0; i < item->metaObject()->propertyCount(); ++i) {
        if(item->metaObject()->property(i).isStored(item)) {
            dataStream << item->metaObject()->property(i).read(item);
        }
    }
    return dataStream;
}


QDataStream &operator>>(QDataStream &dataStream, CustomObject *item)
{
    QVariant var;
    for(int i = 0; i < item->metaObject()->propertyCount(); ++i) {
        if(item->metaObject()->property(i).isStored(item)) {
            dataStream >> var;
            item->metaObject()->property(i).write(item, var);
        }
    }
    return dataStream;
}

This is the save() function (m_objectsList is QList<CustomObject*>)

QFile saveFile("/tmp/SaveFile");
if(!saveFile.open(QIODevice::WriteOnly)) {
    qWarning() << saveFile.errorString();
    return;
}

QDataStream outStream(&saveFile);
outStream.setVersion(QDataStream::Qt_4_8);
outStream << m_objectsList;
saveFile.close();

and this is the read() routine:

QFile saveFile("/tmp/SaveFile");
if(!saveFile.open(QIODevice::ReadOnly)) {
    qWarning() << saveFile.errorString();
    return;
}

QDataStream inStream(&saveFile);
inStream >> m_objectsList;
saveFile.close();

The application segfaults at the condition statement of for loop in operator>>:

i < item->metaObject()->propertyCount()

item is not accessible.

Can you explain me where is the error?

Very thanks.

Maxim Makhun
  • 2,197
  • 1
  • 22
  • 26
bullet
  • 83
  • 7

2 Answers2

2

Your code is crashing because in your input operator the item pointer does not actually point to an object, and is probably null. To fix this, the operator should take a reference to the pointer, and create a new instance of CustomObject(). Something like this:

QDataStream &operator>>(QDataStream &dataStream, CustomObject *& item)
{
    QVariant var;
    item = new CustomObject();
    for(int i = 0; i < item->metaObject()->propertyCount(); ++i) {
        if(item->metaObject()->property(i).isStored(item)) {
            dataStream >> var;
            item->metaObject()->property(i).write(item, var);
        }
    }
    return dataStream;
}
Dan Milburn
  • 5,600
  • 1
  • 25
  • 18
  • Using your line now the application can successfull pass operator>>, but I got a segfault trying to get properties from objects stored in QList: foreach (CustomObject *obj, m_objectsList) { qDebug() << obj->name(); } The debugger point to line 725 of qstring.h: inline QString::QString(const QString &other) : d(other.d) { Q_ASSERT(&other != this); d->ref.ref(); } (`name()` should return a `QString`) – bullet Jun 13 '14 at 13:17
1

Your code segfaults because item is a dangling pointer. Nobody has initialized it. It's on you to instantiate an item before you read it.

You should probably also attempt to store the dynamic properties, and ensure that there's at least some potential for backwards compatibility.

The code below provides two template functions that serialize an object list (as defined by its properties). First, let's re-iterate the object type:

// https://github.com/KubaO/stackoverflown/tree/master/questions/prop-storage-24185694
#include <QtCore>

class CustomObject : public QObject {
   Q_OBJECT
   Q_PROPERTY(QString name READ name WRITE setName STORED true)
   QString m_name;
public:
#ifdef Q_MOC_RUN
   Q_INVOKABLE CustomObject(QObject *parent = {})
#endif
   using QObject::QObject;

   QString name() const { return m_name; }
   void setName(const QString &name) { m_name = name; }
};

Some helpers:

/// Returns a zero-copy byte array wrapping a C string constant
static QByteArray baFromCStr(const char *str) {
   return QByteArray::fromRawData(str, qstrlen(str));
}

/// Returns a list of stored properties for a given type
QList<QMetaProperty> storedProperties(const QMetaObject *mo) {
   QList<QMetaProperty> stored;
   for (int i = 0; i < mo->propertyCount(); ++i) {
      auto prop = mo->property(i);
      if (prop.isStored())
         stored << prop;
   }
   return stored;
}

/// Caches strings for saving to a data stream
struct SaveCache {
   QMap<QByteArray, qint32> strings;
   QDataStream &save(QDataStream &str, const QByteArray &string) {
      auto it = strings.find(string);
      if (it != strings.end())
         return str << (qint32)it.value();
      auto key = strings.count();
      strings.insert(string, key);
      return str << (qint32)key << string;
   }
   QDataStream &save(QDataStream &str, const char *string) {
      return save(str, baFromCStr(string));
   }
};

/// Caches strings while loading from a data stream
struct LoadCache {
   QList<QByteArray> strings;
   QDataStream &load(QDataStream &str, QByteArray &string) {
      qint32 index;
      str >> index;
      if (index >= strings.count()) {
         str >> string;
         while (strings.size() < index)
            strings << QByteArray{};
         strings << string;
      } else
         string = strings.at(index);
      return str;
   }
};

The format stored in the stream is may be described as follows:

template <typename T>
QDataStream &writeObjectList(QDataStream &str, const QList<T*> &items) {
   str << (quint32)items.count();
   if (! items.count()) return str;
   str << (quint8)1; // version

   SaveCache strings;
   for (QObject *item : items) {
      auto *mo = item->metaObject();
      // Type
      strings.save(str, mo->className());
      // Properties
      auto const stored = storedProperties(mo);
      auto const dynamic = item->dynamicPropertyNames();
      str << (quint32)(stored.count() + dynamic.count());
      for (auto &prop : qAsConst(stored))
         strings.save(str, prop.name()) << prop.read(item);
      for (auto &name : dynamic)
         strings.save(str, name) << item->property(name);
   }
   return str;
}

The reading method has to try two ways of instantiating the object:

template <typename T> QDataStream &readObjectList(QDataStream &str,
                                                  QList<T*> &items)
{
   quint32 itemCount;
   str >> itemCount;
   if (!itemCount) return str;
   quint8 version;
   str >> version;
   if (version != 1) {
      str.setStatus(QDataStream::ReadCorruptData);
      return str;
   }

   LoadCache strings;
   for (; itemCount; itemCount--) {
      QByteArray string;
      // Type
      T *obj = {};
      strings.load(str, string);
      if (T::staticMetaObject.className() == string)
         obj = new T();
      else {
         string.append('*');
         auto id = QMetaType::type(string);
         const auto *mo = QMetaType::metaObjectForType(id);
         if (mo)
            obj = qobject_cast<T*>(mo->newInstance());
      }
      if (obj)
         items << obj;
      // Properties
      quint32 propertyCount;
      str >> propertyCount;
      for (uint i = 0; i < propertyCount; ++i) {
         QVariant value;
         strings.load(str, string) >> value;
         if (obj) obj->setProperty(string, value);
      }
   }
   return str;
}

And a very simple test harness:

QDataStream &operator<<(QDataStream &str, const QList<CustomObject*> &items) {
   return writeObjectList(str, items);
}

QDataStream& operator>>(QDataStream &str, QList<CustomObject*> &items) {
   return readObjectList(str, items);
}

int main() {
   qRegisterMetaType<CustomObject*>(); // necessary for any classes derived from CustomObject*

   QBuffer buf;
   buf.open(QBuffer::ReadWrite);
   QDataStream ds(&buf);

   CustomObject obj;
   obj.setObjectName("customsky");
   obj.setProperty("prop", 20);
   QList<CustomObject*> list;
   list << &obj;
   ds << list;

   QList<CustomObject*> list2;
   buf.seek(0);
   ds >> list2;
   Q_ASSERT(list2.size() == list.size());
   for (int i = 0; i < list.size(); ++i) {
      auto *obj1 = list.at(i);
      auto *obj2 = list2.at(i);
      Q_ASSERT(obj1->objectName() == obj2->objectName());
      Q_ASSERT(obj1->dynamicPropertyNames() == obj2->dynamicPropertyNames());
      for (auto &name : obj1->dynamicPropertyNames())
         Q_ASSERT(obj1->property(name) == obj2->property(name));
   }
}

#include "main.moc"
Kuba hasn't forgotten Monica
  • 95,931
  • 16
  • 151
  • 313
  • your code works fine, but if I understand it well it doesn't save a QList as is but breaking up it to simple elements, like this naive version: outStream << (quint32)m_objectsList.count(); foreach (CustomObject *obj, m_objectsList) { outStream << obj; } – bullet Jun 13 '14 at 14:47
  • The reason it breaks up the `QList` is that the operator that streams the list itself is too general to prevent a possibly massive redundancy in the output . So far, my code is storing each property name exactly only once. It's conceivable to further apply byte-oriented Huffman coding to the property name indices, further shortening the serialized representation at the cost of temporary storage of Huffman tables. – Kuba hasn't forgotten Monica Jun 13 '14 at 16:08
  • The generality of `QDataStream & operator<<(QDataStream&, const QList&)` comes at a price. You should reuse code when it makes sense. Here, reusing that operator makes little sense. Remember that there's nothing particularly special about the `QList`-specific stream operator. It's not as if it was somehow serializing the fact that a `QList` is stored, etc. The serialized representation of the list is simply a count followed by the serialized representation of list elements, in order from index zero going up. – Kuba hasn't forgotten Monica Jun 13 '14 at 16:09
  • So, what you noticed is true, but is no reason for concern. The way you use it is still that you simply stream the list as-is - but using our custom operator instead of the default one. All that we do is replace a different operator: instead of replacing the `CustomObject*`-specific operator, we replace the `QList`-specific one. – Kuba hasn't forgotten Monica Jun 13 '14 at 16:13