Change in kio[master]: WIP! Adding SlaveBase newEntry and insertEntryProperty for b...

Mark Gaiser (Code Review) noreply at kde.org
Sat Jan 31 17:31:19 UTC 2015


Mark Gaiser has uploaded a new change for review.

  https://gerrit.vesnicky.cesnet.cz/r/348

Change subject: WIP! Adding SlaveBase newEntry and insertEntryProperty for big speed improvement in listDir calls.
......................................................................

WIP! Adding SlaveBase newEntry and insertEntryProperty for big speed improvement in listDir calls.

WIP because the tests:
- copyjob
- testtrash

still fail.. I don't know why yet.

This commit adds adds the following functions:
- newEntry()
- insertEntryProperty(uint field, const QString &value)
- insertEntryProperty(uint field, const long long value)

The existing listEntry function (that was introduced in KIO 5.0) is now deprecated. Internally it is
now using the new functions.

These methods are introduced to completely phase out a need for UDSEntry on the slave side.
Slaves can now - with those function - directly insert stat data in a buffer.
This is much faster then going through QDataStream which does all kinds of magic to be cross platform compatible.

Since QDataStream is dropped, the byte order management (that it did to be cross platform usable) is obviously
gone as well. It shouldn't be an issue since KIO isn't used over the network. It's all living on the
same machine thus the same byte order anyway.

Some numbers for performance comparision.
Running KIO::listDir on a folder with 500.000 files.
Before this patch: ~1300ms
After this patch: ~800ms

Native speed (without KIO, just C++ and storing the stat data in a vector): ~300ms

Change-Id: Ibb61f0c3a406337c54d066cac169ca534ec1d8b2
CHANGELOG: Introduce newEntry and insertEntryProperty for better listdir performance. Deprecate listEntry.
---
M autotests/udsentrytest.cpp
M src/core/slavebase.cpp
M src/core/slavebase.h
M src/core/udsentry.cpp
M src/core/udsentry.h
M src/ioslaves/file/file.cpp
M src/ioslaves/file/file.h
M src/ioslaves/file/file_unix.cpp
8 files changed, 267 insertions(+), 100 deletions(-)


  git pull ssh://gerrit.vesnicky.cesnet.cz:29418/kio refs/changes/48/348/1

diff --git a/autotests/udsentrytest.cpp b/autotests/udsentrytest.cpp
index 04e072e..066fd86 100644
--- a/autotests/udsentrytest.cpp
+++ b/autotests/udsentrytest.cpp
@@ -183,17 +183,21 @@
     {
         QDataStream stream(&data, QIODevice::WriteOnly);
         foreach (const QVector<UDSTestField> &testCase, testCases) {
-            stream << testCase.count();
+            const int size = testCase.count();
+            stream.writeRawData(reinterpret_cast<const char*>(&size), sizeof(int));
 
             foreach (const UDSTestField &field, testCase) {
                 uint uds = field.m_uds;
-                stream << uds;
+                stream.writeRawData(reinterpret_cast<const char*>(&uds), sizeof(uint));
 
                 if (uds & KIO::UDSEntry::UDS_STRING) {
-                    stream << field.m_string;
+                    const QString &str = field.m_string;
+                    int size = str.size();
+                    stream.writeRawData(reinterpret_cast<const char*>(&size), sizeof(int));
+                    stream.writeRawData(reinterpret_cast<const char*>(str.utf16()), sizeof(ushort) * size);
                 } else {
                     Q_ASSERT(uds & KIO::UDSEntry::UDS_NUMBER);
-                    stream << field.m_long;
+                    stream.writeRawData(reinterpret_cast<const char*>(&field.m_long), sizeof(long long));
                 }
             }
         }
diff --git a/src/core/slavebase.cpp b/src/core/slavebase.cpp
index db1c2ee..2e0333c 100644
--- a/src/core/slavebase.cpp
+++ b/src/core/slavebase.cpp
@@ -37,7 +37,6 @@
 #include <QtCore/QDateTime>
 #include <QtCore/QElapsedTimer>
 #include <QtCore/QCoreApplication>
-#include <QtCore/QDataStream>
 
 #include <kconfig.h>
 #include <kconfiggroup.h>
@@ -74,6 +73,59 @@
 namespace KIO
 {
 
+// This class is a small QByteArray wrapper which provides some convenience functions.
+class EntryBuffer
+{
+public:
+    EntryBuffer()
+        : m_buffer()
+        , m_pos(0)
+    {
+        // Initialize m_buffer with 8KB. That should be enough room for small folders (lets say ~ 70 entries).
+        m_buffer.resize(8192);
+    }
+
+    // Return the raw QByteArray buffer. The user should only use this function to get a buffer of the written bytes.
+    // That would be a call like: buffer().left(pos()).
+    QByteArray& buffer()
+    {
+        return m_buffer;
+    }
+
+    int pos()
+    {
+        return m_pos;
+    }
+
+    void seek(int pos)
+    {
+        m_pos = pos;
+    }
+
+    void reset()
+    {
+        m_pos = 0;
+    }
+
+    // This function writes the provided data in the internam m_buffer. The user should not write more then 4096 bytes at once!
+    qint64 write(const char *data, qint64 len)
+    {
+        // Increment the m_buffer in chunks of 4KB.
+        // Realistically this should only happen for larger folders (>70 entries).
+        if ((m_pos + len) > m_buffer.size()) {
+            m_buffer.resize(m_buffer.size() + 4096);
+        }
+
+        memcpy(m_buffer.data() + m_pos, (uchar *)data, int(len));
+        m_pos += len;
+
+        return len;
+    }
+
+    QByteArray m_buffer;
+    int m_pos;
+};
+
 class SlaveBasePrivate
 {
 public:
@@ -83,14 +135,24 @@
         if (!qgetenv("KIOSLAVE_ENABLE_TESTMODE").isEmpty()) {
             QStandardPaths::enableTestMode(true);
         }
-        pendingListEntries.reserve(KIO_MAX_ENTRIES_PER_BATCH);
     }
     ~SlaveBasePrivate()
     {
         delete m_passwdServer;
     }
 
-    UDSEntryList pendingListEntries;
+    // This value contains the number of entries currently in pendingEntriesBuffer.
+    int pendingEntries;
+
+    // This buffer contains the entries written.
+    EntryBuffer pendingEntriesBuffer;
+
+    // This contains the number of fields that an individual entry contains.
+    int entryFields;
+
+    // This value is being used to seek back in pendingEntriesBuffer to set the number of fields for an individual entry.
+    int startEntryPosition;
+
     QElapsedTimer m_timeSinceLastBatch;
     Connection appConnection;
     QString poolSocket;
@@ -237,6 +299,10 @@
 #endif
 
     globalSlave = this;
+
+    d->pendingEntries = 0;
+    d->entryFields = 0;
+    d->startEntryPosition = 0;
 
     d->isConnectedToApp = true;
 
@@ -459,9 +525,24 @@
 
 void SlaveBase::finished()
 {
-    if (!d->pendingListEntries.isEmpty()) {
-        listEntries(d->pendingListEntries);
-        d->pendingListEntries.clear();
+    if (d->pendingEntries > 0) {
+
+        // Complete the current batch by letting the last entry know how many fields it has.
+        if (d->entryFields > 0) {
+            const int endPosition = d->pendingEntriesBuffer.pos();
+            d->pendingEntriesBuffer.seek(d->startEntryPosition);
+            d->pendingEntriesBuffer.write(reinterpret_cast<const char*>(&d->entryFields), sizeof(int));
+            d->pendingEntriesBuffer.seek(endPosition);
+        }
+
+        // Send our current batch of entries to the requestee.
+        send(MSG_LIST_ENTRIES, d->pendingEntriesBuffer.buffer().left(d->pendingEntriesBuffer.pos()));
+
+        // Reset the buffer.
+        d->pendingEntriesBuffer.reset();
+
+        // Reset the pending entries.
+        d->pendingEntries = 0;
     }
 
     if (d->m_state == d->FinishedCalled) {
@@ -676,45 +757,94 @@
 #ifndef KIOCORE_NO_DEPRECATED
 void SlaveBase::listEntry(const UDSEntry &entry, bool _ready)
 {
-    if (_ready) {
-        listEntries(d->pendingListEntries);
-        d->pendingListEntries.clear();
-    } else {
+    // Previously we would have called listEntries when _ready was true. However, the finished() function also has to be called by the slave
+    // when it's done filling. So the finished() function will handle this meaning we don't have to do anything in here if _ready is true.
+    if (!_ready) {
         listEntry(entry);
+    }
+}
+
+void SlaveBase::listEntry(const UDSEntry &entry)
+{
+    // We start a new entry
+    newEntry();
+
+    // Get the entry fields and send them to insertEntryProperty.
+    const QList<uint> fields = entry.listFields();
+    foreach (uint field, fields)
+    {
+        if (field & KIO::UDSEntry::UDS_STRING) {
+            insertEntryProperty(field, entry.numberValue(field));
+        } else if (field & KIO::UDSEntry::UDS_NUMBER) {
+            insertEntryProperty(field, entry.stringValue(field));
+        } else {
+            Q_ASSERT_X(false, "KIO::UDSEntry", "Found a field with an invalid type");
+        }
     }
 }
 #endif
 
-void SlaveBase::listEntry(const UDSEntry &entry)
+void SlaveBase::listEntries(const UDSEntryList &entryList)
 {
-    // We start measuring the time from the point we start filling the list
-    if (d->pendingListEntries.isEmpty()) {
-        d->m_timeSinceLastBatch.restart();
-    }
-
-    d->pendingListEntries.append(entry);
-
-    // If more then KIO_MAX_SEND_BATCH_TIME time is passed, emit the current batch
-    // Also emit if we have piled up a large number of entries already, to save memory (and time)
-    if (d->m_timeSinceLastBatch.elapsed() > KIO_MAX_SEND_BATCH_TIME || d->pendingListEntries.size() > KIO_MAX_ENTRIES_PER_BATCH) {
-        listEntries(d->pendingListEntries);
-        d->pendingListEntries.clear();
-
-        // Restart time
-        d->m_timeSinceLastBatch.restart();
+    foreach (const UDSEntry& entry, entryList)
+    {
+        listEntry(entry);
     }
 }
 
-void SlaveBase::listEntries(const UDSEntryList &list)
+void SlaveBase::newEntry()
 {
-    QByteArray data;
-    QDataStream stream(&data, QIODevice::WriteOnly);
-
-    foreach (const UDSEntry &entry, list) {
-        stream << entry;
+    // If we're done with an entry, seek back to the beginning of this entry and write the number of fields this entry has.
+    if (d->entryFields > 0) {
+        const int endPosition = d->pendingEntriesBuffer.pos();
+        d->pendingEntriesBuffer.seek(d->startEntryPosition);
+        d->pendingEntriesBuffer.write(reinterpret_cast<const char*>(&d->entryFields), sizeof(int));
+        d->pendingEntriesBuffer.seek(endPosition);
     }
 
-    send(MSG_LIST_ENTRIES, data);
+    // If more then KIO_MAX_SEND_BATCH_TIME time is passed, emit the current batch
+    // Also emit if we have piled up a large number of entries already, to save memory (and time)
+    if (d->m_timeSinceLastBatch.elapsed() > KIO_MAX_SEND_BATCH_TIME || d->pendingEntries > KIO_MAX_ENTRIES_PER_BATCH) {
+        // Send our current batch of entries to the requestee.
+        send(MSG_LIST_ENTRIES, d->pendingEntriesBuffer.buffer().left(d->pendingEntriesBuffer.pos()));
+
+        // Reset the buffer.
+        d->pendingEntriesBuffer.reset();
+
+        // Reset the pending entries.
+        d->pendingEntries = 0;
+    }
+
+    if (d->pendingEntries == 0) {
+        // Restart time
+        d->m_timeSinceLastBatch.restart();
+    }
+
+    // Increase the number of pending entries.
+    d->pendingEntries++;
+    d->entryFields = 0;
+
+    // Save our current position since we need to come back later on to modify this position with the number of fields for this entry.
+    d->startEntryPosition = d->pendingEntriesBuffer.pos();
+    d->pendingEntriesBuffer.write(reinterpret_cast<const char*>(&d->entryFields), sizeof(int));
+}
+
+void SlaveBase::insertEntryProperty(uint field, const QString &value)
+{
+    const ushort* valuePtr = value.utf16();
+    int size = value.size();
+
+    d->pendingEntriesBuffer.write(reinterpret_cast<const char*>(&field), sizeof(uint));
+    d->pendingEntriesBuffer.write(reinterpret_cast<const char*>(&size), sizeof(int));
+    d->pendingEntriesBuffer.write(reinterpret_cast<const char*>(valuePtr), sizeof(ushort) * value.size());
+    d->entryFields++;
+}
+
+void SlaveBase::insertEntryProperty(uint field, const long long value)
+{
+    d->pendingEntriesBuffer.write(reinterpret_cast<const char*>(&field), sizeof(uint));
+    d->pendingEntriesBuffer.write(reinterpret_cast<const char*>(&value), sizeof(long long));
+    d->entryFields++;
 }
 
 static void sigsegv_handler(int sig)
diff --git a/src/core/slavebase.h b/src/core/slavebase.h
index d1d94de..50d69dd 100644
--- a/src/core/slavebase.h
+++ b/src/core/slavebase.h
@@ -158,7 +158,44 @@
      * to report.
      * @param _entry The UDSEntry containing all of the object attributes.
      */
-    void listEntries(const UDSEntryList &_entry);
+   void listEntries(const UDSEntryList &entryList);
+
+    /**
+     * Call this function right before you start to write \ref UDSEntry::StandardFieldTypes fields.
+     * Example usage for listing a directory:
+     *   DIR *dp = opendir("/some/path");
+     *   QT_DIRENT *ep;
+     *   QT_STATBUF buff;
+     *   while ((ep = QT_READDIR(dp)) != 0) {
+     *       if (QT_LSTAT(ep->d_name, &buff) == 0)  {
+     *         newEntry();
+     *         insertEntryProperty(KIO::UDSEntry::UDS_NAME, QString(ep->d_name));
+     *         insertEntryProperty(KIO::UDSEntry::UDS_DEVICE_ID, buff.st_dev);
+     *         insertEntryProperty(KIO::UDSEntry::UDS_INODE, buff.st_ino);
+     *         etc...
+     *       }
+     *   }
+     *
+     * @since 5.8
+     */
+    void newEntry();
+
+    /**
+     * Call this when you want to insert a string based stat property.
+     * @param field The field you want to write. See \ref UDSEntry::StandardFieldTypes for the possible values.
+     * @param value Value to set.
+     * @since 5.8
+     */
+    void insertEntryProperty(uint field, const QString &value);
+
+    /**
+     * Call this when you want to insert a number based stat property.
+     * @param field The field you want to write. See \ref UDSEntry::StandardFieldTypes for the possible values.
+     * @param value Value to set.
+     * @since 5.8
+     */
+    void insertEntryProperty(uint field, const long long value);
+
 
     /**
      * Call this at the beginning of put(), to give the size of the existing
@@ -707,8 +744,9 @@
      * @param entry The UDSEntry containing all of the object attributes.
      * @since 5.0
      */
-    void listEntry(const UDSEntry &entry);
-
+#ifndef KIOCORE_NO_DEPRECATED
+   KIOCORE_DEPRECATED  void listEntry(const UDSEntry &entry);
+#endif
     /**
      * internal function to connect a slave to/ disconnect from
      * either the slave pool or the application
diff --git a/src/core/udsentry.cpp b/src/core/udsentry.cpp
index 38e40c5..d92b3fd 100644
--- a/src/core/udsentry.cpp
+++ b/src/core/udsentry.cpp
@@ -188,16 +188,19 @@
     const QVector<Field> &fields = a.d->fields;
     const int size = udsIndexes.size();
 
-    s << size;
+    s.writeRawData(reinterpret_cast<const char*>(&size), sizeof(int));
 
     for (int index = 0; index < size; ++index) {
         uint uds = udsIndexes.at(index);
-        s << uds;
+        s.writeRawData(reinterpret_cast<const char*>(&uds), sizeof(uint));
 
         if (uds & KIO::UDSEntry::UDS_STRING) {
-            s << fields.at(index).m_str;
+            const QString &str = fields.at(index).m_str;
+            int size = str.size();
+            s.writeRawData(reinterpret_cast<const char*>(&size), sizeof(int));
+            s.writeRawData(reinterpret_cast<const char*>(str.utf16()), sizeof(ushort) * size);
         } else if (uds & KIO::UDSEntry::UDS_NUMBER) {
-            s << fields.at(index).m_long;
+            s.writeRawData(reinterpret_cast<const char*>(&fields.at(index).m_long), sizeof(long long));
         } else {
             Q_ASSERT_X(false, "KIO::UDSEntry", "Found a field with an invalid type");
         }
@@ -211,8 +214,9 @@
     QVector<Field> &fields = a.d->fields;
     QVector<uint> &udsIndexes = a.d->udsIndexes;
 
-    quint32 size;
-    s >> size;
+    int size;
+    s.readRawData(reinterpret_cast<char*>(&size), sizeof(int));
+
     fields.reserve(size);
     udsIndexes.reserve(size);
 
@@ -220,21 +224,27 @@
     // will often be the same for many entries in a row. Caching them
     // permits to use implicit sharing to save memory.
     static QVector<QString> cachedStrings;
-    if (quint32(cachedStrings.size()) < size) {
+    if (int(cachedStrings.size()) < size) {
         cachedStrings.resize(size);
     }
 
-    for (quint32 i = 0; i < size; ++i) {
-        quint32 uds;
-        s >> uds;
+    for (int i = 0; i < size; ++i) {
+        uint uds;
+        s.readRawData(reinterpret_cast<char*>(&uds), sizeof(uint));
+
         udsIndexes.append(uds);
 
         if (uds & KIO::UDSEntry::UDS_STRING) {
             // If the QString is the same like the one we read for the
             // previous UDSEntry at the i-th position, use an implicitly
             // shared copy of the same QString to save memory.
-            QString buffer;
-            s >> buffer;
+            int length;
+            s.readRawData(reinterpret_cast<char*>(&length), sizeof(int));
+
+            char tempBuffer[length * sizeof(ushort)];
+            s.readRawData(tempBuffer, length * sizeof(ushort));
+
+            QString buffer = QString::fromUtf16(reinterpret_cast<const ushort*>(tempBuffer), length);
 
             if (buffer != cachedStrings.at(i)) {
                 cachedStrings[i] = buffer;
@@ -243,7 +253,7 @@
             fields.append(Field(cachedStrings.at(i)));
         } else if (uds & KIO::UDSEntry::UDS_NUMBER) {
             long long value;
-            s >> value;
+            s.readRawData(reinterpret_cast<char*>(&value), sizeof(long long));
             fields.append(Field(value));
         } else {
             Q_ASSERT_X(false, "KIO::UDSEntry", "Found a field with an invalid type");
diff --git a/src/core/udsentry.h b/src/core/udsentry.h
index 13d50ea..e42ef23 100644
--- a/src/core/udsentry.h
+++ b/src/core/udsentry.h
@@ -100,14 +100,14 @@
     void reserve(int size);
 
     /**
-     * insert field with numeric value
+     * insert field with string value
      * @param field numeric field id
-     * @param value
+     * @param value to set
      */
     void insert(uint field, const QString &value);
 
     /**
-     * insert field with string value
+     * insert field with numeric value
      * @param field numeric tield id
      * @param l value to set
      */
diff --git a/src/ioslaves/file/file.cpp b/src/ioslaves/file/file.cpp
index d31be6d..c6d36d9 100644
--- a/src/ioslaves/file/file.cpp
+++ b/src/ioslaves/file/file.cpp
@@ -83,8 +83,7 @@
 
 static QString readLogFile(const QByteArray &_filename);
 #if HAVE_POSIX_ACL
-static void appendACLAtoms(const QByteArray &path, UDSEntry &entry,
-                           mode_t type);
+static void appendACLAtoms(const QByteArray &path, mode_t type, FileProtocol *slave);
 #endif
 
 extern "C" Q_DECL_EXPORT int kdemain(int argc, char **argv)
@@ -757,13 +756,9 @@
     return mGroupcache[gid];
 }
 
-bool FileProtocol::createUDSEntry(const QString &filename, const QByteArray &path, UDSEntry &entry,
-                                  short int details)
+bool FileProtocol::createUDSEntry(const QString &filename, const QByteArray &path, short int details)
 {
-    assert(entry.count() == 0); // by contract :-)
-    entry.reserve(8);
-
-    entry.insert(KIO::UDSEntry::UDS_NAME, filename);
+    insertEntryProperty(KIO::UDSEntry::UDS_NAME, filename);
 
     mode_t type;
     mode_t access;
@@ -772,15 +767,15 @@
     if (QT_LSTAT(path.data(), &buff) == 0)  {
 
         if (details > 2) {
-            entry.insert(KIO::UDSEntry::UDS_DEVICE_ID, buff.st_dev);
-            entry.insert(KIO::UDSEntry::UDS_INODE, buff.st_ino);
+            insertEntryProperty(KIO::UDSEntry::UDS_DEVICE_ID, buff.st_dev);
+            insertEntryProperty(KIO::UDSEntry::UDS_INODE, buff.st_ino);
         }
 
         if ((buff.st_mode & QT_STAT_MASK) == QT_STAT_LNK) {
 
             const QString linkTarget = QFile::symLinkTarget(QFile::decodeName(path));
 
-            entry.insert(KIO::UDSEntry::UDS_LINK_DEST, linkTarget);
+            insertEntryProperty(KIO::UDSEntry::UDS_LINK_DEST, linkTarget);
 
             // A symlink -> follow it only if details>1
             if (details > 1 && QT_STAT(path.constData(), &buff) == -1) {
@@ -788,9 +783,9 @@
                 type = S_IFMT - 1;
                 access = S_IRWXU | S_IRWXG | S_IRWXO;
 
-                entry.insert(KIO::UDSEntry::UDS_FILE_TYPE, type);
-                entry.insert(KIO::UDSEntry::UDS_ACCESS, access);
-                entry.insert(KIO::UDSEntry::UDS_SIZE, 0LL);
+                insertEntryProperty(KIO::UDSEntry::UDS_FILE_TYPE, type);
+                insertEntryProperty(KIO::UDSEntry::UDS_ACCESS, access);
+                insertEntryProperty(KIO::UDSEntry::UDS_SIZE, 0LL);
                 goto notype;
 
             }
@@ -803,30 +798,30 @@
     type = buff.st_mode & S_IFMT; // extract file type
     access = buff.st_mode & 07777; // extract permissions
 
-    entry.insert(KIO::UDSEntry::UDS_FILE_TYPE, type);
-    entry.insert(KIO::UDSEntry::UDS_ACCESS, access);
+    insertEntryProperty(KIO::UDSEntry::UDS_FILE_TYPE, type);
+    insertEntryProperty(KIO::UDSEntry::UDS_ACCESS, access);
 
-    entry.insert(KIO::UDSEntry::UDS_SIZE, buff.st_size);
+    insertEntryProperty(KIO::UDSEntry::UDS_SIZE, buff.st_size);
 
 #if HAVE_POSIX_ACL
     if (details > 1) {
         /* Append an atom indicating whether the file has extended acl information
          * and if withACL is specified also one with the acl itself. If it's a directory
          * and it has a default ACL, also append that. */
-        appendACLAtoms(path, entry, type);
+        appendACLAtoms(path, type, this);
     }
 #endif
 
 notype:
     if (details > 0) {
-        entry.insert(KIO::UDSEntry::UDS_MODIFICATION_TIME, buff.st_mtime);
+        insertEntryProperty(KIO::UDSEntry::UDS_MODIFICATION_TIME, buff.st_mtime);
 #ifndef Q_OS_WIN
-        entry.insert(KIO::UDSEntry::UDS_USER, getUserName(KUserId(buff.st_uid)));
-        entry.insert(KIO::UDSEntry::UDS_GROUP, getGroupName(KGroupId(buff.st_gid)));
+        insertEntryProperty(KIO::UDSEntry::UDS_USER, getUserName(KUserId(buff.st_uid)));
+        insertEntryProperty(KIO::UDSEntry::UDS_GROUP, getGroupName(KGroupId(buff.st_gid)));
 #else
 #pragma message("TODO: st_uid and st_gid are always zero, use GetSecurityInfo to find the owner")
 #endif
-        entry.insert(KIO::UDSEntry::UDS_ACCESS_TIME, buff.st_atime);
+        insertEntryProperty(KIO::UDSEntry::UDS_ACCESS_TIME, buff.st_atime);
     }
 
     // Note: buff.st_ctime isn't the creation time !
@@ -1224,7 +1219,7 @@
     return (acl_equiv_mode(acl, 0) != 0);
 }
 
-static void appendACLAtoms(const QByteArray &path, UDSEntry &entry, mode_t type)
+static void appendACLAtoms(const QByteArray &path, mode_t type, FileProtocol *slave)
 {
     // first check for a noop
     if (acl_extended_file(path.data()) == 0) {
@@ -1249,18 +1244,18 @@
     }
     if (acl || defaultAcl) {
         // qDebug() << path.constData() << "has extended ACL entries";
-        entry.insert(KIO::UDSEntry::UDS_EXTENDED_ACL, 1);
+        slave->insertEntryProperty(KIO::UDSEntry::UDS_EXTENDED_ACL, 1);
 
         if (acl) {
             const QString str = aclToText(acl);
-            entry.insert(KIO::UDSEntry::UDS_ACL_STRING, str);
+            slave->insertEntryProperty(KIO::UDSEntry::UDS_ACL_STRING, str);
             // qDebug() << path.constData() << "ACL:" << str;
             acl_free(acl);
         }
 
         if (defaultAcl) {
             const QString str = aclToText(defaultAcl);
-            entry.insert(KIO::UDSEntry::UDS_DEFAULT_ACL_STRING, str);
+            slave->insertEntryProperty(KIO::UDSEntry::UDS_DEFAULT_ACL_STRING, str);
             // qDebug() << path.constData() << "DEFAULT ACL:" << str;
             acl_free(defaultAcl);
         }
diff --git a/src/ioslaves/file/file.h b/src/ioslaves/file/file.h
index d966ac1..9011c35 100644
--- a/src/ioslaves/file/file.h
+++ b/src/ioslaves/file/file.h
@@ -88,8 +88,7 @@
     void virtual_hook(int id, void *data) Q_DECL_OVERRIDE;
 
 private:
-    bool createUDSEntry(const QString &filename, const QByteArray &path, KIO::UDSEntry &entry,
-                        short int details);
+    bool createUDSEntry(const QString &filename, const QByteArray &path, short int details);
     int setACL(const char *path, mode_t perm, bool _directoryDefault);
     QString getUserName(KUserId uid) const;
     QString getGroupName(KGroupId gid) const;
diff --git a/src/ioslaves/file/file_unix.cpp b/src/ioslaves/file/file_unix.cpp
index c8202f8..c9595b8 100644
--- a/src/ioslaves/file/file_unix.cpp
+++ b/src/ioslaves/file/file_unix.cpp
@@ -350,15 +350,13 @@
     const QString sDetails = metaData(QLatin1String("details"));
     const int details = sDetails.isEmpty() ? 2 : sDetails.toInt();
     //qDebug() << "========= LIST " << url << "details=" << details << " =========";
-    UDSEntry entry;
 
 #ifndef HAVE_DIRENT_D_TYPE
     QT_STATBUF st;
 #endif
     QT_DIRENT *ep;
     while ((ep = QT_READDIR(dp)) != 0) {
-        entry.clear();
-
+        newEntry();
         const QString filename = QFile::decodeName(ep->d_name);
 
         /*
@@ -372,9 +370,9 @@
          *
          */
         if (details == 0) {
-            entry.insert(KIO::UDSEntry::UDS_NAME, filename);
+            insertEntryProperty(KIO::UDSEntry::UDS_NAME, filename);
 #ifdef HAVE_DIRENT_D_TYPE
-            entry.insert(KIO::UDSEntry::UDS_FILE_TYPE,
+            insertEntryProperty(KIO::UDSEntry::UDS_FILE_TYPE,
                          (ep->d_type == DT_DIR) ? S_IFDIR : S_IFREG);
             const bool isSymLink = (ep->d_type == DT_LNK);
 #else
@@ -389,18 +387,12 @@
             if (isSymLink) {
                 // for symlinks obey the UDSEntry contract and provide UDS_LINK_DEST
                 // even if we don't know the link dest (and DeleteJob doesn't care...)
-                entry.insert(KIO::UDSEntry::UDS_LINK_DEST, QLatin1String("Dummy Link Target"));
+                insertEntryProperty(KIO::UDSEntry::UDS_LINK_DEST, QLatin1String("Dummy Link Target"));
             }
-            listEntry(entry);
-
         } else {
-            if (createUDSEntry(filename, QByteArray(ep->d_name), entry, details)) {
-                listEntry(entry);
-            }
+            createUDSEntry(filename, QByteArray(ep->d_name), details);
         }
     }
-
-    closedir(dp);
 
     // Restore the path
     QDir::setCurrent(pathBuffer);
@@ -616,8 +608,8 @@
     const QString sDetails = metaData(QLatin1String("details"));
     const int details = sDetails.isEmpty() ? 2 : sDetails.toInt();
 
-    UDSEntry entry;
-    if (!createUDSEntry(url.fileName(), _path, entry, details)) {
+    newEntry();
+    if (!createUDSEntry(url.fileName(), _path, details)) {
         error(KIO::ERR_DOES_NOT_EXIST, path);
         return;
     }
@@ -629,7 +621,6 @@
     }
 /////////
 #endif
-    statEntry(entry);
 
     finished();
 }

-- 
To view, visit https://gerrit.vesnicky.cesnet.cz/r/348
To unsubscribe, visit https://gerrit.vesnicky.cesnet.cz/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibb61f0c3a406337c54d066cac169ca534ec1d8b2
Gerrit-PatchSet: 1
Gerrit-Project: kio
Gerrit-Branch: master
Gerrit-Owner: Mark Gaiser <markg85 at gmail.com>
Gerrit-Reviewer: Alex Richardson <arichardson.kde at gmail.com>
Gerrit-Reviewer: Frank Reininghaus <frank78ac at googlemail.com>
Gerrit-Reviewer: Sysadmin Testing Account <null at kde.org>


More information about the Kde-frameworks-devel mailing list