summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorElvis Angelaccio <elvis.angelaccio@kde.org>2016-10-12 17:14:20 (GMT)
committerElvis Angelaccio <elvis.angelaccio@kde.org>2016-10-12 17:20:49 (GMT)
commit377dfcfebdd6bd3cafcc1497182efd0660273503 (patch)
tree4f29e1bcec18313f02f33df14eafe5d1fe54ad4e
parent6d5d3ff69b9ac38b25add327542f8cd444d7d02c (diff)
Fix crash when moving files with clizip
Entry objects are created by the plugins, so it is wrong to delete them from other places. If the plugins run in the main thread, we can just set a parent to the entries. Otherwise (in the libarchive case) we need to manually delete them. Closes T3988 Differential Revision: D3027
-rw-r--r--kerfuffle/archiveentry.cpp11
-rw-r--r--kerfuffle/archiveentry.h1
-rw-r--r--part/archivemodel.cpp38
-rw-r--r--part/archivemodel.h4
-rw-r--r--plugins/clizipplugin/cliplugin.cpp2
-rw-r--r--plugins/libarchive/libarchiveplugin.cpp5
-rw-r--r--plugins/libarchive/libarchiveplugin.h1
-rw-r--r--plugins/libsinglefileplugin/singlefileplugin.cpp2
8 files changed, 31 insertions, 33 deletions
diff --git a/kerfuffle/archiveentry.cpp b/kerfuffle/archiveentry.cpp
index 3ffe990..0f60e92 100644
--- a/kerfuffle/archiveentry.cpp
+++ b/kerfuffle/archiveentry.cpp
@@ -42,7 +42,6 @@ Archive::Entry::Entry(QObject *parent, const QString &fullPath, const QString &r
Archive::Entry::~Entry()
{
- clear();
}
void Archive::Entry::copyMetaData(const Archive::Entry *sourceEntry)
@@ -91,7 +90,7 @@ void Archive::Entry::removeEntryAt(int index)
{
Q_ASSERT(isDir());
Q_ASSERT(index < m_entries.count());
- delete m_entries.takeAt(index);
+ m_entries.remove(index);
}
Archive::Entry *Archive::Entry::getParent() const
@@ -179,14 +178,6 @@ void Archive::Entry::returnDirEntries(QVector<Entry*> *store)
}
}
-void Archive::Entry::clear()
-{
- if (isDir()) {
- qDeleteAll(m_entries);
- m_entries.clear();
- }
-}
-
bool Archive::Entry::operator==(const Archive::Entry &right) const
{
return m_fullPath == right.m_fullPath;
diff --git a/kerfuffle/archiveentry.h b/kerfuffle/archiveentry.h
index e569430..662a4b0 100644
--- a/kerfuffle/archiveentry.h
+++ b/kerfuffle/archiveentry.h
@@ -96,7 +96,6 @@ public:
Entry *find(const QString &name) const;
Entry *findByPath(const QStringList & pieces, int index = 0) const;
void returnDirEntries(QVector<Entry*> *store);
- void clear();
bool operator==(const Archive::Entry &right) const;
diff --git a/part/archivemodel.cpp b/part/archivemodel.cpp
index 31fa689..51948b4 100644
--- a/part/archivemodel.cpp
+++ b/part/archivemodel.cpp
@@ -148,12 +148,11 @@ private:
ArchiveModel::ArchiveModel(const QString &dbusPathName, QObject *parent)
: QAbstractItemModel(parent)
- , m_rootEntry()
, m_dbusPathName(dbusPathName)
, m_numberOfFiles(0)
, m_numberOfFolders(0)
{
- m_rootEntry.setProperty("isDirectory", true);
+ initRootEntry();
}
ArchiveModel::~ArchiveModel()
@@ -293,7 +292,7 @@ QModelIndex ArchiveModel::index(int row, int column, const QModelIndex &parent)
if (hasIndex(row, column, parent)) {
const Archive::Entry *parentEntry = parent.isValid()
? static_cast<Archive::Entry*>(parent.internalPointer())
- : &m_rootEntry;
+ : m_rootEntry.data();
Q_ASSERT(parentEntry->isDir());
@@ -311,7 +310,7 @@ QModelIndex ArchiveModel::parent(const QModelIndex &index) const
if (index.isValid()) {
Archive::Entry *item = static_cast<Archive::Entry*>(index.internalPointer());
Q_ASSERT(item);
- if (item->getParent() && (item->getParent() != &m_rootEntry)) {
+ if (item->getParent() && (item->getParent() != m_rootEntry.data())) {
return createIndex(item->getParent()->row(), 0, item->getParent());
}
}
@@ -355,7 +354,7 @@ int ArchiveModel::rowCount(const QModelIndex &parent) const
if (parent.column() <= 0) {
const Archive::Entry *parentEntry = parent.isValid()
? static_cast<Archive::Entry*>(parent.internalPointer())
- : &m_rootEntry;
+ : m_rootEntry.data();
if (parentEntry && parentEntry->isDir()) {
return parentEntry->entries().count();
@@ -379,8 +378,8 @@ void ArchiveModel::sort(int column, Qt::SortOrder order)
emit layoutAboutToBeChanged();
QVector<Archive::Entry*> dirEntries;
- m_rootEntry.returnDirEntries(&dirEntries);
- dirEntries.append(&m_rootEntry);
+ m_rootEntry->returnDirEntries(&dirEntries);
+ dirEntries.append(m_rootEntry.data());
const ArchiveModelSorter modelSorter(m_showColumns.at(column), order);
@@ -498,6 +497,12 @@ QString ArchiveModel::cleanFileName(const QString& fileName)
return fileName;
}
+void ArchiveModel::initRootEntry()
+{
+ m_rootEntry.reset(new Archive::Entry());
+ m_rootEntry->setProperty("isDirectory", true);
+}
+
Archive::Entry *ArchiveModel::parentFor(const Archive::Entry *entry, InsertBehaviour behaviour)
{
QStringList pieces = entry->fullPath().split(QLatin1Char('/'), QString::SkipEmptyParts);
@@ -528,7 +533,7 @@ Archive::Entry *ArchiveModel::parentFor(const Archive::Entry *entry, InsertBehav
}
}
- Archive::Entry *parent = &m_rootEntry;
+ Archive::Entry *parent = m_rootEntry.data();
foreach(const QString &piece, pieces) {
Archive::Entry *entry = parent->find(piece);
@@ -538,7 +543,7 @@ Archive::Entry *ArchiveModel::parentFor(const Archive::Entry *entry, InsertBehav
// and then delete the existing one (see ArchiveModel::newEntry).
entry = new Archive::Entry(parent);
- entry->setProperty("fullPath", (parent == &m_rootEntry)
+ entry->setProperty("fullPath", (parent == m_rootEntry.data())
? piece + QLatin1Char('/')
: parent->fullPath(WithTrailingSlash) + piece + QLatin1Char('/'));
entry->setProperty("isDirectory", true);
@@ -563,7 +568,7 @@ Archive::Entry *ArchiveModel::parentFor(const Archive::Entry *entry, InsertBehav
QModelIndex ArchiveModel::indexForEntry(Archive::Entry *entry)
{
Q_ASSERT(entry);
- if (entry != &m_rootEntry) {
+ if (entry != m_rootEntry.data()) {
Q_ASSERT(entry->getParent());
Q_ASSERT(entry->getParent()->isDir());
return createIndex(entry->row(), 0, entry);
@@ -578,7 +583,7 @@ void ArchiveModel::slotEntryRemoved(const QString & path)
return;
}
- Archive::Entry *entry = m_rootEntry.findByPath(entryFileName.split(QLatin1Char('/'), QString::SkipEmptyParts));
+ Archive::Entry *entry = m_rootEntry->findByPath(entryFileName.split(QLatin1Char('/'), QString::SkipEmptyParts));
if (entry) {
Archive::Entry *parent = entry->getParent();
QModelIndex index = indexForEntry(entry);
@@ -656,7 +661,7 @@ void ArchiveModel::newEntry(Archive::Entry *receivedEntry, InsertBehaviour behav
}
// Skip already created entries.
- Archive::Entry *existing = m_rootEntry.findByPath(entryFileName.split(QLatin1Char('/')));
+ Archive::Entry *existing = m_rootEntry->findByPath(entryFileName.split(QLatin1Char('/')));
if (existing) {
qCDebug(ARK) << "Refreshing entry for" << entryFileName;
@@ -677,7 +682,6 @@ void ArchiveModel::newEntry(Archive::Entry *receivedEntry, InsertBehaviour behav
if (entry) {
entry->copyMetaData(receivedEntry);
entry->setProperty("fullPath", entryFileName);
- delete receivedEntry;
} else {
receivedEntry->setParent(parent);
insertEntry(receivedEntry, behaviour);
@@ -729,9 +733,9 @@ Kerfuffle::Archive* ArchiveModel::archive() const
void ArchiveModel::reset()
{
m_archive.reset(Q_NULLPTR);
- m_rootEntry.clear();
s_previousMatch = Q_NULLPTR;
s_previousPieces->clear();
+ initRootEntry();
// TODO: make sure if it's ok to not have calls to beginRemoveColumns here
m_showColumns.clear();
@@ -883,9 +887,9 @@ bool ArchiveModel::conflictingEntries(QList<const Archive::Entry*> &conflictingE
QStringList destinationParts = entries.first().split(QLatin1Char('/'), QString::SkipEmptyParts);
destinationParts.removeLast();
if (destinationParts.count() > 0) {
- destination = m_rootEntry.findByPath(destinationParts);
+ destination = m_rootEntry->findByPath(destinationParts);
} else {
- destination = &m_rootEntry;
+ destination = m_rootEntry.data();
}
}
const Archive::Entry *lastDirEntry = destination;
@@ -1010,7 +1014,7 @@ void ArchiveModel::countEntriesAndSize()
QElapsedTimer timer;
timer.start();
- traverseAndCountDirNode(&m_rootEntry);
+ traverseAndCountDirNode(m_rootEntry.data());
qCDebug(ARK) << "Time to count entries and size:" << timer.elapsed() << "ms";
}
diff --git a/part/archivemodel.h b/part/archivemodel.h
index 58b3cff..56dbddf 100644
--- a/part/archivemodel.h
+++ b/part/archivemodel.h
@@ -146,6 +146,8 @@ private:
*/
QString cleanFileName(const QString& fileName);
+ void initRootEntry();
+
enum InsertBehaviour { NotifyViews, DoNotNotifyViews };
Archive::Entry *parentFor(const Kerfuffle::Archive::Entry *entry, InsertBehaviour behaviour = NotifyViews);
QModelIndex indexForEntry(Archive::Entry *entry);
@@ -163,7 +165,7 @@ private:
QList<int> m_showColumns;
QScopedPointer<Kerfuffle::Archive> m_archive;
- Archive::Entry m_rootEntry;
+ QScopedPointer<Archive::Entry> m_rootEntry;
QHash<QString, QIcon> m_entryIcons;
QString m_dbusPathName;
diff --git a/plugins/clizipplugin/cliplugin.cpp b/plugins/clizipplugin/cliplugin.cpp
index 3d927fe..b5d7f8c 100644
--- a/plugins/clizipplugin/cliplugin.cpp
+++ b/plugins/clizipplugin/cliplugin.cpp
@@ -166,7 +166,7 @@ bool CliPlugin::readListLine(const QString &line)
case ParseStateEntry:
QRegularExpressionMatch rxMatch = entryPattern.match(line);
if (rxMatch.hasMatch()) {
- Archive::Entry *e = new Archive::Entry();
+ Archive::Entry *e = new Archive::Entry(this);
e->setProperty("permissions", rxMatch.captured(1));
// #280354: infozip may not show the right attributes for a given directory, so an entry
diff --git a/plugins/libarchive/libarchiveplugin.cpp b/plugins/libarchive/libarchiveplugin.cpp
index c808fd4..51ddaef 100644
--- a/plugins/libarchive/libarchiveplugin.cpp
+++ b/plugins/libarchive/libarchiveplugin.cpp
@@ -432,7 +432,7 @@ bool LibarchivePlugin::initializeReader()
void LibarchivePlugin::emitEntryFromArchiveEntry(struct archive_entry *aentry)
{
- Archive::Entry *e = new Archive::Entry(Q_NULLPTR);
+ auto e = QSharedPointer<Archive::Entry>(new Archive::Entry());
#ifdef _MSC_VER
e->setProperty("fullPath", QDir::fromNativeSeparators(QString::fromUtf16((ushort*)archive_entry_pathname_w(aentry))));
@@ -460,7 +460,8 @@ void LibarchivePlugin::emitEntryFromArchiveEntry(struct archive_entry *aentry)
e->setProperty("timestamp", QDateTime::fromTime_t(archive_entry_mtime(aentry)));
- emit entry(e);
+ emit entry(e.data());
+ m_emittedEntries << e;
}
int LibarchivePlugin::extractionFlags() const
diff --git a/plugins/libarchive/libarchiveplugin.h b/plugins/libarchive/libarchiveplugin.h
index 6bb799e..71e8e90 100644
--- a/plugins/libarchive/libarchiveplugin.h
+++ b/plugins/libarchive/libarchiveplugin.h
@@ -96,6 +96,7 @@ private:
qlonglong m_currentExtractedFilesSize;
bool m_emitNoEntries;
qlonglong m_extractedFilesSize;
+ QVector<QSharedPointer<Archive::Entry>> m_emittedEntries;
};
#endif // LIBARCHIVEPLUGIN_H
diff --git a/plugins/libsinglefileplugin/singlefileplugin.cpp b/plugins/libsinglefileplugin/singlefileplugin.cpp
index ddd5158..e2735b3 100644
--- a/plugins/libsinglefileplugin/singlefileplugin.cpp
+++ b/plugins/libsinglefileplugin/singlefileplugin.cpp
@@ -105,7 +105,7 @@ bool LibSingleFileInterface::list()
{
qCDebug(ARK) << "Listing archive contents";
- Kerfuffle::Archive::Entry *e = new Kerfuffle::Archive::Entry();
+ Kerfuffle::Archive::Entry *e = new Kerfuffle::Archive::Entry(this);
e->setProperty("fullPath", uncompressedFileName());
emit entry(e);