Skip to content
  • Categories
  • Recent
  • Tags
  • Popular
  • Users
  • Groups
  • Search
  • Get Qt Extensions
  • Unsolved
Collapse
Brand Logo
  1. Home
  2. Qt Development
  3. General and Desktop
  4. How do I get QTemporaryFile and QtConcurrent::run to play nicely?
Forum Updated to NodeBB v4.3 + New Features

How do I get QTemporaryFile and QtConcurrent::run to play nicely?

Scheduled Pinned Locked Moved Solved General and Desktop
22 Posts 6 Posters 8.1k Views 3 Watching
  • Oldest to Newest
  • Newest to Oldest
  • Most Votes
Reply
  • Reply as topic
Log in to reply
This topic has been deleted. Only users with topic management privileges can see it.
  • V Vadi2

    In theory I don't, but to keep it simple my unzip function accepts just the filepath to unzip.

    Yeah, my function is const, so it should be thread safe, right?

    bool Host::unzip(const QString &archivePath, const QString &destination, const QDir &tmpDir) const
    {
        int err = 0;
        //from: https://gist.github.com/mobius/1759816
        struct zip_stat zs;
        struct zip_file* zf;
        zip_uint64_t bytesRead = 0;
        char buf[4096]; // Was 100 but that seems unduly stingy...!
        zip* archive = zip_open(archivePath.toStdString().c_str(), 0, &err);
        if (err != 0) {
    //        zip_error_to_str(buf, sizeof(buf), err, errno);
    
            zip_error_t error;
            zip_error_init_with_code(&error, err);
            qDebug() << "unzip error:" << zip_error_strerror(&error);
            zip_error_fini(&error);
    
            return false;
        }
    
        // We now scan for directories first, and gather needed ones first, not
        // just relying on (zero length) archive entries ending in '/' as some
        // (possibly broken) archive building libraries seem to forget to
        // include them.
        QMap<QString, QString> directoriesNeededMap;
        //   Key is: relative path stored in archive
        // Value is: absolute path needed when extracting files
        for (zip_int64_t i = 0, total = zip_get_num_entries(archive, 0); i < total; ++i) {
            if (!zip_stat_index(archive, static_cast<zip_uint64_t>(i), 0, &zs)) {
                QString entryInArchive(QString::fromUtf8(zs.name));
                QString pathInArchive(entryInArchive.section(QLatin1Literal("/"), 0, -2));
                // TODO: We are supposed to validate the fields (except the
                // "valid" one itself) in zs before using them:
                // i.e. check that zs.name is valid ( zs.valid & ZIP_STAT_NAME )
                if (entryInArchive.endsWith(QLatin1Char('/'))) {
                    if (!directoriesNeededMap.contains(pathInArchive)) {
                        directoriesNeededMap.insert(pathInArchive, pathInArchive);
                    }
                } else {
                    if (!pathInArchive.isEmpty() && !directoriesNeededMap.contains(pathInArchive)) {
                        directoriesNeededMap.insert(pathInArchive, pathInArchive);
                    }
                }
            }
        }
    
        // Now create the needed directories:
        QMapIterator<QString, QString> itPath(directoriesNeededMap);
        while (itPath.hasNext()) {
            itPath.next();
            QString folderToCreate = QStringLiteral("%1/%2").arg(destination, itPath.value());
            if (!tmpDir.exists(folderToCreate)) {
                if (!tmpDir.mkpath(folderToCreate)) {
                    zip_close(archive);
                    return false; // Abort reading rest of archive
                }
                tmpDir.refresh();
            }
        }
    
        // Now extract the files
        for (zip_int64_t i = 0, total = zip_get_num_entries(archive, 0); i < total; ++i) {
            // No need to check return value as we've already done it first time
            zip_stat_index(archive, static_cast<zip_uint64_t>(i), 0, &zs);
            QString entryInArchive(QString::fromUtf8(zs.name));
            if (!entryInArchive.endsWith(QLatin1Char('/'))) {
                // TODO: check that zs.size is valid ( zs.valid & ZIP_STAT_SIZE )
                zf = zip_fopen_index(archive, static_cast<zip_uint64_t>(i), 0);
                if (!zf) {
                    zip_close(archive);
                    return false;
                }
    
                QFile fd(QStringLiteral("%1%2").arg(destination, entryInArchive));
    
                if (!fd.open(QIODevice::ReadWrite | QIODevice::Truncate)) {
                    zip_fclose(zf);
                    zip_close(archive);
                    return false;
                }
    
                bytesRead = 0;
                zip_uint64_t bytesExpected = zs.size;
                while (bytesRead < bytesExpected && fd.error() == QFileDevice::NoError) {
                    zip_int64_t len = zip_fread(zf, buf, sizeof(buf));
                    if (len < 0) {
                        fd.close();
                        zip_fclose(zf);
                        zip_close(archive);
                        return false;
                    }
    
                    if (fd.write(buf, len) == -1) {
                        fd.close();
                        zip_fclose(zf);
                        zip_close(archive);
                        return false;
                    }
                    bytesRead += static_cast<zip_uint64_t>(len);
                }
                fd.close();
                zip_fclose(zf);
            }
        }
    
        err = zip_close(archive);
        if (err) {
            zip_error_to_str(buf, sizeof(buf), err, errno);
            return false;
        }
    
        return true;
    }
    
    
    VRoninV Offline
    VRoninV Offline
    VRonin
    wrote on last edited by VRonin
    #12

    @Vadi2 said in How do I get QTemporaryFile and QtConcurrent::run to play nicely?:

    In theory I don't, but to keep it simple my unzip function accepts just the filepath to unzip.

    You can also use the built in qUncompress to unzip the data

    "La mort n'est rien, mais vivre vaincu et sans gloire, c'est mourir tous les jours"
    ~Napoleon Bonaparte

    On a crusade to banish setIndexWidget() from the holy land of Qt

    1 Reply Last reply
    0
    • V Offline
      V Offline
      Vadi2
      wrote on last edited by
      #13

      I wish I could, but I've read that it doesn't deal with zip archives.

      We also have had bad experience with QuaZip compiling on Windows, so we had to take it out - and KDE dependencies are not an option.

      I have followed the good advice here and unzip is now like this:

      static bool unzip(const QString &archivePath, const QString &destination, const QDir &tmpDir);

      I didn't realise const function was not enforced by the compiler! I take it that static is?

      Thanks for the help everyone on this :)

      kshegunovK VRoninV 2 Replies Last reply
      0
      • V Vadi2

        I wish I could, but I've read that it doesn't deal with zip archives.

        We also have had bad experience with QuaZip compiling on Windows, so we had to take it out - and KDE dependencies are not an option.

        I have followed the good advice here and unzip is now like this:

        static bool unzip(const QString &archivePath, const QString &destination, const QDir &tmpDir);

        I didn't realise const function was not enforced by the compiler! I take it that static is?

        Thanks for the help everyone on this :)

        kshegunovK Offline
        kshegunovK Offline
        kshegunov
        Moderators
        wrote on last edited by
        #14

        @Vadi2 said in How do I get QTemporaryFile and QtConcurrent::run to play nicely?:

        I didn't realise const function was not enforced by the compiler!

        It is enforced, but in the example the member's declared as mutable, which I find pretty abominable to begin with. mutable basically means that the specific member can be modified from anywhere as it is "not part of the class interface". It's equivalent to stripping down the const manually, e.g.:

        class SomeClass
        {
        public:
            void someMethod() const
            {
                SomeType & playingWithFireHere = const_cast<SomeType &>(member)
            }
        
        private:
             SomeType member;
        };
        

        Read and abide by the Qt Code of Conduct

        1 Reply Last reply
        0
        • J.HilkJ Offline
          J.HilkJ Offline
          J.Hilk
          Moderators
          wrote on last edited by
          #15

          @Vadi2
          just a small note.

          You never delete tempThemesArchive, nether after finishing the unzip nor after exiting because of an error. That'll leak memory.


          Be aware of the Qt Code of Conduct, when posting : https://forum.qt.io/topic/113070/qt-code-of-conduct


          Q: What's that?
          A: It's blue light.
          Q: What does it do?
          A: It turns blue.

          kshegunovK 1 Reply Last reply
          0
          • J.HilkJ J.Hilk

            @Vadi2
            just a small note.

            You never delete tempThemesArchive, nether after finishing the unzip nor after exiting because of an error. That'll leak memory.

            kshegunovK Offline
            kshegunovK Offline
            kshegunov
            Moderators
            wrote on last edited by
            #16

            Actually there's tempThemesArchive->deleteLater(); I see, I haven't checked the whole source though ...

            Read and abide by the Qt Code of Conduct

            J.HilkJ 1 Reply Last reply
            0
            • V Offline
              V Offline
              Vadi2
              wrote on last edited by
              #17

              Ohh oops - in case of an error, it would not have gotten deleted. I've fixed it now:

                  connect(getReply,
                          &QNetworkReply::finished,
                          this,
                          std::bind(
                                  [=](QNetworkReply* reply) {
                                      // don't do anything if there was an error
                                      if (reply->error() != QNetworkReply::NoError) {
                                          return;
                                      }
                                      QByteArray downloadedArchive = reply->readAll();
              
                                      tempThemesArchive = new QTemporaryFile();
                                      if (!tempThemesArchive->open()) {
                                          return;
                                      }
                                      tempThemesArchive->write(downloadedArchive);
                                      tempThemesArchive->close();
              
                                      QTemporaryDir temporaryDir;
                                      if (!temporaryDir.isValid()) {
                                          return;
                                      }
              
                                      // perform unzipping in a worker thread so as not to freeze the UI
                                      auto future =
                                              QtConcurrent::run(mudlet::unzip, tempThemesArchive->fileName(), QStringLiteral("%1/.config/mudlet/edbee/").arg(QDir::homePath()), temporaryDir.path());
                                      auto watcher = new QFutureWatcher<bool>;
                                      QObject::connect(watcher, &QFutureWatcher<bool>::finished, [=]() {
                                          if (future.result() == true) {
                                              loadEdbeeThemes(true);
                                          }
              
                                          theme_download_label->hide();
                                          tempThemesArchive->deleteLater();
                                      });
                                      watcher->setFuture(future);
                                      reply->deleteLater();
                                  },
                                  getReply));
              
              1 Reply Last reply
              0
              • kshegunovK kshegunov

                Actually there's tempThemesArchive->deleteLater(); I see, I haven't checked the whole source though ...

                J.HilkJ Offline
                J.HilkJ Offline
                J.Hilk
                Moderators
                wrote on last edited by
                #18

                @kshegunov you're right, but that applies only if the whole operation is is successful.


                Be aware of the Qt Code of Conduct, when posting : https://forum.qt.io/topic/113070/qt-code-of-conduct


                Q: What's that?
                A: It's blue light.
                Q: What does it do?
                A: It turns blue.

                1 Reply Last reply
                1
                • V Offline
                  V Offline
                  Vadi2
                  wrote on last edited by
                  #19

                  I'm happy with being able to keep all the code together using lambdas instead of having people jump around slot function definitions. Do others find this style readable? I'm sure there is room for improvement.

                  kshegunovK J.HilkJ 2 Replies Last reply
                  0
                  • V Vadi2

                    I'm happy with being able to keep all the code together using lambdas instead of having people jump around slot function definitions. Do others find this style readable? I'm sure there is room for improvement.

                    kshegunovK Offline
                    kshegunovK Offline
                    kshegunov
                    Moderators
                    wrote on last edited by
                    #20

                    @Vadi2 said in How do I get QTemporaryFile and QtConcurrent::run to play nicely?:

                    Do others find this style readable?

                    Not me. I don't write functions that are longer than 1 screen (about 100-120 lines) and I don't write into a lambda anything that's longer than few lines, but I'm old-fashioned.

                    Read and abide by the Qt Code of Conduct

                    1 Reply Last reply
                    1
                    • V Vadi2

                      I'm happy with being able to keep all the code together using lambdas instead of having people jump around slot function definitions. Do others find this style readable? I'm sure there is room for improvement.

                      J.HilkJ Offline
                      J.HilkJ Offline
                      J.Hilk
                      Moderators
                      wrote on last edited by
                      #21

                      @Vadi2 For me personally its a bit to much especially as your lambda function has a nested lambda in it.

                      I probably would have made this a seperated class .


                      Be aware of the Qt Code of Conduct, when posting : https://forum.qt.io/topic/113070/qt-code-of-conduct


                      Q: What's that?
                      A: It's blue light.
                      Q: What does it do?
                      A: It turns blue.

                      1 Reply Last reply
                      1
                      • V Vadi2

                        I wish I could, but I've read that it doesn't deal with zip archives.

                        We also have had bad experience with QuaZip compiling on Windows, so we had to take it out - and KDE dependencies are not an option.

                        I have followed the good advice here and unzip is now like this:

                        static bool unzip(const QString &archivePath, const QString &destination, const QDir &tmpDir);

                        I didn't realise const function was not enforced by the compiler! I take it that static is?

                        Thanks for the help everyone on this :)

                        VRoninV Offline
                        VRoninV Offline
                        VRonin
                        wrote on last edited by VRonin
                        #22

                        @Vadi2 said in How do I get QTemporaryFile and QtConcurrent::run to play nicely?:

                        and KDE dependencies are not an option

                        Just to be clear, the KDE API was redesigned in version 5 and now only uses Qt Calls so using KArchive does not link your application in any way to the KDE environment or the Linux OS. I have cross platform applications using those APIs. KArchive clearly states that works on: Android, FreeBSD, Linux, MacOSX and Windows.

                        @Vadi2 said in How do I get QTemporaryFile and QtConcurrent::run to play nicely?:

                        I wish I could, but I've read that it doesn't deal with zip archives.

                        I use KArchive so I did not test it but by looking at the documentation of the method, it makes me think it does:

                        from http://doc.qt.io/qt-5/qbytearray.html#qUncompress

                        If you want to use this function to uncompress external data that was compressed using zlib...

                        "La mort n'est rien, mais vivre vaincu et sans gloire, c'est mourir tous les jours"
                        ~Napoleon Bonaparte

                        On a crusade to banish setIndexWidget() from the holy land of Qt

                        1 Reply Last reply
                        1

                        • Login

                        • Login or register to search.
                        • First post
                          Last post
                        0
                        • Categories
                        • Recent
                        • Tags
                        • Popular
                        • Users
                        • Groups
                        • Search
                        • Get Qt Extensions
                        • Unsolved