From 2458f07ca19ee45addc79f4dbe63935541494dad Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Mon, 23 Nov 2015 13:44:49 +0100 Subject: [PATCH] Checksums: Reuse the discovery checksum where possible --- csync/src/csync.c | 2 ++ csync/src/csync.h | 12 +++++++----- csync/src/csync_private.h | 2 +- csync/src/csync_update.c | 10 +++++++--- src/libsync/checksums.cpp | 25 +++++++++++++++++-------- src/libsync/checksums.h | 12 ++++++------ src/libsync/propagateupload.cpp | 18 +++++++++++++----- src/libsync/syncengine.cpp | 6 ++++++ 8 files changed, 59 insertions(+), 28 deletions(-) diff --git a/csync/src/csync.c b/csync/src/csync.c index 7d04708c4..a3cc77d7f 100644 --- a/csync/src/csync.c +++ b/csync/src/csync.c @@ -421,6 +421,8 @@ static int _csync_treewalk_visitor(void *obj, void *data) { trav.error_status = cur->error_status; trav.should_update_metadata = cur->should_update_metadata; trav.has_ignored_files = cur->has_ignored_files; + trav.checksum = cur->checksum; + trav.checksumTypeId = cur->checksumTypeId; if( other_node ) { csync_file_stat_t *other_stat = (csync_file_stat_t*)other_node->data; diff --git a/csync/src/csync.h b/csync/src/csync.h index 892d3e6ac..06a449756 100644 --- a/csync/src/csync.h +++ b/csync/src/csync.h @@ -262,6 +262,10 @@ struct csync_tree_walk_file_s { const char *remotePerm; char *directDownloadUrl; char *directDownloadCookies; + + const char *checksum; + uint32_t checksumTypeId; + struct { int64_t size; time_t modtime; @@ -301,11 +305,9 @@ typedef void (*csync_vio_closedir_hook) (csync_vio_handle_t *dhhandle, typedef int (*csync_vio_stat_hook) (csync_vio_handle_t *dhhandle, void *userdata); -/* compute the checksum of the given \a checksumTypeId for \a path - * and return true if it's the same as \a checksum */ -typedef bool (*csync_checksum_hook) (const char *path, - uint32_t checksumTypeId, const char *checksum, - void *userdata); +/* Compute the checksum of the given \a checksumTypeId for \a path. */ +typedef const char* (*csync_checksum_hook) ( + const char *path, uint32_t checksumTypeId, void *userdata); /** * @brief Allocate a csync context. diff --git a/csync/src/csync_private.h b/csync/src/csync_private.h index 291482d13..cfdaa2032 100644 --- a/csync/src/csync_private.h +++ b/csync/src/csync_private.h @@ -197,7 +197,7 @@ struct csync_file_stat_s { char *directDownloadCookies; char remotePerm[REMOTE_PERM_BUF_SIZE+1]; - char *checksum; + const char *checksum; uint32_t checksumTypeId; CSYNC_STATUS error_status; diff --git a/csync/src/csync_update.c b/csync/src/csync_update.c index 2f20c7837..44c97038e 100644 --- a/csync/src/csync_update.c +++ b/csync/src/csync_update.c @@ -280,12 +280,16 @@ static int _csync_detect_update(CSYNC *ctx, const char *file, || (tmp->size != 0 && fs->size != tmp->size))) { if (fs->size == tmp->size && tmp->checksumTypeId) { - bool checksumIdentical = false; if (ctx->callbacks.checksum_hook) { - checksumIdentical = ctx->callbacks.checksum_hook( - file, tmp->checksumTypeId, tmp->checksum, + st->checksum = ctx->callbacks.checksum_hook( + file, tmp->checksumTypeId, ctx->callbacks.checksum_userdata); } + bool checksumIdentical = false; + if (st->checksum) { + st->checksumTypeId = tmp->checksumTypeId; + checksumIdentical = strncmp(st->checksum, tmp->checksum, 1000) == 0; + } if (checksumIdentical) { st->instruction = CSYNC_INSTRUCTION_NONE; st->should_update_metadata = true; diff --git a/src/libsync/checksums.cpp b/src/libsync/checksums.cpp index 8fba41bdf..80fffafa6 100644 --- a/src/libsync/checksums.cpp +++ b/src/libsync/checksums.cpp @@ -160,26 +160,35 @@ CSyncChecksumHook::CSyncChecksumHook(SyncJournalDb *journal) { } -bool CSyncChecksumHook::hook(const char* path, uint32_t checksumTypeId, const char* checksum, void *this_obj) +const char* CSyncChecksumHook::hook(const char* path, uint32_t checksumTypeId, void *this_obj) { CSyncChecksumHook* checksumHook = static_cast(this_obj); - return checksumHook->check(QString::fromUtf8(path), checksumTypeId, QByteArray(checksum)); + QByteArray checksum = checksumHook->compute(QString::fromUtf8(path), checksumTypeId); + if (checksum.isNull()) { + return NULL; + } + + char* result = (char*)malloc(checksum.size() + 1); + memcpy(result, checksum.constData(), checksum.size()); + result[checksum.size()] = 0; + return result; } -bool CSyncChecksumHook::check(const QString& path, int checksumTypeId, const QByteArray& checksum) +QByteArray CSyncChecksumHook::compute(const QString& path, int checksumTypeId) { QByteArray checksumType = _journal->getChecksumType(checksumTypeId); if (checksumType.isEmpty()) { qDebug() << "Checksum type" << checksumTypeId << "not found"; - return false; + return QByteArray(); } - QByteArray newChecksum = ComputeChecksum::computeNow(path, checksumType); - if (newChecksum.isNull()) { + QByteArray checksum = ComputeChecksum::computeNow(path, checksumType); + if (checksum.isNull()) { qDebug() << "Failed to compute checksum" << checksumType << "for" << path; - return false; + return QByteArray(); } - return newChecksum == checksum; + + return checksum; } diff --git a/src/libsync/checksums.h b/src/libsync/checksums.h index bee78cd69..30a040301 100644 --- a/src/libsync/checksums.h +++ b/src/libsync/checksums.h @@ -121,15 +121,15 @@ public: explicit CSyncChecksumHook(SyncJournalDb* journal); /** - * Returns true if the checksum for \a path is the same as the one provided. + * Returns the checksum value for \a path for the given \a checksumTypeId. * - * Called from csync, where a instance of CSyncChecksumHook - * has to be set as userdata. + * Called from csync, where a instance of CSyncChecksumHook has + * to be set as userdata. + * The return value will be owned by csync. */ - static bool hook(const char* path, uint32_t checksumTypeId, const char* checksum, - void* this_obj); + static const char* hook(const char* path, uint32_t checksumTypeId, void* this_obj); - bool check(const QString& path, int checksumTypeId, const QByteArray& checksum); + QByteArray compute(const QString& path, int checksumTypeId); private: SyncJournalDb* _journal; diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp index d9ff2438d..3db9f63e3 100644 --- a/src/libsync/propagateupload.cpp +++ b/src/libsync/propagateupload.cpp @@ -216,16 +216,24 @@ void PropagateUploadFileQNAM::start() _stopWatch.start(); - // Compute the content checksum. - auto computeChecksum = new ComputeChecksum(this); + QByteArray contentChecksumType; // We currently only do content checksums for the particular .eml case // This should be done more generally in the future! if (filePath.endsWith(QLatin1String(".eml"), Qt::CaseInsensitive)) { - computeChecksum->setChecksumType("MD5"); - } else { - computeChecksum->setChecksumType(QByteArray()); + contentChecksumType = "MD5"; } + // Maybe the discovery already computed the checksum? + if (_item->_contentChecksumType == contentChecksumType + && !_item->_contentChecksum.isEmpty()) { + slotComputeTransmissionChecksum(contentChecksumType, _item->_contentChecksum); + return; + } + + // Compute the content checksum. + auto computeChecksum = new ComputeChecksum(this); + computeChecksum->setChecksumType(contentChecksumType); + connect(computeChecksum, SIGNAL(done(QByteArray,QByteArray)), SLOT(slotComputeTransmissionChecksum(QByteArray,QByteArray))); computeChecksum->start(filePath); diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 36acf77ca..cf99c6365 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -375,6 +375,12 @@ int SyncEngine::treewalkFile( TREE_WALK_FILE *file, bool remote ) item->_serverHasIgnoredFiles = (file->has_ignored_files > 0); } + // Sometimes the discovery computes checksums for local files + if (!remote && file->checksum && file->checksumTypeId) { + item->_contentChecksum = QByteArray(file->checksum); + item->_contentChecksumType = _journal->getChecksumType(file->checksumTypeId); + } + // record the seen files to be able to clean the journal later _seenFiles.insert(item->_file); if (!renameTarget.isEmpty()) {