QZip improvements (#4768)

Removed unused constructors
Replaced raw pointer with a unique_ptr to clearly pass ownership.
Fixed a bug where we compared if a signed integer is negative (it never is).
Fixed a case where we take the pointer to a pointer we de-reference!

contrib/qzip/zip.cpp

@@ -397,21 +397,22 @@ bool ZipReader::FileInfo::isValid() const
 class QZipPrivate
 {
 public:
-    QZipPrivate(QIODevice *device, bool ownDev)
-        : device(device), ownDevice(ownDev), dirtyFileTree(true), start_of_directory(0)
+    QZipPrivate(QIODevice *device)
+        : device(device), dirtyFileTree(true), start_of_directory(0)
     {
     }
 
-    ~QZipPrivate()
+    QZipPrivate(std::unique_ptr<QIODevice> dev)
+        : device(dev.get()), ownedDevice(std::move(dev)), dirtyFileTree(true), start_of_directory(0)
     {
-        if (ownDevice)
-            delete device;
     }
 
+    virtual ~QZipPrivate() = default;
+
     void fillFileInfo(int index, ZipReader::FileInfo &fileInfo) const;
 
     QIODevice *device;
-    bool ownDevice;
+    std::unique_ptr<QIODevice> ownedDevice;
     bool dirtyFileTree;
     QList<FileHeader> fileHeaders;
     QByteArray comment;
@@ -439,8 +440,13 @@ void QZipPrivate::fillFileInfo(int index, ZipReader::FileInfo &fileInfo) const
 class ZipReaderPrivate : public QZipPrivate
 {
 public:
-    ZipReaderPrivate(QIODevice *device, bool ownDev)
-        : QZipPrivate(device, ownDev), status(ZipReader::NoError)
+    ZipReaderPrivate(QIODevice *device)
+        : QZipPrivate(device), status(ZipReader::NoError)
+    {
+    }
+
+    ZipReaderPrivate(std::unique_ptr<QIODevice> device)
+        : QZipPrivate(std::move(device)), status(ZipReader::NoError)
     {
     }
 
@@ -452,8 +458,16 @@ public:
 class ZipWriterPrivate : public QZipPrivate
 {
 public:
-    ZipWriterPrivate(QIODevice *device, bool ownDev)
-        : QZipPrivate(device, ownDev),
+    ZipWriterPrivate(QIODevice *device)
+        : QZipPrivate(device),
+        status(ZipWriter::NoError),
+        permissions(QFile::ReadOwner | QFile::WriteOwner),
+        compressionPolicy(ZipWriter::AlwaysCompress)
+    {
+    }
+
+    ZipWriterPrivate(std::unique_ptr<QIODevice> device)
+        : QZipPrivate(std::move(device)),
         status(ZipWriter::NoError),
         permissions(QFile::ReadOwner | QFile::WriteOwner),
         compressionPolicy(ZipWriter::AlwaysCompress)
@@ -755,19 +769,14 @@ ZipReader::ZipReader(const QString &archive, QIODevice::OpenMode mode)
             status = FileError;
     }
 
-    d = new ZipReaderPrivate(&(*f.release()), /*ownDevice=*/true);
+    d = std::make_unique<ZipReaderPrivate>(std::move(f));
     d->status = status;
 }
 
-/*!
-    Create a new zip archive that operates on the archive found in \a device.
-    You have to open the device previous to calling the constructor and only a
-    device that is readable will be scanned for zip filecontent.
- */
-ZipReader::ZipReader(QIODevice *device)
-    : d(new ZipReaderPrivate(device, /*ownDevice=*/false))
+ZipReader::ZipReader(std::unique_ptr<QIODevice> device)
+    : d(std::make_unique<ZipReaderPrivate>(std::move(device)))
 {
-    Q_ASSERT(device);
+    Q_ASSERT(d->device);
 }
 
 /*!
@@ -776,7 +785,6 @@ ZipReader::ZipReader(QIODevice *device)
 ZipReader::~ZipReader()
 {
     close();
-    delete d;
 }
 
 /*!
@@ -1039,25 +1047,13 @@ ZipWriter::ZipWriter(const QString &fileName, QIODevice::OpenMode mode)
             status = ZipWriter::FileError;
     }
 
-    d = new ZipWriterPrivate(&(*f.release()), /*ownDevice=*/true);
+    d = std::make_unique<ZipWriterPrivate>(std::move(f));
     d->status = status;
 }
 
-/*!
-    Create a new zip archive that operates on the archive found in \a device.
-    You have to open the device previous to calling the constructor and
-    only a device that is readable will be scanned for zip filecontent.
- */
-ZipWriter::ZipWriter(QIODevice *device)
-    : d(new ZipWriterPrivate(device, /*ownDevice=*/false))
-{
-    Q_ASSERT(device);
-}
-
 ZipWriter::~ZipWriter()
 {
     close();
-    delete d;
 }
 
 /*!

contrib/qzip/zipreader.h

@@ -67,8 +67,8 @@ class ZipReader
 {
 public:
     ZipReader(const QString &fileName, QIODevice::OpenMode mode = QIODevice::ReadOnly );
+    explicit ZipReader(std::unique_ptr<QIODevice> device);
 
-    explicit ZipReader(QIODevice *device);
     ~ZipReader();
 
     QIODevice* device() const;
@@ -114,8 +114,7 @@ public:
     void close();
 
 private:
-    ZipReaderPrivate *d;
-    Q_DISABLE_COPY(ZipReader)
+    std::unique_ptr<ZipReaderPrivate> d;
 };
 
 QT_END_NAMESPACE

contrib/qzip/zipwriter.h

@@ -66,7 +66,6 @@ class ZipWriter
 public:
     ZipWriter(const QString &fileName, QIODevice::OpenMode mode = (QIODevice::WriteOnly | QIODevice::Truncate) );
 
-    explicit ZipWriter(QIODevice *device);
     ~ZipWriter();
 
     QIODevice* device() const;
@@ -105,9 +104,9 @@ public:
     void addSymLink(const QString &fileName, const QString &destination);
 
     void close();
-private:
-    ZipWriterPrivate *d;
-    Q_DISABLE_COPY(ZipWriter)
+
+    private:
+    std::unique_ptr<ZipWriterPrivate> d;
 };
 
 QT_END_NAMESPACE

src/Gui/IconManager.cpp

@@ -208,8 +208,8 @@ bool
 IconManager::importBundle
 (const QString &filename)
 {
-    QFile zipFile(filename);
-    return importBundle(&zipFile);
+    auto qfile = std::make_unique<QFile>(filename);
+    return importBundle(std::move(qfile));
 }
 
 
@@ -218,19 +218,19 @@ IconManager::importBundle
 (const QUrl &url)
 {
     QByteArray zipData = downloadUrl(url);
-    QBuffer buffer;
-    buffer.setData(zipData);
-    buffer.open(QIODevice::ReadOnly);
+    auto buffer = std::make_unique<QBuffer>();
+    buffer->setData(zipData);
+    buffer->open(QIODevice::ReadOnly);
 
-    return importBundle(&buffer);
+    return importBundle(std::move(buffer));
 }
 
 
 bool
 IconManager::importBundle
-(QIODevice *device)
+(std::unique_ptr<QIODevice> device)
 {
-    ZipReader reader(device);
+    ZipReader reader(std::move(device));
     for (ZipReader::FileInfo info : reader.fileInfoList()) {
         if (info.isFile) {
             QByteArray data = reader.fileData(info.filePath);

src/Gui/IconManager.h

@@ -19,6 +19,8 @@
 #ifndef _GC_IconManager_h
 #define _GC_IconManager_h
 
+#include <memory>
+
 #include <QString>
 #include <QHash>
 #include <QDir>
@@ -52,7 +54,7 @@ public:
     bool exportBundle(const QString &filepath);
     bool importBundle(const QString &filepath);
     bool importBundle(const QUrl &url);
-    bool importBundle(QIODevice *device);
+    bool importBundle(std::unique_ptr<QIODevice> device);
 
     bool saveConfig() const;