csync: Simplify csync_walk_local_tree/csync_walk_remote_tree

Small refactoring

6 files changed, 20 insertions, 99 deletions

diff --git a/src/csync/csync.cpp b/src/csync/csync.cpp
index dce8169bc..825e28ef5 100644
--- a/src/csync/csync.cpp
+++ b/src/csync/csync.cpp
@@ -178,16 +178,9 @@ int csync_reconcile(CSYNC *ctx) {
 /*
  * local visitor which calls the user visitor with repacked stat info.
  */
-static int _csync_treewalk_visitor(csync_file_stat_t *cur, CSYNC * ctx) {
-    int rc = 0;
-    csync_treewalk_visit_func *visitor   = NULL;
-    _csync_treewalk_context *twctx = NULL;
+static int _csync_treewalk_visitor(csync_file_stat_t *cur, CSYNC * ctx, const csync_treewalk_visit_func &visitor) {
     csync_s::FileMap *other_tree = nullptr;
 
-    if (ctx == NULL) {
-      return -1;
-    }
-
     /* we need the opposite tree! */
     switch (ctx->current) {
     case LOCAL_REPLICA:
@@ -220,80 +213,43 @@ static int _csync_treewalk_visitor(csync_file_stat_t *cur, CSYNC * ctx) {
 
     ctx->status_code = CSYNC_STATUS_OK;
 
-    twctx = (_csync_treewalk_context*) ctx->callbacks.userdata;
-    if (twctx == NULL) {
-      ctx->status_code = CSYNC_STATUS_PARAM_ERROR;
-      return -1;
-    }
-
-    if (twctx->instruction_filter > 0 &&
-        !(twctx->instruction_filter & cur->instruction) ) {
-        return 0;
-    }
-
-    visitor = (csync_treewalk_visit_func*)(twctx->user_visitor);
-    if (visitor != NULL) {
-      rc = (*visitor)(cur, other, twctx->userdata);
-
-      return rc;
-    }
-    ctx->status_code = CSYNC_STATUS_PARAM_ERROR;
-    return -1;
+    Q_ASSERT(visitor);
+    return visitor(cur, other);
 }
 
 /*
  * treewalk function, called from its wrappers below.
- *
- * it encapsulates the user visitor function, the filter and the userdata
- * into a treewalk_context structure and calls the rb treewalk function,
- * which calls the local _csync_treewalk_visitor in this module.
- * The user visitor is called from there.
  */
-static int _csync_walk_tree(CSYNC *ctx, csync_s::FileMap *tree, csync_treewalk_visit_func *visitor, int filter)
+static int _csync_walk_tree(CSYNC *ctx, csync_s::FileMap &tree, const csync_treewalk_visit_func &visitor)
 {
-    _csync_treewalk_context tw_ctx;
-    int rc = 0;
-
-    tw_ctx.userdata = ctx->callbacks.userdata;
-    tw_ctx.user_visitor = visitor;
-    tw_ctx.instruction_filter = filter;
-
-    ctx->callbacks.userdata = &tw_ctx;
-
-    for (auto &pair : *tree) {
-        if (_csync_treewalk_visitor(pair.second.get(), ctx) < 0) {
-          rc = -1;
-          break;
+    for (auto &pair : tree) {
+        if (_csync_treewalk_visitor(pair.second.get(), ctx, visitor) < 0) {
+            if( ctx->status_code == CSYNC_STATUS_OK )
+                ctx->status_code = csync_errno_to_status(errno, CSYNC_STATUS_TREE_ERROR);
+            return -1;
         }
     }
-
-    if( rc < 0 ) {
-      if( ctx->status_code == CSYNC_STATUS_OK )
-          ctx->status_code = csync_errno_to_status(errno, CSYNC_STATUS_TREE_ERROR);
-    }
-    ctx->callbacks.userdata = tw_ctx.userdata;
-
-    return rc;
+    return 0;
 }
 
 /*
  * wrapper function for treewalk on the remote tree
  */
-int csync_walk_remote_tree(CSYNC *ctx,  csync_treewalk_visit_func *visitor, int filter)
+int csync_walk_remote_tree(CSYNC *ctx, const csync_treewalk_visit_func &visitor)
 {
     ctx->status_code = CSYNC_STATUS_OK;
     ctx->current = REMOTE_REPLICA;
-    return _csync_walk_tree(ctx, &ctx->remote.files, visitor, filter);
+    return _csync_walk_tree(ctx, ctx->remote.files, visitor);
 }
 
 /*
  * wrapper function for treewalk on the local tree
  */
-int csync_walk_local_tree(CSYNC *ctx, csync_treewalk_visit_func *visitor, int filter)
+int csync_walk_local_tree(CSYNC *ctx, const csync_treewalk_visit_func &visitor)
 {
     ctx->status_code = CSYNC_STATUS_OK;
     ctx->current = LOCAL_REPLICA;
-    return _csync_walk_tree(ctx, &ctx->local.files, visitor, filter);
+    return _csync_walk_tree(ctx, ctx->local.files, visitor);
 }
 
 int csync_s::reinitialize() {
diff --git a/src/csync/csync.h b/src/csync/csync.h
index 23c57710c..94a7ce088 100644
--- a/src/csync/csync.h
+++ b/src/csync/csync.h
@@ -40,6 +40,7 @@
 #include <stdint.h>
 #include <sys/types.h>
 #include <config_csync.h>
+#include <functional>
 #include <memory>
 #include <QByteArray>
 #include "common/remotepermissions.h"
@@ -306,29 +307,27 @@ CSYNC_STATUS OCSYNC_EXPORT csync_get_status(CSYNC *ctx);
 /* Used for special modes or debugging */
 int OCSYNC_EXPORT csync_set_status(CSYNC *ctx, int status);
 
-typedef int csync_treewalk_visit_func(csync_file_stat_t *cur, csync_file_stat_t *other, void*);
+using csync_treewalk_visit_func = std::function<int(csync_file_stat_t *cur, csync_file_stat_t *other)>;
 
 /**
  * @brief Walk the local file tree and call a visitor function for each file.
  *
  * @param ctx           The csync context.
  * @param visitor       A callback function to handle the file info.
- * @param filter        A filter, built from or'ed csync_instructions_e
  *
  * @return              0 on success, less than 0 if an error occurred.
  */
-int OCSYNC_EXPORT csync_walk_local_tree(CSYNC *ctx, csync_treewalk_visit_func *visitor, int filter);
+int OCSYNC_EXPORT csync_walk_local_tree(CSYNC *ctx, const csync_treewalk_visit_func &visitor);
 
 /**
  * @brief Walk the remote file tree and call a visitor function for each file.
  *
  * @param ctx           The csync context.
  * @param visitor       A callback function to handle the file info.
- * @param filter        A filter, built from and'ed csync_instructions_e
  *
  * @return              0 on success, less than 0 if an error occurred.
  */
-int OCSYNC_EXPORT csync_walk_remote_tree(CSYNC *ctx, csync_treewalk_visit_func *visitor, int filter);
+int OCSYNC_EXPORT csync_walk_remote_tree(CSYNC *ctx, const csync_treewalk_visit_func &visitor);
 
 /**
  * @brief Get the csync status string.
diff --git a/src/csync/csync_private.h b/src/csync/csync_private.h
index 8f4d41c7f..1f9204b53 100644
--- a/src/csync/csync_private.h
+++ b/src/csync/csync_private.h
@@ -219,17 +219,6 @@ struct OCSYNC_EXPORT csync_s {
   csync_s &operator=(const csync_s &) = delete;
 };
 
-/*
- * context for the treewalk function
- */
-struct _csync_treewalk_context_s
-{
-    csync_treewalk_visit_func *user_visitor;
-    int instruction_filter;
-    void *userdata;
-};
-typedef struct _csync_treewalk_context_s _csync_treewalk_context;
-
 void set_errno_from_http_errcode( int err );
 
 /**
diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp
index cd0bf04af..bf4a7ce6b 100644
--- a/src/libsync/syncengine.cpp
+++ b/src/libsync/syncengine.cpp
@@ -385,16 +385,6 @@ void SyncEngine::conflictRecordMaintenance()
     }
 }
 
-int SyncEngine::treewalkLocal(csync_file_stat_t *file, csync_file_stat_t *other, void *data)
-{
-    return static_cast<SyncEngine *>(data)->treewalkFile(file, other, false);
-}
-
-int SyncEngine::treewalkRemote(csync_file_stat_t *file, csync_file_stat_t *other, void *data)
-{
-    return static_cast<SyncEngine *>(data)->treewalkFile(file, other, true);
-}
-
 /**
  * The main function in the post-reconcile phase.
  *
@@ -1019,11 +1009,11 @@ void SyncEngine::slotDiscoveryJobFinished(int discoveryResult)
     _temporarilyUnavailablePaths.clear();
     _renamedFolders.clear();
 
-    if (csync_walk_local_tree(_csync_ctx.data(), &treewalkLocal, 0) < 0) {
+    if (csync_walk_local_tree(_csync_ctx.data(), [this](csync_file_stat_t *f, csync_file_stat_t *o) { return treewalkFile(f, o, false); } ) < 0) {
         qCWarning(lcEngine) << "Error in local treewalk.";
         walkOk = false;
     }
-    if (walkOk && csync_walk_remote_tree(_csync_ctx.data(), &treewalkRemote, 0) < 0) {
+    if (walkOk && csync_walk_remote_tree(_csync_ctx.data(), [this](csync_file_stat_t *f, csync_file_stat_t *o) { return treewalkFile(f, o, true); } ) < 0) {
         qCWarning(lcEngine) << "Error in remote treewalk.";
     }
 
diff --git a/src/libsync/syncengine.h b/src/libsync/syncengine.h
index aba6164de..903c75598 100644
--- a/src/libsync/syncengine.h
+++ b/src/libsync/syncengine.h
@@ -203,8 +203,6 @@ private:
 
     QString journalDbFilePath() const;
 
-    static int treewalkLocal(csync_file_stat_t *file, csync_file_stat_t *other, void *);
-    static int treewalkRemote(csync_file_stat_t *file, csync_file_stat_t *other, void *);
     int treewalkFile(csync_file_stat_t *file, csync_file_stat_t *other, bool);
     bool checkErrorBlacklisting(SyncFileItem &item);
 
diff --git a/test/csync/csync_tests/check_csync_update.cpp b/test/csync/csync_tests/check_csync_update.cpp
index 504ea2096..b4eca074d 100644
--- a/test/csync/csync_tests/check_csync_update.cpp
+++ b/test/csync/csync_tests/check_csync_update.cpp
@@ -325,16 +325,6 @@ static void check_csync_detect_update_db_new(void **state)
     csync_set_status(csync, 0xFFFF);
 }
 
-static void check_csync_detect_update_null(void **state)
-{
-    CSYNC *csync = (CSYNC*)*state;
-    std::unique_ptr<csync_file_stat_t> fs;
-    int rc;
-
-    rc = _csync_detect_update(csync, NULL);
-    assert_int_equal(rc, -1);
-}
-
 static void check_csync_ftw(void **state)
 {
     CSYNC *csync = (CSYNC*)*state;
@@ -370,7 +360,6 @@ int torture_run_tests(void)
         cmocka_unit_test_setup_teardown(check_csync_detect_update_db_eval, setup, teardown),
         cmocka_unit_test_setup_teardown(check_csync_detect_update_db_rename, setup, teardown),
         cmocka_unit_test_setup_teardown(check_csync_detect_update_db_new, setup, teardown_rm),
-        cmocka_unit_test_setup_teardown(check_csync_detect_update_null, setup, teardown_rm),
 
         cmocka_unit_test_setup_teardown(check_csync_ftw, setup_ftw, teardown_rm),
         cmocka_unit_test_setup_teardown(check_csync_ftw_empty_uri, setup_ftw, teardown_rm),