QReadLocker / QWriteLocker instead manual lock() unlock() for safety (e.g. exception before unlock())

master ahead manual fix

.travis.yml

@@ -2,7 +2,6 @@ language: cpp
 
 compiler:
  - gcc
- - clang
 
 env:
  - QT4_BUILD=ON

src/msgpack.h

@@ -1,10 +1,12 @@
 #ifndef MSGPACK_H
 #define MSGPACK_H
-#include <QByteArray>
-#include <QVariantList>
+
 #include "msgpack_common.h"
 #include "msgpack_export.h"
 
+#include <QByteArray>
+#include <QVariantList>
+
 namespace MsgPack
 {
     MSGPACK_EXPORT QVariant unpack(const QByteArray &data);

src/private/pack_p.cpp

@@ -9,6 +9,9 @@
 
 #include <limits>
 
+#include <QReadLocker>
+#include <QWriteLocker>
+
 QHash<QMetaType::Type, MsgPackPrivate::packer_t> MsgPackPrivate::user_packers;
 bool MsgPackPrivate::compatibilityMode = false;
 QReadWriteLock MsgPackPrivate::packers_lock;
@@ -41,9 +44,9 @@ quint8 *MsgPackPrivate::pack(const QVariant &v, quint8 *p, bool wr)
     else {
         if (t == QMetaType::User)
             t = (QMetaType::Type)v.userType();
-        packers_lock.lockForRead();
+        QReadLocker locker(&packers_lock);
         bool has_packer = user_packers.contains(t);
-        packers_lock.unlock();
+        locker.unlock();
         if (has_packer)
             p = pack_user(v, p, wr);
         else
@@ -281,17 +284,15 @@ bool MsgPackPrivate::register_packer(QMetaType::Type q_type, qint8 msgpack_type,
         qWarning() << "MsgPack::packer for qtype" << q_type << "is invalid";
         return false;
     }
-    packers_lock.lockForWrite();
+    QWriteLocker locker(&packers_lock);
     if (user_packers.contains(q_type)) {
         qWarning() << "MsgPack::packer for qtype" << q_type << "already exist";
-        packers_lock.unlock();
         return false;
     }
     packer_t p;
     p.packer = packer;
     p.type = msgpack_type;
     user_packers.insert(q_type, p);
-    packers_lock.unlock();
     return true;
 }
 
@@ -300,9 +301,9 @@ quint8 *MsgPackPrivate::pack_user(const QVariant &v, quint8 *p, bool wr)
     QMetaType::Type t = (QMetaType::Type)v.type() == QMetaType::User ?
                 (QMetaType::Type)v.userType() : (QMetaType::Type)v.type();
     QByteArray data;
-    packers_lock.lockForRead();
+    QReadLocker locker(&packers_lock);
     packer_t pt = user_packers[t];
-    packers_lock.unlock();
+    locker.unlock();
     quint32 len = pt.packer(v, data, wr);
     if (len == 1) {
         if (wr) *p = 0xd4;

src/private/unpack_p.cpp

@@ -5,6 +5,9 @@
 #include <QDebug>
 #include <QMap>
 
+#include <QReadLocker>
+#include <QWriteLocker>
+
 MsgPackPrivate::type_parser_f MsgPackPrivate::unpackers[32] = {
     unpack_nil,
     unpack_never_used,
@@ -315,14 +318,13 @@ quint8 * MsgPackPrivate::unpack_map32(QVariant &v, quint8 *p)
 
 quint8 *MsgPackPrivate::unpack_ext(QVariant &v, quint8 *p, qint8 type, quint32 len)
 {
-    unpackers_lock.lockForRead();
+    QReadLocker locker(&unpackers_lock);
     if (!user_unpackers.contains(type)) {
         qWarning() << "MsgPack::unpack() unpacker for type" << type << "doesn't exist";
         return p + len;
     }
     QByteArray data((char *)p, len);
-    v = user_unpackers[type](data);
-    unpackers_lock.unlock();
+    v = user_unpackers[type](data);s
     return p + len;
 }
 
@@ -389,13 +391,11 @@ bool MsgPackPrivate::register_unpacker(qint8 msgpack_type, MsgPack::unpack_user_
         qWarning() << "MsgPack::unpacker for type" << msgpack_type << "is invalid";
         return false;
     }
-    unpackers_lock.lockForWrite();
+    QWriteLocker locker(&unpackers_lock);
     if (user_unpackers.contains(msgpack_type)) {
         qWarning() << "MsgPack::unpacker for type" << msgpack_type << "already exists";
-        unpackers_lock.unlock();
         return false;
     }
     user_unpackers.insert(msgpack_type, unpacker);
-    unpackers_lock.unlock();
     return true;
 }