diff --git a/src/core/kcoredirlister.cpp b/src/core/kcoredirlister.cpp --- a/src/core/kcoredirlister.cpp +++ b/src/core/kcoredirlister.cpp @@ -661,7 +661,7 @@ const QUrl dir = _dir.adjusted(QUrl::StripTrailingSlash); if (!checkUpdate(dir)) { - if (dir.isLocalFile() && findByUrl(nullptr, dir)) { + if (dir.isLocalFile() && !findByUrl(nullptr, dir).isNull()) { pendingUpdates.insert(dir.toLocalFile()); if (!pendingUpdateTimer.isActive()) { pendingUpdateTimer.start(500); @@ -770,12 +770,7 @@ KFileItem KCoreDirListerCache::itemForUrl(const QUrl &url) const { - KFileItem *item = findByUrl(nullptr, url); - if (item) { - return *item; - } else { - return KFileItem(); - } + return kDirListerCache()->findByUrl(nullptr, url); } KCoreDirListerCache::DirItem *KCoreDirListerCache::dirItemForUrl(const QUrl &dir) const @@ -811,7 +806,7 @@ return KFileItem(); } -KFileItem *KCoreDirListerCache::findByUrl(const KCoreDirLister *lister, const QUrl &_u) const +KFileItem KCoreDirListerCache::findByUrl(const KCoreDirLister *lister, const QUrl &_u, const bool take) { QUrl url(_u); url = url.adjusted(QUrl::StripTrailingSlash); @@ -821,11 +816,17 @@ if (dirItem) { // If lister is set, check that it contains this dir if (!lister || lister->d->lstDirs.contains(parentDir)) { + NonMovableFileItemList::iterator it = dirItem->lstItems.begin(); const NonMovableFileItemList::iterator end = dirItem->lstItems.end(); for (; it != end; ++it) { if ((*it).url() == url) { - return &*it; + KFileItem retKFileItem = *it; + // If take remove the element from the list. + if (take) { + dirItem->lstItems.erase(it); + } + return retKFileItem; } } } @@ -838,11 +839,12 @@ if (dirItem && !dirItem->rootItem.isNull() && dirItem->rootItem.url() == url) { // If lister is set, check that it contains this dir if (!lister || lister->d->lstDirs.contains(url)) { - return &dirItem->rootItem; + // FIXME: ¿take? + return dirItem->rootItem; } } - return nullptr; + return KFileItem(); } void KCoreDirListerCache::slotFilesAdded(const QString &dir /*url*/) // from KDirNotify signals @@ -927,8 +929,9 @@ QStringList::const_iterator it = fileList.begin(); for (; it != fileList.end(); ++it) { QUrl url(*it); - KFileItem *fileitem = findByUrl(nullptr, url); - if (!fileitem) { + // remove the url from the list. + const KFileItem fileitem = findByUrl(nullptr, url, true); + if (!fileitem.isNull()) { qCDebug(KIO_CORE_DIRLISTER) << "item not found for" << url; continue; } @@ -965,20 +968,21 @@ #endif QUrl oldurl = src.adjusted(QUrl::StripTrailingSlash); - KFileItem *fileitem = findByUrl(nullptr, oldurl); - if (!fileitem) { + // remove the item from the list + KFileItem fileitem = findByUrl(nullptr, oldurl, true); + if (fileitem.isNull()) { qCDebug(KIO_CORE_DIRLISTER) << "Item not found:" << oldurl; return; } - const KFileItem oldItem = *fileitem; + const KFileItem oldItem = fileitem; // Dest already exists? Was overwritten then (testcase: #151851) // We better emit it as deleted -before- doing the renaming, otherwise // the "update" mechanism will emit the old one as deleted and // kdirmodel will delete the new (renamed) one! - KFileItem *existingDestItem = findByUrl(nullptr, dst); - if (existingDestItem) { + KFileItem existingDestItem = findByUrl(nullptr, dst); + if (!existingDestItem.isNull()) { qCDebug(KIO_CORE_DIRLISTER) << dst << "already existed, let's delete it"; slotFilesRemoved(QList() << dst); } @@ -987,15 +991,15 @@ // to be updating the name only (since they can't see the URL). // Check to see if a URL exists, and if so, if only the file part has changed, // only update the name and not the underlying URL. - bool nameOnly = !fileitem->entry().stringValue(KIO::UDSEntry::UDS_URL).isEmpty(); + bool nameOnly = !fileitem.entry().stringValue(KIO::UDSEntry::UDS_URL).isEmpty(); nameOnly = nameOnly && src.adjusted(QUrl::RemoveFilename) == dst.adjusted(QUrl::RemoveFilename); - if (!nameOnly && fileitem->isDir()) { + if (!nameOnly && fileitem.isDir()) { renameDir(oldurl, dst); // #172945 - if the fileitem was the root item of a DirItem that was just removed from the cache, // then it's a dangling pointer now... fileitem = findByUrl(nullptr, oldurl); - if (!fileitem) { //deleted from cache altogether, #188807 + if (fileitem.isNull()) { //deleted from cache altogether, #188807 return; } } @@ -1005,18 +1009,22 @@ slotFilesChanged(QStringList() << src.toString()); } else { if (nameOnly) { - fileitem->setName(dst.fileName()); + fileitem.setName(dst.fileName()); } else { - fileitem->setUrl(dst); + fileitem.setUrl(dst); } if (!dstPath.isEmpty()) { - fileitem->setLocalPath(dstPath); + fileitem.setLocalPath(dstPath); } - fileitem->refreshMimeType(); - fileitem->determineMimeType(); - QSet listers = emitRefreshItem(oldItem, *fileitem); + fileitem.refreshMimeType(); + fileitem.determineMimeType(); + + // FIXME: Reinsert fileitem in the directory list? + + + QSet listers = emitRefreshItem(oldItem, fileitem); Q_FOREACH (KCoreDirLister *kdl, listers) { kdl->d->emitItems(); } @@ -1146,10 +1154,10 @@ void KCoreDirListerCache::handleFileDirty(const QUrl &url) { // A file: do we know about it already? - KFileItem *existingItem = findByUrl(nullptr, url); + KFileItem existingItem = findByUrl(nullptr, url); const QUrl dir = url.adjusted(QUrl::RemoveFilename | QUrl::StripTrailingSlash); QString filePath = url.toLocalFile(); - if (!existingItem) { + if (existingItem.isNull()) { // No - update the parent dir then handleDirDirty(dir); } @@ -1777,13 +1785,13 @@ delayedMimeTypes &= kdl->d->delayedMimeTypes; } - typedef QHash FileItemHash; // fileName -> KFileItem* + typedef QHash FileItemHash; // fileName -> KFileItem FileItemHash fileItems; // Fill the hash from the old list of items. We'll remove entries as we see them // in the new listing, and the resulting hash entries will be the deleted items. for (NonMovableFileItemList::iterator kit = dir->lstItems.begin(), kend = dir->lstItems.end(); kit != kend; ++kit) { - fileItems.insert((*kit).name(), &*kit); + fileItems.insert((*kit).name(), *kit); } QSet filesToHide; @@ -1836,23 +1844,23 @@ // Find this item FileItemHash::iterator fiit = fileItems.find(item.name()); if (fiit != fileItems.end()) { - KFileItem* tmp = fiit.value(); - QSet::iterator pru_it = pendingRemoteUpdates.find(tmp); + KFileItem tmp = fiit.value(); + QSet::iterator pru_it = pendingRemoteUpdates.find(tmp); const bool inPendingRemoteUpdates = (pru_it != pendingRemoteUpdates.end()); // check if something changed for this file, using KFileItem::cmp() - if (!tmp->cmp(item) || inPendingRemoteUpdates) { + if (!tmp.cmp(item) || inPendingRemoteUpdates) { if (inPendingRemoteUpdates) { pendingRemoteUpdates.erase(pru_it); } - qCDebug(KIO_CORE_DIRLISTER) << "file changed:" << tmp->name(); + qCDebug(KIO_CORE_DIRLISTER) << "file changed:" << tmp.name(); - const KFileItem oldItem = *tmp; - *tmp = item; + const KFileItem oldItem = tmp; + tmp = item; foreach (KCoreDirLister *kdl, listers) { - kdl->d->addRefreshItem(jobUrl, oldItem, *tmp); + kdl->d->addRefreshItem(jobUrl, oldItem, tmp); } } // Seen, remove @@ -1928,15 +1936,15 @@ job->kill(); } -void KCoreDirListerCache::deleteUnmarkedItems(const QList &listers, NonMovableFileItemList &lstItems, const QHash &itemsToDelete) +void KCoreDirListerCache::deleteUnmarkedItems(const QList &listers, NonMovableFileItemList &lstItems, const QHash &itemsToDelete) { // Make list of deleted items (for emitting) KFileItemList deletedItems; - QHashIterator kit(itemsToDelete); + QHashIterator kit(itemsToDelete); while (kit.hasNext()) { - const KFileItem& item = *kit.next().value(); + const KFileItem item = kit.next().value(); deletedItems.append(item); - qCDebug(KIO_CORE_DIRLISTER) << "deleted:" << item.name() << &item; + qCDebug(KIO_CORE_DIRLISTER) << "deleted:" << item.name() << item; } // Delete all remaining items @@ -2039,13 +2047,13 @@ foreach (const QString &file, pendingUpdates) { // always a local path qCDebug(KIO_CORE_DIRLISTER) << file; QUrl u = QUrl::fromLocalFile(file); - KFileItem *item = findByUrl(nullptr, u); // search all items - if (item) { + KFileItem item = findByUrl(nullptr, u); // search all items + if (!item.isNull()) { // we need to refresh the item, because e.g. the permissions can have changed. - KFileItem oldItem = *item; - item->refresh(); - if (!oldItem.cmp(*item)) { - listers |= emitRefreshItem(oldItem, *item); + KFileItem oldItem = item; + item.refresh(); + if (!oldItem.cmp(item)) { + listers |= emitRefreshItem(oldItem, item); } } } @@ -2310,12 +2318,7 @@ KFileItem KCoreDirLister::findByUrl(const QUrl &_url) const { - KFileItem *item = kDirListerCache()->findByUrl(this, _url); - if (item) { - return *item; - } else { - return KFileItem(); - } + return kDirListerCache()->findByUrl(this, _url); } KFileItem KCoreDirLister::findByName(const QString &_name) const diff --git a/src/core/kcoredirlister_p.h b/src/core/kcoredirlister_p.h --- a/src/core/kcoredirlister_p.h +++ b/src/core/kcoredirlister_p.h @@ -47,8 +47,7 @@ * achieved by allocating space for each individual item, and store pointers to * these items in the contiguous region in memory. * - * TODO: Try to get rid of the raw KFileItem pointers in KCoreDirListerCache, and - * replace all occurrences of NonMovableFileItem(List) with KFileItem(List). + * TODO: replace all occurrences of NonMovableFileItem(List) with KFileItem(List). */ class NonMovableFileItem : public KFileItem { @@ -66,8 +65,8 @@ KFileItem findByName(const QString &fileName) const { - const_iterator it = begin(); - const const_iterator itend = end(); + const_iterator it = constBegin(); + const const_iterator itend = constEnd(); for (; it != itend; ++it) { if ((*it).name() == fileName) { return *it; @@ -252,10 +251,12 @@ void forgetDirs(KCoreDirLister *lister, const QUrl &_url, bool notify); KFileItem findByName(const KCoreDirLister *lister, const QString &_name) const; + // findByUrl returns a pointer so that it's possible to modify the item. // See itemForUrl for the version that returns a readonly kfileitem. // @param lister can be 0. If set, it is checked that the url is held by the lister - KFileItem *findByUrl(const KCoreDirLister *lister, const QUrl &url) const; + // If take=true, the KFileItem found is removed from the list of lister + KFileItem findByUrl(const KCoreDirLister *lister, const QUrl &url, const bool take=false); // Called by CachedItemsJob: // Emits the cached items, for this lister and this url @@ -332,7 +333,7 @@ // when there were items deleted from the filesystem all the listers holding // the parent directory need to be notified, the items have to be deleted // and removed from the cache including all the children. - void deleteUnmarkedItems(const QList&, NonMovableFileItemList &lstItems, const QHash &itemsToDelete); + void deleteUnmarkedItems(const QList&, NonMovableFileItemList &lstItems, const QHash &itemsToDelete); // Helper method called when we know that a list of items was deleted void itemsDeleted(const QList &listers, const KFileItemList &deletedItems); @@ -517,7 +518,7 @@ // changes yet, we need to wait for the "update" directory listing. // The cmp() call can't differ mimetypes since they are determined on demand, // this is why we need to remember those files here. - QSet pendingRemoteUpdates; + QSet pendingRemoteUpdates; // the KDirNotify signals OrgKdeKDirNotifyInterface *kdirnotify;