[Pkg-owncloud-commits] [owncloud-client] 79/159: SyncEngine: Fix a crash caused by an invalid DiscoveryDirectoryResult::iterator #3051
Sandro Knauß
hefee-guest at moszumanska.debian.org
Fri May 1 13:05:25 UTC 2015
This is an automated email from the git hooks/post-receive script.
hefee-guest pushed a commit to branch master
in repository owncloud-client.
commit 4a890eae38f86d5f689983e84da951a1b42c17d8
Author: Jocelyn Turcotte <jturcotte at woboq.com>
Date: Thu Apr 2 15:16:33 2015 +0200
SyncEngine: Fix a crash caused by an invalid DiscoveryDirectoryResult::iterator #3051
The default constructor of the iterator points to NULL, which makes
it != end() but invalid to dereference.
Use an integer index instead to keep 0 as a valid default value that
can always correctly be checked against size().
Also make sure that no data is shared between threads by making the
csync_vio_file_stat_t copyable and passing it as const.
---
src/libsync/discoveryphase.cpp | 29 ++++++---------------
src/libsync/discoveryphase.h | 58 +++++++++++++++++++++---------------------
2 files changed, 37 insertions(+), 50 deletions(-)
diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp
index f68a697..b8c580a 100644
--- a/src/libsync/discoveryphase.cpp
+++ b/src/libsync/discoveryphase.cpp
@@ -268,7 +268,7 @@ void DiscoverySingleDirectoryJob::directoryListingIteratedSlot(QString file,QMap
}
- csync_vio_file_stat_t *file_stat = propertyMapToFileStat(map);
+ FileStatPointer file_stat(propertyMapToFileStat(map));
file_stat->name = strdup(file.toUtf8());
if (!file_stat->etag || strlen(file_stat->etag) == 0) {
qDebug() << "WARNING: etag of" << file_stat->name << "is" << file_stat->etag << " This must not happen.";
@@ -319,15 +319,6 @@ void DiscoveryMainThread::setupHooks(DiscoveryJob *discoveryJob, const QString &
connect(discoveryJob, SIGNAL(doOpendirSignal(QString,DiscoveryDirectoryResult*)),
this, SLOT(doOpendirSlot(QString,DiscoveryDirectoryResult*)),
Qt::QueuedConnection);
- connect(discoveryJob, SIGNAL(doClosedirSignal(QString)),
- this, SLOT(doClosedirSlot(QString)),
- Qt::QueuedConnection);
-}
-
-void DiscoveryMainThread::doClosedirSlot(QString path)
-{
- //qDebug() << Q_FUNC_INFO << "Invalidating" << path;
- deleteCacheEntry(path);
}
// Coming from owncloud_opendir -> DiscoveryJob::vio_opendir_hook -> doOpendirSignal
@@ -352,8 +343,8 @@ void DiscoveryMainThread::doOpendirSlot(QString subPath, DiscoveryDirectoryResul
// Schedule the DiscoverySingleDirectoryJob
_singleDirJob = new DiscoverySingleDirectoryJob(_account, fullPath, this);
- QObject::connect(_singleDirJob, SIGNAL(finishedWithResult(QLinkedList<csync_vio_file_stat_t *>)),
- this, SLOT(singleDirectoryJobResultSlot(QLinkedList<csync_vio_file_stat_t*>)));
+ QObject::connect(_singleDirJob, SIGNAL(finishedWithResult(const QList<FileStatPointer> &)),
+ this, SLOT(singleDirectoryJobResultSlot(const QList<FileStatPointer> &)));
QObject::connect(_singleDirJob, SIGNAL(finishedWithError(int,QString)),
this, SLOT(singleDirectoryJobFinishedWithErrorSlot(int,QString)));
QObject::connect(_singleDirJob, SIGNAL(firstDirectoryPermissions(QString)),
@@ -364,7 +355,7 @@ void DiscoveryMainThread::doOpendirSlot(QString subPath, DiscoveryDirectoryResul
}
-void DiscoveryMainThread::singleDirectoryJobResultSlot(QLinkedList<csync_vio_file_stat_t *> result)
+void DiscoveryMainThread::singleDirectoryJobResultSlot(const QList<FileStatPointer> & result)
{
if (!_currentDiscoveryDirectoryResult) {
return; // possibly aborted
@@ -372,11 +363,9 @@ void DiscoveryMainThread::singleDirectoryJobResultSlot(QLinkedList<csync_vio_fil
qDebug() << Q_FUNC_INFO << "Have" << result.count() << "results for " << _currentDiscoveryDirectoryResult->path;
- _directoryContents.insert(_currentDiscoveryDirectoryResult->path, result);
-
_currentDiscoveryDirectoryResult->list = result;
_currentDiscoveryDirectoryResult->code = 0;
- _currentDiscoveryDirectoryResult->iterator = _currentDiscoveryDirectoryResult->list.begin();
+ _currentDiscoveryDirectoryResult->listIndex = 0;
_currentDiscoveryDirectoryResult = 0; // the sync thread owns it now
_discoveryJob->_vioMutex.lock();
@@ -414,7 +403,7 @@ void DiscoveryMainThread::abort() {
if (_singleDirJob) {
_singleDirJob->disconnect(SIGNAL(finishedWithError(int,QString)), this);
_singleDirJob->disconnect(SIGNAL(firstDirectoryPermissions(QString)), this);
- _singleDirJob->disconnect(SIGNAL(finishedWithResult(QLinkedList<csync_vio_file_stat_t*>)), this);
+ _singleDirJob->disconnect(SIGNAL(finishedWithResult(const QList<FileStatPointer> &)), this);
_singleDirJob->abort();
}
if (_currentDiscoveryDirectoryResult) {
@@ -469,9 +458,8 @@ csync_vio_file_stat_t* DiscoveryJob::remote_vio_readdir_hook (csync_vio_handle_t
DiscoveryJob *discoveryJob = static_cast<DiscoveryJob*>(userdata);
if (discoveryJob) {
DiscoveryDirectoryResult *directoryResult = static_cast<DiscoveryDirectoryResult*>(dhandle);
- if (directoryResult->iterator != directoryResult->list.end()) {
- csync_vio_file_stat_t *file_stat = *(directoryResult->iterator);
- directoryResult->iterator++;
+ if (directoryResult->listIndex < directoryResult->list.size()) {
+ csync_vio_file_stat_t *file_stat = directoryResult->list.at(directoryResult->listIndex++).data();
// Make a copy, csync_update will delete the copy
return csync_vio_file_stat_copy(file_stat);
}
@@ -487,7 +475,6 @@ void DiscoveryJob::remote_vio_closedir_hook (csync_vio_handle_t *dhandle, void
QString path = directoryResult->path;
qDebug() << Q_FUNC_INFO << discoveryJob << path;
delete directoryResult; // just deletes the struct and the iterator, the data itself is owned by the SyncEngine/DiscoveryMainThread
- emit discoveryJob->doClosedirSignal(path);
}
}
diff --git a/src/libsync/discoveryphase.h b/src/libsync/discoveryphase.h
index b1989d0..5ceb477 100644
--- a/src/libsync/discoveryphase.h
+++ b/src/libsync/discoveryphase.h
@@ -34,12 +34,36 @@ class Account;
* if the files are new, or changed.
*/
+class FileStatPointer {
+public:
+ FileStatPointer(csync_vio_file_stat_t *stat)
+ : _stat(stat)
+ { }
+ FileStatPointer(const FileStatPointer &other)
+ : _stat(csync_vio_file_stat_copy(other._stat))
+ { }
+ ~FileStatPointer() {
+ csync_vio_file_stat_destroy(_stat);
+ }
+ FileStatPointer &operator=(const FileStatPointer &other) {
+ csync_vio_file_stat_destroy(_stat);
+ _stat = csync_vio_file_stat_copy(other._stat);
+ return *this;
+ }
+ inline csync_vio_file_stat_t *data() const { return _stat; }
+ inline csync_vio_file_stat_t *operator->() const { return _stat; }
+
+private:
+ csync_vio_file_stat_t *_stat;
+};
+
struct DiscoveryDirectoryResult {
QString path;
QString msg;
int code;
- QLinkedList<csync_vio_file_stat_t*>::iterator iterator;
- QLinkedList<csync_vio_file_stat_t *> list;
+ QList<FileStatPointer> list;
+ int listIndex;
+ DiscoveryDirectoryResult() : code(0), listIndex(0) { }
};
// Run in the main thread, reporting to the DiscoveryJobMainThread object
@@ -53,14 +77,14 @@ public:
signals:
void firstDirectoryPermissions(const QString &);
void etagConcatenation(const QString &);
- void finishedWithResult(QLinkedList<csync_vio_file_stat_t*>);
+ void finishedWithResult(const QList<FileStatPointer> &);
void finishedWithError(int csyncErrnoCode, QString msg);
private slots:
void directoryListingIteratedSlot(QString,QMap<QString,QString>);
void lsJobFinishedWithoutErrorSlot();
void lsJobFinishedWithErrorSlot(QNetworkReply*);
private:
- QLinkedList<csync_vio_file_stat_t*> _results;
+ QList<FileStatPointer> _results;
QString _subPath;
QString _etagConcatenation;
AccountPtr _account;
@@ -73,11 +97,6 @@ class DiscoveryJob;
class DiscoveryMainThread : public QObject {
Q_OBJECT
- // For non-recursive and recursive
- // If it is not in this map it needs to be requested
- QMap<QString, QLinkedList<csync_vio_file_stat_t*> > _directoryContents;
-
-
QPointer<DiscoveryJob> _discoveryJob;
QPointer<DiscoverySingleDirectoryJob> _singleDirJob;
QString _pathPrefix;
@@ -88,32 +107,15 @@ public:
DiscoveryMainThread(AccountPtr account) : QObject(), _account(account), _currentDiscoveryDirectoryResult(0) {
}
- void deleteCacheEntry(QString path) {
- //qDebug() << path << _directoryContents.value(path).count();
- foreach (csync_vio_file_stat_t* stat, _directoryContents.value(path)) {
- csync_vio_file_stat_destroy(stat);
- }
- _directoryContents.remove(path);
- }
-
- ~DiscoveryMainThread() {
- // Delete the _contents_ of the list-map explicitly:
- foreach (const QLinkedList<csync_vio_file_stat_t*> & list, _directoryContents) {
- foreach (csync_vio_file_stat_t* stat, list) {
- csync_vio_file_stat_destroy(stat);
- }
- }
- }
void abort();
public slots:
// From DiscoveryJob:
void doOpendirSlot(QString url, DiscoveryDirectoryResult* );
- void doClosedirSlot(QString path);
// From Job:
- void singleDirectoryJobResultSlot(QLinkedList<csync_vio_file_stat_t*>);
+ void singleDirectoryJobResultSlot(const QList<FileStatPointer> &);
void singleDirectoryJobFinishedWithErrorSlot(int csyncErrnoCode, QString msg);
void singleDirectoryJobFirstDirectoryPermissionsSlot(QString);
signals:
@@ -175,8 +177,6 @@ signals:
// After the discovery job has been woken up again (_vioWaitCondition)
void doOpendirSignal(QString url, DiscoveryDirectoryResult*);
- // to tell the main thread to invalidate its directory data
- void doClosedirSignal(QString path);
};
}
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/owncloud-client.git
More information about the Pkg-owncloud-commits
mailing list