From 4c1fdf1dee74bd3e1b3ada5ca6e7a8209f4e4659 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Tue, 7 Feb 2017 13:52:15 +0100 Subject: [PATCH] Double check usage of asserts #5429 A few are supposed to be fatal. --- src/gui/folderman.cpp | 8 +- src/gui/folderstatusmodel.cpp | 15 +-- src/gui/notificationwidget.cpp | 3 +- src/gui/socketapi.cpp | 7 +- src/libsync/account.cpp | 6 +- src/libsync/asserts.h | 52 ++++++++++ src/libsync/configfile.cpp | 19 ++-- src/libsync/creds/abstractcredentials.cpp | 4 +- src/libsync/discoveryphase.cpp | 9 +- src/libsync/owncloudpropagator.cpp | 8 +- src/libsync/ownsql.cpp | 115 +++++++++++----------- src/libsync/propagatedownload.cpp | 3 +- src/libsync/propagateremotedelete.cpp | 3 +- src/libsync/propagateremotemkdir.cpp | 3 +- src/libsync/propagateremotemove.cpp | 7 +- src/libsync/propagateupload.cpp | 5 +- src/libsync/propagateuploadng.cpp | 16 +-- src/libsync/propagateuploadv1.cpp | 4 +- src/libsync/syncengine.cpp | 22 +++-- src/libsync/syncfilestatustracker.cpp | 15 +-- src/libsync/syncjournaldb.cpp | 5 +- 21 files changed, 204 insertions(+), 125 deletions(-) create mode 100644 src/libsync/asserts.h diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index 228cb15e4..1a230a28c 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -23,6 +23,7 @@ #include "accountmanager.h" #include "filesystem.h" #include "lockwatcher.h" +#include "asserts.h" #include #ifdef Q_OS_MAC @@ -48,7 +49,7 @@ FolderMan::FolderMan(QObject *parent) : _lockWatcher(new LockWatcher), _appRestartRequired(false) { - Q_ASSERT(!_instance); + ASSERT(!_instance); _instance = this; _socketApi.reset(new SocketApi); @@ -133,12 +134,13 @@ int FolderMan::unloadAndDeleteAllFolders() delete f; cnt++; } + ASSERT(_folderMap.isEmpty()); + _lastSyncFolder = 0; _currentSyncFolder = 0; _scheduledFolders.clear(); emit scheduleQueueChanged(); - Q_ASSERT(_folderMap.count() == 0); return cnt; } @@ -462,7 +464,7 @@ void FolderMan::slotFolderSyncPaused( Folder *f, bool paused ) void FolderMan::slotFolderCanSyncChanged() { Folder *f = qobject_cast(sender()); - Q_ASSERT(f); + ASSERT(f); if (f->canSync()) { _socketApi->slotRegisterPath(f->alias()); } else { diff --git a/src/gui/folderstatusmodel.cpp b/src/gui/folderstatusmodel.cpp index dc9acce8d..ebd9ab0b7 100644 --- a/src/gui/folderstatusmodel.cpp +++ b/src/gui/folderstatusmodel.cpp @@ -16,6 +16,7 @@ #include "folderman.h" #include "accountstate.h" #include "utility.h" +#include "asserts.h" #include #include #include "folderstatusdelegate.h" @@ -480,14 +481,14 @@ QModelIndex FolderStatusModel::parent(const QModelIndex& child) const } auto pathIdx = static_cast(child.internalPointer())->_pathIdx; int i = 1; - Q_ASSERT(pathIdx.at(0) < _folders.count()); + ASSERT(pathIdx.at(0) < _folders.count()); if (pathIdx.count() == 1) { return createIndex(pathIdx.at(0), 0/*, nullptr*/); } const SubFolderInfo *info = &_folders[pathIdx.at(0)]; while (i < pathIdx.count() - 1) { - Q_ASSERT(pathIdx.at(i) < info->_subs.count()); + ASSERT(pathIdx.at(i) < info->_subs.count()); info = &info->_subs[pathIdx.at(i)]; ++i; } @@ -576,7 +577,7 @@ void FolderStatusModel::slotGatherPermissions(const QString &href, const QMapproperty(propertyPermissionMap).toMap(); job->setProperty(propertyPermissionMap, QVariant()); // avoid a detach of the map while it is modified - Q_ASSERT(!href.endsWith(QLatin1Char('/'))); // LsColXMLParser::parse removes the trailing slash before calling us. + ASSERT(!href.endsWith(QLatin1Char('/')), "LsColXMLParser::parse should remove the trailing slash before calling us."); permissionMap[href] = *it; job->setProperty(propertyPermissionMap, permissionMap); } @@ -584,7 +585,7 @@ void FolderStatusModel::slotGatherPermissions(const QString &href, const QMap(sender()); - Q_ASSERT(job); + ASSERT(job); QModelIndex idx = qvariant_cast(job->property(propertyParentIndexC)); auto parentInfo = infoForIndex(idx); if (!parentInfo) { @@ -715,7 +716,7 @@ void FolderStatusModel::slotUpdateDirectories(const QStringList &list) void FolderStatusModel::slotLscolFinishedWithError(QNetworkReply* r) { auto job = qobject_cast(sender()); - Q_ASSERT(job); + ASSERT(job); QModelIndex idx = qvariant_cast(job->property(propertyParentIndexC)); if (!idx.isValid()) { return; @@ -730,7 +731,7 @@ void FolderStatusModel::slotLscolFinishedWithError(QNetworkReply* r) if (r->error() == QNetworkReply::ContentNotFoundError) { parentInfo->_fetched = true; } else { - Q_ASSERT(!parentInfo->hasLabel()); + ASSERT(!parentInfo->hasLabel()); beginInsertRows(idx, 0, 0); parentInfo->_hasError = true; endInsertRows(); @@ -1135,7 +1136,7 @@ void FolderStatusModel::slotSyncNoPendingBigFolders() void FolderStatusModel::slotNewBigFolder() { auto f = qobject_cast(sender()); - Q_ASSERT(f); + ASSERT(f); int folderIndex = -1; for (int i = 0; i < _folders.count(); ++i) { diff --git a/src/gui/notificationwidget.cpp b/src/gui/notificationwidget.cpp index 8d09b2efd..7aa317802 100644 --- a/src/gui/notificationwidget.cpp +++ b/src/gui/notificationwidget.cpp @@ -15,6 +15,7 @@ #include "notificationwidget.h" #include "QProgressIndicator.h" #include "utility.h" +#include "asserts.h" #include @@ -33,8 +34,8 @@ void NotificationWidget::setActivity(const Activity& activity) { _myActivity = activity; - Q_ASSERT( !activity._accName.isEmpty() ); _accountName = activity._accName; + ASSERT(!_accountName.isEmpty()); // _ui._headerLabel->setText( ); _ui._subjectLabel->setVisible( !activity._subject.isEmpty() ); diff --git a/src/gui/socketapi.cpp b/src/gui/socketapi.cpp index 0ca15c8c7..ffcda712a 100644 --- a/src/gui/socketapi.cpp +++ b/src/gui/socketapi.cpp @@ -31,6 +31,7 @@ #include "accountstate.h" #include "account.h" #include "capabilities.h" +#include "asserts.h" #include #include @@ -216,7 +217,7 @@ SocketApi::~SocketApi() DEBUG << "dtor"; _localServer.close(); // All remaining sockets will be destroyed with _localServer, their parent - Q_ASSERT(_listeners.isEmpty() || _listeners.first().socket->parent() == &_localServer); + ASSERT(_listeners.isEmpty() || _listeners.first().socket->parent() == &_localServer); _listeners.clear(); } @@ -231,7 +232,7 @@ void SocketApi::slotNewConnection() connect(socket, SIGNAL(readyRead()), this, SLOT(slotReadSocket())); connect(socket, SIGNAL(disconnected()), this, SLOT(onLostConnection())); connect(socket, SIGNAL(destroyed(QObject*)), this, SLOT(slotSocketDestroyed(QObject*))); - Q_ASSERT(socket->readAll().isEmpty()); + ASSERT(socket->readAll().isEmpty()); _listeners.append(SocketListener(socket)); SocketListener &listener = _listeners.last(); @@ -259,7 +260,7 @@ void SocketApi::slotSocketDestroyed(QObject* obj) void SocketApi::slotReadSocket() { QIODevice* socket = qobject_cast(sender()); - Q_ASSERT(socket); + ASSERT(socket); SocketListener *listener = &*std::find_if(_listeners.begin(), _listeners.end(), ListenerHasSocketPred(socket)); while(socket->canReadLine()) { diff --git a/src/libsync/account.cpp b/src/libsync/account.cpp index 203d55570..8dc93aea5 100644 --- a/src/libsync/account.cpp +++ b/src/libsync/account.cpp @@ -20,6 +20,7 @@ #include "creds/abstractcredentials.h" #include "capabilities.h" #include "theme.h" +#include "asserts.h" #include #include @@ -153,8 +154,9 @@ QUrl Account::davUrl() const */ void Account::clearCookieJar() { - Q_ASSERT(qobject_cast(_am->cookieJar())); - static_cast(_am->cookieJar())->setAllCookies(QList()); + auto jar = qobject_cast(_am->cookieJar()); + ASSERT(jar); + jar->setAllCookies(QList()); emit wantsAccountSaved(this); } diff --git a/src/libsync/asserts.h b/src/libsync/asserts.h new file mode 100644 index 000000000..19b00df92 --- /dev/null +++ b/src/libsync/asserts.h @@ -0,0 +1,52 @@ +#ifndef OWNCLOUD_ASSERTS_H +#define OWNCLOUD_ASSERTS_H + +#include + +#if defined(QT_FORCE_ASSERTS) || !defined(QT_NO_DEBUG) +#define OC_ASSERT_MSG qFatal +#else +#define OC_ASSERT_MSG qWarning +#endif + +// For overloading macros by argument count +// See stackoverflow.com/questions/16683146/can-macros-be-overloaded-by-number-of-arguments +#define OC_ASSERT_CAT(A, B) A ## B +#define OC_ASSERT_SELECT(NAME, NUM) OC_ASSERT_CAT(NAME ## _, NUM) +#define OC_ASSERT_GET_COUNT(_1, _2, _3, COUNT, ...) COUNT +#define OC_ASSERT_VA_SIZE(...) OC_ASSERT_GET_COUNT(__VA_ARGS__, 3, 2, 1, 0) + +#define OC_ASSERT_OVERLOAD(NAME, ...) OC_ASSERT_SELECT(NAME, OC_ASSERT_VA_SIZE(__VA_ARGS__))(__VA_ARGS__) + +// Default assert: If the condition is false in debug builds, terminate. +// +// Prints a message on failure, even in release builds. +#define ASSERT(...) OC_ASSERT_OVERLOAD(ASSERT, __VA_ARGS__) +#define ASSERT_1(cond) \ + if (!(cond)) { \ + OC_ASSERT_MSG("ASSERT: \"%s\" in file %s, line %d", #cond, __FILE__, __LINE__); \ + } else {} +#define ASSERT_2(cond, message) \ + if (!(cond)) { \ + OC_ASSERT_MSG("ASSERT: \"%s\" in file %s, line %d with message: %s", #cond, __FILE__, __LINE__, message); \ + } else {} + +// Enforce condition to be true, even in release builds. +// +// Prints 'message' and aborts execution if 'cond' is false. +#define ENFORCE(...) OC_ASSERT_OVERLOAD(ENFORCE, __VA_ARGS__) +#define ENFORCE_1(cond) \ + if (!(cond)) { \ + qFatal("ENFORCE: \"%s\" in file %s, line %d", #cond, __FILE__, __LINE__); \ + } else {} +#define ENFORCE_2(cond, message) \ + if (!(cond)) { \ + qFatal("ENFORCE: \"%s\" in file %s, line %d with message: %s", #cond, __FILE__, __LINE__, message); \ + } else {} + +// An assert that is only present in debug builds: typically used for +// asserts that are too expensive for release mode. +// +// Q_ASSERT + +#endif diff --git a/src/libsync/configfile.cpp b/src/libsync/configfile.cpp index 8982b33b3..39f6303f6 100644 --- a/src/libsync/configfile.cpp +++ b/src/libsync/configfile.cpp @@ -17,6 +17,7 @@ #include "configfile.h" #include "theme.h" #include "utility.h" +#include "asserts.h" #include "creds/abstractcredentials.h" @@ -139,7 +140,7 @@ void ConfigFile::setOptionalDesktopNotifications(bool show) void ConfigFile::saveGeometry(QWidget *w) { #ifndef TOKEN_AUTH_ONLY - Q_ASSERT(!w->objectName().isNull()); + ASSERT(!w->objectName().isNull()); QSettings settings(configFile(), QSettings::IniFormat); settings.beginGroup(w->objectName()); settings.setValue(QLatin1String(geometryC), w->saveGeometry()); @@ -158,7 +159,7 @@ void ConfigFile::saveGeometryHeader(QHeaderView *header) { #ifndef TOKEN_AUTH_ONLY if(!header) return; - Q_ASSERT(!header->objectName().isEmpty()); + ASSERT(!header->objectName().isEmpty()); QSettings settings(configFile(), QSettings::IniFormat); settings.beginGroup(header->objectName()); @@ -171,7 +172,7 @@ void ConfigFile::restoreGeometryHeader(QHeaderView *header) { #ifndef TOKEN_AUTH_ONLY if(!header) return; - Q_ASSERT(!header->objectName().isNull()); + ASSERT(!header->objectName().isNull()); QSettings settings(configFile(), QSettings::IniFormat); settings.beginGroup(header->objectName()); @@ -230,8 +231,8 @@ QString ConfigFile::excludeFile(Scope scope) const // directories. QFileInfo fi; - if (scope != SystemScope) { - QFileInfo fi; + switch (scope) { + case UserScope: fi.setFile( configPath(), exclFile ); if( ! fi.isReadable() ) { @@ -241,12 +242,12 @@ QString ConfigFile::excludeFile(Scope scope) const fi.setFile( configPath(), exclFile ); } return fi.absoluteFilePath(); - } else if (scope != UserScope) { + case SystemScope: return ConfigFile::excludeFileFromSystem(); - } else { - Q_ASSERT(false); - return QString(); // unreachable } + + ASSERT(false); + return QString(); } QString ConfigFile::excludeFileFromSystem() diff --git a/src/libsync/creds/abstractcredentials.cpp b/src/libsync/creds/abstractcredentials.cpp index c54c2c77c..f3f38b22a 100644 --- a/src/libsync/creds/abstractcredentials.cpp +++ b/src/libsync/creds/abstractcredentials.cpp @@ -12,10 +12,10 @@ * for more details. */ - #include #include +#include "asserts.h" #include "creds/abstractcredentials.h" namespace OCC @@ -28,7 +28,7 @@ AbstractCredentials::AbstractCredentials() void AbstractCredentials::setAccount(Account *account) { - Q_ASSERT(!_account); + ENFORCE(!_account, "should only setAccount once"); _account = account; } diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp index faa7c0b07..e338db5df 100644 --- a/src/libsync/discoveryphase.cpp +++ b/src/libsync/discoveryphase.cpp @@ -13,14 +13,17 @@ */ #include "discoveryphase.h" + +#include "account.h" +#include "theme.h" +#include "asserts.h" + #include #include -#include +#include #include -#include "account.h" #include -#include "theme.h" #include diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index f8a8db892..ada918980 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -25,6 +25,7 @@ #include "configfile.h" #include "utility.h" #include "account.h" +#include "asserts.h" #include #ifdef Q_OS_WIN @@ -638,7 +639,7 @@ bool PropagateDirectory::scheduleNextJob() return true; } - Q_ASSERT(_subJobs.at(i)->_state == Running); + ASSERT(_subJobs.at(i)->_state == Running); auto paral = _subJobs.at(i)->parallelism(); if (paral == WaitForFinished) { @@ -654,6 +655,7 @@ bool PropagateDirectory::scheduleNextJob() void PropagateDirectory::slotSubJobFinished(SyncFileItem::Status status) { PropagatorJob *subJob = static_cast(sender()); + ASSERT(subJob); // Delete the job and remove it from our list of jobs. subJob->deleteLater(); @@ -663,7 +665,7 @@ void PropagateDirectory::slotSubJobFinished(SyncFileItem::Status status) _firstJob.reset(); } else { int i = _subJobs.indexOf(subJob); - Q_ASSERT(i >= 0); + ASSERT(i >= 0); _subJobs.remove(i); } @@ -759,7 +761,7 @@ void CleanupPollsJob::start() void CleanupPollsJob::slotPollFinished() { PollJob *job = qobject_cast(sender()); - Q_ASSERT(job); + ASSERT(job); if (job->_item->_status == SyncFileItem::FatalError) { emit aborted(job->_item->_errorString); deleteLater(); diff --git a/src/libsync/ownsql.cpp b/src/libsync/ownsql.cpp index 8a41b476f..e4cebd793 100644 --- a/src/libsync/ownsql.cpp +++ b/src/libsync/ownsql.cpp @@ -20,6 +20,7 @@ #include "ownsql.h" #include "utility.h" +#include "asserts.h" #define SQLITE_SLEEP_TIME_USEC 100000 #define SQLITE_REPEAT_COUNT 20 @@ -147,10 +148,8 @@ void SqlDatabase::close() { if( _db ) { SQLITE_DO(sqlite3_close(_db) ); - if (_errId != SQLITE_OK) { - qWarning() << "ERROR When closing DB" << _error; - Q_ASSERT(!"SQLite Close Error"); - } + // Fatal because reopening an unclosed db might be problematic. + ENFORCE(_errId == SQLITE_OK, "Error when closing DB"); _db = 0; } } @@ -223,11 +222,7 @@ int SqlQuery::prepare( const QString& sql, bool allow_failure ) if( _errId != SQLITE_OK ) { _error = QString::fromUtf8(sqlite3_errmsg(_db)); qWarning() << "Sqlite prepare statement error:" << _error << "in" <<_sql; - if (!allow_failure) { - qFatal("SQLITE Prepare error: %s in %s", - _error.toLocal8Bit().data(), - sql.toLocal8Bit().data()); - } + ENFORCE(allow_failure, "SQLITE Prepare error"); } } return _errId; @@ -284,61 +279,63 @@ bool SqlQuery::next() void SqlQuery::bindValue(int pos, const QVariant& value) { int res = -1; - Q_ASSERT(_stmt); - if( _stmt ) { - switch (value.type()) { - case QVariant::Int: - case QVariant::Bool: - res = sqlite3_bind_int(_stmt, pos, value.toInt()); - break; - case QVariant::Double: - res = sqlite3_bind_double(_stmt, pos, value.toDouble()); - break; - case QVariant::UInt: - case QVariant::LongLong: - res = sqlite3_bind_int64(_stmt, pos, value.toLongLong()); - break; - case QVariant::DateTime: { - const QDateTime dateTime = value.toDateTime(); - const QString str = dateTime.toString(QLatin1String("yyyy-MM-ddThh:mm:ss.zzz")); - res = sqlite3_bind_text16(_stmt, pos, str.utf16(), - str.size() * sizeof(ushort), SQLITE_TRANSIENT); - break; - } - case QVariant::Time: { - const QTime time = value.toTime(); - const QString str = time.toString(QLatin1String("hh:mm:ss.zzz")); - res = sqlite3_bind_text16(_stmt, pos, str.utf16(), - str.size() * sizeof(ushort), SQLITE_TRANSIENT); - break; - } - case QVariant::String: { - if( !value.toString().isNull() ) { - // lifetime of string == lifetime of its qvariant - const QString *str = static_cast(value.constData()); - res = sqlite3_bind_text16(_stmt, pos, str->utf16(), - (str->size()) * sizeof(QChar), SQLITE_TRANSIENT); - } else { - res = sqlite3_bind_null(_stmt, pos); - } - break; } - case QVariant::ByteArray: { - auto ba = value.toByteArray(); - res = sqlite3_bind_text(_stmt, pos, ba.constData(), ba.size(), SQLITE_TRANSIENT); - break; - } - default: { - QString str = value.toString(); - // SQLITE_TRANSIENT makes sure that sqlite buffers the data - res = sqlite3_bind_text16(_stmt, pos, str.utf16(), - (str.size()) * sizeof(QChar), SQLITE_TRANSIENT); - break; } + if (!_stmt) { + ASSERT(false); + return; + } + + switch (value.type()) { + case QVariant::Int: + case QVariant::Bool: + res = sqlite3_bind_int(_stmt, pos, value.toInt()); + break; + case QVariant::Double: + res = sqlite3_bind_double(_stmt, pos, value.toDouble()); + break; + case QVariant::UInt: + case QVariant::LongLong: + res = sqlite3_bind_int64(_stmt, pos, value.toLongLong()); + break; + case QVariant::DateTime: { + const QDateTime dateTime = value.toDateTime(); + const QString str = dateTime.toString(QLatin1String("yyyy-MM-ddThh:mm:ss.zzz")); + res = sqlite3_bind_text16(_stmt, pos, str.utf16(), + str.size() * sizeof(ushort), SQLITE_TRANSIENT); + break; + } + case QVariant::Time: { + const QTime time = value.toTime(); + const QString str = time.toString(QLatin1String("hh:mm:ss.zzz")); + res = sqlite3_bind_text16(_stmt, pos, str.utf16(), + str.size() * sizeof(ushort), SQLITE_TRANSIENT); + break; + } + case QVariant::String: { + if( !value.toString().isNull() ) { + // lifetime of string == lifetime of its qvariant + const QString *str = static_cast(value.constData()); + res = sqlite3_bind_text16(_stmt, pos, str->utf16(), + (str->size()) * sizeof(QChar), SQLITE_TRANSIENT); + } else { + res = sqlite3_bind_null(_stmt, pos); } + break; } + case QVariant::ByteArray: { + auto ba = value.toByteArray(); + res = sqlite3_bind_text(_stmt, pos, ba.constData(), ba.size(), SQLITE_TRANSIENT); + break; + } + default: { + QString str = value.toString(); + // SQLITE_TRANSIENT makes sure that sqlite buffers the data + res = sqlite3_bind_text16(_stmt, pos, str.utf16(), + (str.size()) * sizeof(QChar), SQLITE_TRANSIENT); + break; } } if (res != SQLITE_OK) { qDebug() << Q_FUNC_INFO << "ERROR" << value << res; } - Q_ASSERT( res == SQLITE_OK ); + ASSERT( res == SQLITE_OK ); } bool SqlQuery::nullValue(int index) diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index f3d0ec002..030826c0b 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -23,6 +23,7 @@ #include "filesystem.h" #include "propagatorjobs.h" #include "checksums.h" +#include "asserts.h" #include #include @@ -441,7 +442,7 @@ void PropagateDownloadFile::slotGetFinished() propagator()->_activeJobList.removeOne(this); GETFileJob *job = qobject_cast(sender()); - Q_ASSERT(job); + ASSERT(job); qDebug() << Q_FUNC_INFO << job->reply()->request().url() << "FINISHED WITH STATUS" << job->reply()->error() diff --git a/src/libsync/propagateremotedelete.cpp b/src/libsync/propagateremotedelete.cpp index 5074269b3..ff9d863eb 100644 --- a/src/libsync/propagateremotedelete.cpp +++ b/src/libsync/propagateremotedelete.cpp @@ -15,6 +15,7 @@ #include "propagateremotedelete.h" #include "owncloudpropagator_p.h" #include "account.h" +#include "asserts.h" namespace OCC { @@ -81,7 +82,7 @@ void PropagateRemoteDelete::slotDeleteJobFinished() { propagator()->_activeJobList.removeOne(this); - Q_ASSERT(_job); + ASSERT(_job); qDebug() << Q_FUNC_INFO << _job->reply()->request().url() << "FINISHED WITH STATUS" << _job->reply()->error() diff --git a/src/libsync/propagateremotemkdir.cpp b/src/libsync/propagateremotemkdir.cpp index 01fd38a52..3e4f40b56 100644 --- a/src/libsync/propagateremotemkdir.cpp +++ b/src/libsync/propagateremotemkdir.cpp @@ -17,6 +17,7 @@ #include "account.h" #include "syncjournalfilerecord.h" #include "propagateremotedelete.h" +#include "asserts.h" #include namespace OCC { @@ -70,7 +71,7 @@ void PropagateRemoteMkdir::slotMkcolJobFinished() { propagator()->_activeJobList.removeOne(this); - Q_ASSERT(_job); + ASSERT(_job); qDebug() << Q_FUNC_INFO << _job->reply()->request().url() << "FINISHED WITH STATUS" << _job->reply()->error() diff --git a/src/libsync/propagateremotemove.cpp b/src/libsync/propagateremotemove.cpp index f3f939150..11370d573 100644 --- a/src/libsync/propagateremotemove.cpp +++ b/src/libsync/propagateremotemove.cpp @@ -18,6 +18,7 @@ #include "account.h" #include "syncjournalfilerecord.h" #include "filesystem.h" +#include "asserts.h" #include #include #include @@ -123,7 +124,7 @@ void PropagateRemoteMove::slotMoveJobFinished() { propagator()->_activeJobList.removeOne(this); - Q_ASSERT(_job); + ASSERT(_job); qDebug() << Q_FUNC_INFO << _job->reply()->request().url() << "FINISHED WITH STATUS" << _job->reply()->error() @@ -207,8 +208,8 @@ bool PropagateRemoteMove::adjustSelectiveSync(SyncJournalDb *journal, const QStr return false; bool changed = false; - Q_ASSERT(!from_.endsWith(QLatin1String("/"))); - Q_ASSERT(!to_.endsWith(QLatin1String("/"))); + ASSERT(!from_.endsWith(QLatin1String("/"))); + ASSERT(!to_.endsWith(QLatin1String("/"))); QString from = from_ + QLatin1String("/"); QString to = to_ + QLatin1String("/"); diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp index b3b737a6b..2f8c544fb 100644 --- a/src/libsync/propagateupload.cpp +++ b/src/libsync/propagateupload.cpp @@ -25,6 +25,7 @@ #include "checksums.h" #include "syncengine.h" #include "propagateremotedelete.h" +#include "asserts.h" #include #include @@ -375,7 +376,7 @@ bool UploadDevice::prepareAndOpen(const QString& fileName, qint64 start, qint64 qint64 UploadDevice::writeData(const char* , qint64 ) { - Q_ASSERT(!"write to read only device"); + ASSERT(false, "write to read only device"); return 0; } @@ -486,7 +487,7 @@ void PropagateUploadFileCommon::startPollJob(const QString& path) void PropagateUploadFileCommon::slotPollFinished() { PollJob *job = qobject_cast(sender()); - Q_ASSERT(job); + ASSERT(job); propagator()->_activeJobList.removeOne(this); diff --git a/src/libsync/propagateuploadng.cpp b/src/libsync/propagateuploadng.cpp index 5d637d467..7109abb94 100644 --- a/src/libsync/propagateuploadng.cpp +++ b/src/libsync/propagateuploadng.cpp @@ -25,7 +25,7 @@ #include "syncengine.h" #include "propagateremotemove.h" #include "propagateremotedelete.h" - +#include "asserts.h" #include #include @@ -188,7 +188,7 @@ void PropagateUploadFileNG::slotPropfindFinishedWithError() void PropagateUploadFileNG::slotDeleteJobFinished() { auto job = qobject_cast(sender()); - Q_ASSERT(job); + ASSERT(job); _jobs.remove(_jobs.indexOf(job)); QNetworkReply::NetworkError err = job->reply()->error(); @@ -220,7 +220,7 @@ void PropagateUploadFileNG::slotDeleteJobFinished() void PropagateUploadFileNG::startNewUpload() { - Q_ASSERT(propagator()->_activeJobList.count(this) == 1); + ASSERT(propagator()->_activeJobList.count(this) == 1); _transferId = qrand() ^ _item->_modtime ^ (_item->_size << 16) ^ qHash(_item->_file); _sent = 0; _currentChunk = 0; @@ -270,11 +270,12 @@ void PropagateUploadFileNG::startNextChunk() return; quint64 fileSize = _item->_size; - Q_ASSERT(fileSize >= _sent); + ENFORCE(fileSize >= _sent, "Sent data exceeds file size"); + quint64 currentChunkSize = qMin(chunkSize(), fileSize - _sent); if (currentChunkSize == 0) { - Q_ASSERT(_jobs.isEmpty()); // There should be no running job anymore + ASSERT(_jobs.isEmpty()); _finished = true; // Finish with a MOVE QString destination = QDir::cleanPath(propagator()->account()->url().path() + QLatin1Char('/') @@ -343,7 +344,8 @@ void PropagateUploadFileNG::startNextChunk() void PropagateUploadFileNG::slotPutFinished() { PUTFileJob *job = qobject_cast(sender()); - Q_ASSERT(job); + ASSERT(job); + slotJobDestroyed(job); // remove it from the _jobs list qDebug() << job->reply()->request().url() << "FINISHED WITH STATUS" @@ -391,7 +393,7 @@ void PropagateUploadFileNG::slotPutFinished() return; } - Q_ASSERT(_sent <= _item->_size); + ENFORCE(_sent <= _item->_size, "can't send more than size"); bool finished = _sent == _item->_size; // Check if the file still exists diff --git a/src/libsync/propagateuploadv1.cpp b/src/libsync/propagateuploadv1.cpp index 081a0dc4f..311bc500c 100644 --- a/src/libsync/propagateuploadv1.cpp +++ b/src/libsync/propagateuploadv1.cpp @@ -25,6 +25,7 @@ #include "checksums.h" #include "syncengine.h" #include "propagateremotedelete.h" +#include "asserts.h" #include #include @@ -171,7 +172,8 @@ void PropagateUploadFileV1::startNextChunk() void PropagateUploadFileV1::slotPutFinished() { PUTFileJob *job = qobject_cast(sender()); - Q_ASSERT(job); + ASSERT(job); + slotJobDestroyed(job); // remove it from the _jobs list qDebug() << Q_FUNC_INFO << job->reply()->request().url() << "FINISHED WITH STATUS" diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 6da819fed..57a00763f 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -24,6 +24,7 @@ #include "csync_private.h" #include "filesystem.h" #include "propagateremotedelete.h" +#include "asserts.h" #ifdef Q_OS_WIN #include @@ -83,7 +84,7 @@ SyncEngine::SyncEngine(AccountPtr account, const QString& localPath, qRegisterMetaType("SyncFileItem::Direction"); // Everything in the SyncEngine expects a trailing slash for the localPath. - Q_ASSERT(localPath.endsWith(QLatin1Char('/'))); + ASSERT(localPath.endsWith(QLatin1Char('/'))); csync_create(&_csync_ctx, localPath.toUtf8().data()); @@ -353,7 +354,7 @@ int SyncEngine::treewalkFile( TREE_WALK_FILE *file, bool remote ) QTextCodec::ConverterState utf8State; static QTextCodec *codec = QTextCodec::codecForName("UTF-8"); - Q_ASSERT(codec); + ASSERT(codec); QString fileUtf8 = codec->toUnicode(file->path, qstrlen(file->path), &utf8State); QString renameTarget; QString key = fileUtf8; @@ -389,9 +390,11 @@ int SyncEngine::treewalkFile( TREE_WALK_FILE *file, bool remote ) item->_modtime = file->modtime; } else { if (instruction != CSYNC_INSTRUCTION_NONE) { - qDebug() << "ERROR: Instruction" << item->_instruction << "vs" << instruction << "for" << fileUtf8; - Q_ASSERT(!"Instructions are both unequal NONE"); - return -1; + qWarning() << "ERROR: Instruction" << item->_instruction << "vs" << instruction << "for" << fileUtf8; + ASSERT(false); + // Set instruction to NONE for safety. + file->instruction = item->_instruction = instruction = CSYNC_INSTRUCTION_NONE; + return -1; // should lead to treewalk error } } @@ -499,7 +502,7 @@ int SyncEngine::treewalkFile( TREE_WALK_FILE *file, bool remote ) item->_status = SyncFileItem::SoftError; break; default: - Q_ASSERT("Non handled error-status"); + ASSERT(false, "Non handled error-status"); /* No error string */ } @@ -709,8 +712,11 @@ void SyncEngine::startSync() } } - Q_ASSERT(!s_anySyncRunning); - Q_ASSERT(!_syncRunning); + if (s_anySyncRunning || _syncRunning) { + ASSERT(false); + return; + } + s_anySyncRunning = true; _syncRunning = true; _anotherSyncNeeded = NoFollowUpSync; diff --git a/src/libsync/syncfilestatustracker.cpp b/src/libsync/syncfilestatustracker.cpp index 4179e3bf6..df54c8cdf 100644 --- a/src/libsync/syncfilestatustracker.cpp +++ b/src/libsync/syncfilestatustracker.cpp @@ -17,6 +17,7 @@ #include "syncengine.h" #include "syncjournaldb.h" #include "syncjournalfilerecord.h" +#include "asserts.h" namespace OCC { @@ -86,7 +87,7 @@ SyncFileStatusTracker::SyncFileStatusTracker(SyncEngine *syncEngine) SyncFileStatus SyncFileStatusTracker::fileStatus(const QString& relativePath) { - Q_ASSERT(!relativePath.endsWith(QLatin1Char('/'))); + ASSERT(!relativePath.endsWith(QLatin1Char('/'))); if (relativePath.isEmpty()) { // This is the root sync folder, it doesn't have an entry in the database and won't be walked by csync, so resolve manually. @@ -121,8 +122,8 @@ SyncFileStatus SyncFileStatusTracker::fileStatus(const QString& relativePath) void SyncFileStatusTracker::slotPathTouched(const QString& fileName) { QString folderPath = _syncEngine->localPath(); - Q_ASSERT(fileName.startsWith(folderPath)); + ASSERT(fileName.startsWith(folderPath)); QString localPath = fileName.mid(folderPath.size()); _dirtyPaths.insert(localPath); @@ -141,7 +142,7 @@ void SyncFileStatusTracker::incSyncCountAndEmitStatusChanged(const QString &rela // We passed from OK to SYNC, increment the parent to keep it marked as // SYNC while we propagate ourselves and our own children. - Q_ASSERT(!relativePath.endsWith('/')); + ASSERT(!relativePath.endsWith('/')); int lastSlashIndex = relativePath.lastIndexOf('/'); if (lastSlashIndex != -1) incSyncCountAndEmitStatusChanged(relativePath.left(lastSlashIndex), UnknownShared); @@ -163,7 +164,7 @@ void SyncFileStatusTracker::decSyncCountAndEmitStatusChanged(const QString &rela emit fileStatusChanged(getSystemDestination(relativePath), status); // We passed from SYNC to OK, decrement our parent. - Q_ASSERT(!relativePath.endsWith('/')); + ASSERT(!relativePath.endsWith('/')); int lastSlashIndex = relativePath.lastIndexOf('/'); if (lastSlashIndex != -1) decSyncCountAndEmitStatusChanged(relativePath.left(lastSlashIndex), UnknownShared); @@ -174,7 +175,7 @@ void SyncFileStatusTracker::decSyncCountAndEmitStatusChanged(const QString &rela void SyncFileStatusTracker::slotAboutToPropagate(SyncFileItemVector& items) { - Q_ASSERT(_syncCount.isEmpty()); + ASSERT(_syncCount.isEmpty()); std::map oldProblems; std::swap(_syncProblems, oldProblems); @@ -277,8 +278,8 @@ SyncFileStatus SyncFileStatusTracker::resolveSyncAndErrorStatus(const QString &r status.set(problemStatus); } - // The shared status needs to have been fetched from a SyncFileItem or the DB at this point. - Q_ASSERT(sharedFlag != UnknownShared); + ASSERT(sharedFlag != UnknownShared, + "The shared status needs to have been fetched from a SyncFileItem or the DB at this point."); if (sharedFlag == Shared) status.setSharedWithMe(true); diff --git a/src/libsync/syncjournaldb.cpp b/src/libsync/syncjournaldb.cpp index ae0cb8e26..7ee83db46 100644 --- a/src/libsync/syncjournaldb.cpp +++ b/src/libsync/syncjournaldb.cpp @@ -27,6 +27,7 @@ #include "utility.h" #include "version.h" #include "filesystem.h" +#include "asserts.h" #include "../../csync/src/std/c_jhash.h" @@ -178,7 +179,7 @@ bool SyncJournalDb::sqlFail( const QString& log, const SqlQuery& query ) { commitTransaction(); qWarning() << "SQL Error" << log << query.error(); - Q_ASSERT(!"SQL ERROR"); + ASSERT(false); _db.close(); return false; } @@ -1607,7 +1608,7 @@ void SyncJournalDb::setPollInfo(const SyncJournalDb::PollInfo& info) QStringList SyncJournalDb::getSelectiveSyncList(SyncJournalDb::SelectiveSyncListType type, bool *ok ) { QStringList result; - Q_ASSERT(ok); + ASSERT(ok); QMutexLocker locker(&_mutex); if( !checkConnect() ) { -- 2.30.2