From: Jocelyn Turcotte Date: Thu, 17 Aug 2017 08:06:14 +0000 (+0200) Subject: Make csync_file_stat_t public and partly convert to C++ X-Git-Tag: archive/raspbian/3.16.7-1_deb13u1+rpi1~1^2~701^2~136 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=61d949730b7de4d3feb6b08c2f4bdd71d36c8aea;p=nextcloud-desktop.git Make csync_file_stat_t public and partly convert to C++ This is the first commit trying to unify csync_file_stat_s, csync_vio_file_stat_s and csync_tree_walk_file_s. Use QByteArray and unique_ptr already since I'm not used to track memory allocations and this will make the transition easier. Issue #1817 --- diff --git a/src/csync/csync.cpp b/src/csync/csync.cpp index 6ef3d0d1d..eefc8904e 100644 --- a/src/csync/csync.cpp +++ b/src/csync/csync.cpp @@ -356,15 +356,14 @@ static int _csync_treewalk_visitor(void *obj, void *data) { trav.path = cur->path; trav.size = cur->size; trav.modtime = cur->modtime; - trav.mode = cur->mode; trav.type = cur->type; trav.instruction = cur->instruction; - trav.rename_path = cur->destpath; + trav.rename_path = cur->rename_path; trav.etag = cur->etag; trav.file_id = cur->file_id; trav.remotePerm = cur->remotePerm; - trav.directDownloadUrl = cur->directDownloadUrl; - trav.directDownloadCookies = cur->directDownloadCookies; + trav.directDownloadUrl = cur->directDownloadUrl.data(); + trav.directDownloadCookies = cur->directDownloadCookies.data(); trav.inode = cur->inode; trav.error_status = cur->error_status; @@ -389,8 +388,7 @@ static int _csync_treewalk_visitor(void *obj, void *data) { rc = (*visitor)(&trav, twctx->userdata); cur->instruction = trav.instruction; if (trav.etag != cur->etag) { // FIXME It would be nice to have this documented - SAFE_FREE(cur->etag); - cur->etag = c_strdup(trav.etag); + cur->etag = trav.etag; } return rc; @@ -480,7 +478,7 @@ static void _tree_destructor(void *data) { csync_file_stat_t *freedata = NULL; freedata = (csync_file_stat_t *) data; - csync_file_stat_free(freedata); + delete freedata; } /* reset all the list to empty. @@ -637,15 +635,3 @@ int csync_abort_requested(CSYNC *ctx) return (1 == 0); } } - -void csync_file_stat_free(csync_file_stat_t *st) -{ - if (st) { - SAFE_FREE(st->directDownloadUrl); - SAFE_FREE(st->directDownloadCookies); - SAFE_FREE(st->etag); - SAFE_FREE(st->destpath); - SAFE_FREE(st->checksumHeader); - SAFE_FREE(st); - } -} diff --git a/src/csync/csync.h b/src/csync/csync.h index 73eb5790a..4eea748ef 100644 --- a/src/csync/csync.h +++ b/src/csync/csync.h @@ -39,6 +39,7 @@ #include #include #include +#include enum csync_status_codes_e { CSYNC_STATUS_OK = 0, @@ -207,7 +208,6 @@ struct csync_vio_file_stat_s { time_t atime; time_t mtime; time_t ctime; - int64_t size; mode_t mode; @@ -226,6 +226,48 @@ struct csync_vio_file_stat_s { char *checksumHeader; }; +typedef struct csync_file_stat_s csync_file_stat_t; + +struct csync_file_stat_s { + uint64_t phash; + time_t modtime; + int64_t size; + uint64_t inode; + enum csync_ftw_type_e type : 4; + bool child_modified : 1; + bool has_ignored_files : 1; /* specify that a directory, or child directory contains ignored files */ + + QByteArray path; + QByteArray rename_path; + QByteArray etag; + QByteArray file_id; + QByteArray directDownloadUrl; + QByteArray directDownloadCookies; + QByteArray remotePerm; + + // In the local tree, this can hold a checksum and its type if it is + // computed during discovery for some reason. + // In the remote tree, this will have the server checksum, if available. + // In both cases, the format is "SHA1:baff". + QByteArray checksumHeader; + + CSYNC_STATUS error_status; + + enum csync_instructions_e instruction; /* u32 */ + + csync_file_stat_s() + : phash(0) + , modtime(0) + , size(0) + , inode(0) + , type(CSYNC_FTW_TYPE_SKIP) + , child_modified(false) + , has_ignored_files(false) + , error_status(CSYNC_STATUS_OK) + , instruction(CSYNC_INSTRUCTION_NONE) + { } +}; + csync_vio_file_stat_t OCSYNC_EXPORT *csync_vio_file_stat_new(void); csync_vio_file_stat_t OCSYNC_EXPORT *csync_vio_file_stat_copy(csync_vio_file_stat_t *file_stat); @@ -304,8 +346,8 @@ typedef int (*csync_vio_stat_hook) (csync_vio_handle_t *dhhandle, void *userdata); /* Compute the checksum of the given \a checksumTypeId for \a path. */ -typedef const char *(*csync_checksum_hook)( - const char *path, const char *otherChecksumHeader, void *userdata); +typedef QByteArray (*csync_checksum_hook)( + const QByteArray &path, const QByteArray &otherChecksumHeader, void *userdata); /** * @brief Allocate a csync context. diff --git a/src/csync/csync_private.h b/src/csync/csync_private.h index 3d1632e41..aa1ead2ab 100644 --- a/src/csync/csync_private.h +++ b/src/csync/csync_private.h @@ -64,8 +64,6 @@ enum csync_replica_e { REMOTE_REPLICA }; -typedef struct csync_file_stat_s csync_file_stat_t; - /** * @brief csync public structure */ @@ -152,49 +150,6 @@ struct csync_s { bool ignore_hidden_files; }; - -#ifdef _MSC_VER -#pragma pack(1) -#endif -struct csync_file_stat_s { - uint64_t phash; /* u64 */ - time_t modtime; /* u64 */ - int64_t size; /* u64 */ - size_t pathlen; /* u64 */ - uint64_t inode; /* u64 */ - mode_t mode; /* u32 */ - enum csync_ftw_type_e type : 4; - unsigned int child_modified : 1; - unsigned int has_ignored_files : 1; /* specify that a directory, or child directory contains ignored files */ - - char *destpath; /* for renames */ - const char *etag; - char file_id[FILE_ID_BUF_SIZE+1]; /* the ownCloud file id is fixed width in ownCloud. */ - char *directDownloadUrl; - char *directDownloadCookies; - char remotePerm[REMOTE_PERM_BUF_SIZE+1]; - - // In the local tree, this can hold a checksum and its type if it is - // computed during discovery for some reason. - // In the remote tree, this will have the server checksum, if available. - // In both cases, the format is "SHA1:baff". - const char *checksumHeader; - - CSYNC_STATUS error_status; - - enum csync_instructions_e instruction; /* u32 */ - char path[1]; /* u8 */ -} -#if !defined(__SUNPRO_C) && !defined(_MSC_VER) -__attribute__ ((packed)) -#endif -#ifdef _MSC_VER -#pragma pack() -#endif -; - -OCSYNC_EXPORT void csync_file_stat_free(csync_file_stat_t *st); - /* * context for the treewalk function */ diff --git a/src/csync/csync_reconcile.cpp b/src/csync/csync_reconcile.cpp index 3b02398fc..760c5ca46 100644 --- a/src/csync/csync_reconcile.cpp +++ b/src/csync/csync_reconcile.cpp @@ -110,7 +110,7 @@ static bool _csync_is_collision_safe_hash(const char *checksum_header) static int _csync_merge_algorithm_visitor(void *obj, void *data) { csync_file_stat_t *cur = NULL; csync_file_stat_t *other = NULL; - csync_file_stat_t *tmp = NULL; + std::unique_ptr tmp; uint64_t h = 0; int len = 0; @@ -138,7 +138,7 @@ static int _csync_merge_algorithm_visitor(void *obj, void *data) { if (!node) { /* Check the renamed path as well. */ char *renamed_path = csync_rename_adjust_path(ctx, cur->path); - if (!c_streq(renamed_path, cur->path)) { + if (renamed_path != cur->path) { len = strlen( renamed_path ); h = c_jhash64((uint8_t *) renamed_path, len, 0); node = c_rbtree_find(tree, &h); @@ -147,7 +147,7 @@ static int _csync_merge_algorithm_visitor(void *obj, void *data) { } if (!node) { /* Check if it is ignored */ - node = _csync_check_ignored(tree, cur->path, cur->pathlen); + node = _csync_check_ignored(tree, cur->path, cur->path.size()); /* If it is ignored, other->instruction will be IGNORE so this one will also be ignored */ } @@ -181,7 +181,7 @@ static int _csync_merge_algorithm_visitor(void *obj, void *data) { } else if( ctx->current == REMOTE_REPLICA ) { tmp = csync_statedb_get_stat_by_file_id(ctx, cur->file_id); CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "Finding opposite temp through file ID %s: %s", - cur->file_id, tmp ? "true":"false"); + cur->file_id.constData(), tmp ? "true":"false"); } else { CSYNC_LOG(CSYNC_LOG_PRIORITY_DEBUG, "Unknown replica..."); } @@ -189,16 +189,16 @@ static int _csync_merge_algorithm_visitor(void *obj, void *data) { if( tmp ) { len = strlen( tmp->path ); if( len > 0 ) { - h = c_jhash64((uint8_t *) tmp->path, len, 0); + h = c_jhash64((uint8_t *) tmp->path.constData(), len, 0); /* First, check that the file is NOT in our tree (another file with the same name was added) */ node = c_rbtree_find(ctx->current == REMOTE_REPLICA ? ctx->remote.tree : ctx->local.tree, &h); if (node) { - CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "Origin found in our tree : %s", tmp->path); + CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "Origin found in our tree : %s", tmp->path.constData()); } else { /* Find the temporar file in the other tree. */ node = c_rbtree_find(tree, &h); CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "PHash of temporary opposite (%s): %" PRIu64 " %s", - tmp->path , h, node ? "found": "not found" ); + tmp->path.constData() , h, node ? "found": "not found" ); if (node) { other = (csync_file_stat_t*)node->data; } else { @@ -216,18 +216,18 @@ static int _csync_merge_algorithm_visitor(void *obj, void *data) { || other->instruction == CSYNC_INSTRUCTION_UPDATE_METADATA || cur->type == CSYNC_FTW_TYPE_DIR) { other->instruction = CSYNC_INSTRUCTION_RENAME; - other->destpath = c_strdup( cur->path ); - if( !c_streq(cur->file_id, "") ) { - csync_vio_set_file_id( other->file_id, cur->file_id ); + other->rename_path = cur->path; + if( !cur->file_id.isEmpty() ) { + other->file_id = cur->file_id; } other->inode = cur->inode; cur->instruction = CSYNC_INSTRUCTION_NONE; } else if (other->instruction == CSYNC_INSTRUCTION_REMOVE) { other->instruction = CSYNC_INSTRUCTION_RENAME; - other->destpath = c_strdup( cur->path ); + other->rename_path = cur->path; - if( !c_streq(cur->file_id, "") ) { - csync_vio_set_file_id( other->file_id, cur->file_id ); + if( !cur->file_id.isEmpty() ) { + other->file_id = cur->file_id; } other->inode = cur->inode; cur->instruction = CSYNC_INSTRUCTION_NONE; @@ -239,7 +239,6 @@ static int _csync_merge_algorithm_visitor(void *obj, void *data) { cur->instruction = CSYNC_INSTRUCTION_NONE; other->instruction = CSYNC_INSTRUCTION_SYNC; } - csync_file_stat_free(tmp); } break; @@ -270,7 +269,7 @@ static int _csync_merge_algorithm_visitor(void *obj, void *data) { case CSYNC_INSTRUCTION_NEW: // This operation is usually a no-op and will by default return false if (csync_file_locked_or_open(ctx->local.uri, cur->path)) { - CSYNC_LOG(CSYNC_LOG_PRIORITY_DEBUG, "[Reconciler] IGNORING file %s/%s since it is locked / open", ctx->local.uri, cur->path); + CSYNC_LOG(CSYNC_LOG_PRIORITY_DEBUG, "[Reconciler] IGNORING file %s/%s since it is locked / open", ctx->local.uri, cur->path.constData()); cur->instruction = CSYNC_INSTRUCTION_ERROR; if (cur->error_status == CSYNC_STATUS_OK) // don't overwrite error cur->error_status = CYSNC_STATUS_FILE_LOCKED_OR_OPEN; @@ -361,7 +360,7 @@ static int _csync_merge_algorithm_visitor(void *obj, void *data) { "%-30s %s dir: %s", csync_instruction_str(cur->instruction), repo, - cur->path); + cur->path.constData()); } else { @@ -369,7 +368,7 @@ static int _csync_merge_algorithm_visitor(void *obj, void *data) { "%-30s %s file: %s", csync_instruction_str(cur->instruction), repo, - cur->path); + cur->path.constData()); } } else @@ -380,7 +379,7 @@ static int _csync_merge_algorithm_visitor(void *obj, void *data) { "%-30s %s dir: %s", csync_instruction_str(cur->instruction), repo, - cur->path); + cur->path.constData()); } else { @@ -388,7 +387,7 @@ static int _csync_merge_algorithm_visitor(void *obj, void *data) { "%-30s %s file: %s", csync_instruction_str(cur->instruction), repo, - cur->path); + cur->path.constData()); } } diff --git a/src/csync/csync_rename.cpp b/src/csync/csync_rename.cpp index 79bb78e34..1408e6783 100644 --- a/src/csync/csync_rename.cpp +++ b/src/csync/csync_rename.cpp @@ -43,14 +43,6 @@ struct csync_rename_s { std::map folder_renamed_to; // map from->to std::map folder_renamed_from; // map to->from - - struct renameop { - csync_file_stat_t *st; - bool operator<(const renameop &other) const { - return strlen(st->destpath) < strlen(other.st->destpath); - } - }; - std::vector todo; }; void csync_rename_destroy(CSYNC* ctx) diff --git a/src/csync/csync_statedb.cpp b/src/csync/csync_statedb.cpp index 63eb813e7..4c4eb9828 100644 --- a/src/csync/csync_statedb.cpp +++ b/src/csync/csync_statedb.cpp @@ -25,6 +25,7 @@ #define _GNU_SOURCE #endif +#include #include #include #include @@ -228,78 +229,46 @@ int csync_statedb_close(CSYNC *ctx) { return rc; } -#define METADATA_QUERY \ - "phash, pathlen, path, inode, uid, gid, mode, modtime, type, md5, fileid, remotePerm, " \ - "filesize, ignoredChildrenRemote, " \ - "contentchecksumtype.name || ':' || contentChecksum " \ - "FROM metadata " \ +#define METADATA_QUERY \ + "phash, path, inode, modtime, type, md5, fileid, remotePerm, " \ + "filesize, ignoredChildrenRemote, " \ + "contentchecksumtype.name || ':' || contentChecksum " \ + "FROM metadata " \ "LEFT JOIN checksumtype as contentchecksumtype ON metadata.contentChecksumTypeId == contentchecksumtype.id" // This funciton parses a line from the metadata table into the given csync_file_stat // structure which it is also allocating. // Note that this function calls laso sqlite3_step to actually get the info from db and // returns the sqlite return type. -static int _csync_file_stat_from_metadata_table( csync_file_stat_t **st, sqlite3_stmt *stmt ) +static int _csync_file_stat_from_metadata_table( std::unique_ptr &st, sqlite3_stmt *stmt ) { int rc = SQLITE_ERROR; - int column_count; - int len; if( ! stmt ) { CSYNC_LOG(CSYNC_LOG_PRIORITY_ERROR, "Fatal: Statement is NULL."); return SQLITE_ERROR; } - column_count = sqlite3_column_count(stmt); + // Callers should all use METADATA_QUERY for their column list. + assert(sqlite3_column_count(stmt) == 11); SQLITE_BUSY_HANDLED( sqlite3_step(stmt) ); if( rc == SQLITE_ROW ) { - if(column_count > 7) { - const char *name; - - /* phash, pathlen, path, inode, uid, gid, mode, modtime */ - len = sqlite3_column_int(stmt, 1); - *st = (csync_file_stat_t*)c_malloc(sizeof(csync_file_stat_t) + len + 1); - /* clear the whole structure */ - ZERO_STRUCTP(*st); - - /* The query suceeded so use the phash we pass to the function. */ - (*st)->phash = sqlite3_column_int64(stmt, 0); - - (*st)->pathlen = sqlite3_column_int(stmt, 1); - name = (const char*) sqlite3_column_text(stmt, 2); - memcpy((*st)->path, (len ? name : ""), len + 1); - (*st)->inode = sqlite3_column_int64(stmt,3); - (*st)->mode = sqlite3_column_int(stmt, 6); - (*st)->modtime = strtoul((char*)sqlite3_column_text(stmt, 7), NULL, 10); - - if(*st && column_count > 8 ) { - (*st)->type = static_cast(sqlite3_column_int(stmt, 8)); - } - - if(column_count > 9 && sqlite3_column_text(stmt, 9)) { - (*st)->etag = c_strdup( (char*) sqlite3_column_text(stmt, 9) ); - } - if(column_count > 10 && sqlite3_column_text(stmt,10)) { - csync_vio_set_file_id((*st)->file_id, (char*) sqlite3_column_text(stmt, 10)); - } - if(column_count > 11 && sqlite3_column_text(stmt,11)) { - strncpy((*st)->remotePerm, - (char*) sqlite3_column_text(stmt, 11), - REMOTE_PERM_BUF_SIZE); - } - if(column_count > 12 && sqlite3_column_int64(stmt,12)) { - (*st)->size = sqlite3_column_int64(stmt, 12); - } - if(column_count > 13) { - (*st)->has_ignored_files = sqlite3_column_int(stmt, 13); - } - if (column_count > 14 && sqlite3_column_text(stmt, 14)) { - (*st)->checksumHeader = c_strdup((char *)sqlite3_column_text(stmt, 14)); - } - - } + st.reset(new csync_file_stat_t); + + /* The query suceeded so use the phash we pass to the function. */ + st->phash = sqlite3_column_int64(stmt, 0); + st->path = (char*)sqlite3_column_text(stmt, 1); + st->inode = sqlite3_column_int64(stmt, 2); + st->modtime = strtoul((char*)sqlite3_column_text(stmt, 3), NULL, 10); + st->type = static_cast(sqlite3_column_int(stmt, 4)); + st->etag = (char*)sqlite3_column_text(stmt, 5); + st->file_id = (char*)sqlite3_column_text(stmt, 6); + st->remotePerm = (char*)sqlite3_column_text(stmt, 7); + st->size = sqlite3_column_int64(stmt, 8); + st->has_ignored_files = sqlite3_column_int(stmt, 9); + st->checksumHeader = (char *)sqlite3_column_text(stmt, 10); } else { if( rc != SQLITE_DONE ) { CSYNC_LOG(CSYNC_LOG_PRIORITY_WARN, "Query results in %d", rc); @@ -309,10 +278,10 @@ static int _csync_file_stat_from_metadata_table( csync_file_stat_t **st, sqlite3 } /* caller must free the memory */ -csync_file_stat_t *csync_statedb_get_stat_by_hash(CSYNC *ctx, +std::unique_ptr csync_statedb_get_stat_by_hash(CSYNC *ctx, uint64_t phash) { - csync_file_stat_t *st = NULL; + std::unique_ptr st; int rc; if( !ctx || ctx->db_is_empty ) { @@ -336,7 +305,7 @@ csync_file_stat_t *csync_statedb_get_stat_by_hash(CSYNC *ctx, sqlite3_bind_int64(ctx->statedb.by_hash_stmt, 1, (long long signed int)phash); - rc = _csync_file_stat_from_metadata_table(&st, ctx->statedb.by_hash_stmt); + rc = _csync_file_stat_from_metadata_table(st, ctx->statedb.by_hash_stmt); ctx->statedb.lastReturnValue = rc; if( !(rc == SQLITE_ROW || rc == SQLITE_DONE) ) { CSYNC_LOG(CSYNC_LOG_PRIORITY_ERROR, "WRN: Could not get line from metadata: %d!", rc); @@ -346,9 +315,9 @@ csync_file_stat_t *csync_statedb_get_stat_by_hash(CSYNC *ctx, return st; } -csync_file_stat_t *csync_statedb_get_stat_by_file_id(CSYNC *ctx, +std::unique_ptr csync_statedb_get_stat_by_file_id(CSYNC *ctx, const char *file_id ) { - csync_file_stat_t *st = NULL; + std::unique_ptr st; int rc = 0; if (!file_id) { @@ -376,7 +345,7 @@ csync_file_stat_t *csync_statedb_get_stat_by_file_id(CSYNC *ctx, /* bind the query value */ sqlite3_bind_text(ctx->statedb.by_fileid_stmt, 1, file_id, -1, SQLITE_STATIC); - rc = _csync_file_stat_from_metadata_table(&st, ctx->statedb.by_fileid_stmt); + rc = _csync_file_stat_from_metadata_table(st, ctx->statedb.by_fileid_stmt); ctx->statedb.lastReturnValue = rc; if( !(rc == SQLITE_ROW || rc == SQLITE_DONE) ) { CSYNC_LOG(CSYNC_LOG_PRIORITY_ERROR, "WRN: Could not get line from metadata: %d!", rc); @@ -388,10 +357,10 @@ csync_file_stat_t *csync_statedb_get_stat_by_file_id(CSYNC *ctx, } /* caller must free the memory */ -csync_file_stat_t *csync_statedb_get_stat_by_inode(CSYNC *ctx, +std::unique_ptr csync_statedb_get_stat_by_inode(CSYNC *ctx, uint64_t inode) { - csync_file_stat_t *st = NULL; + std::unique_ptr st; int rc; if (!inode) { @@ -419,7 +388,7 @@ csync_file_stat_t *csync_statedb_get_stat_by_inode(CSYNC *ctx, sqlite3_bind_int64(ctx->statedb.by_inode_stmt, 1, (long long signed int)inode); - rc = _csync_file_stat_from_metadata_table(&st, ctx->statedb.by_inode_stmt); + rc = _csync_file_stat_from_metadata_table(st, ctx->statedb.by_inode_stmt); ctx->statedb.lastReturnValue = rc; if( !(rc == SQLITE_ROW || rc == SQLITE_DONE) ) { CSYNC_LOG(CSYNC_LOG_PRIORITY_ERROR, "WRN: Could not get line from metadata by inode: %d!", rc); @@ -465,9 +434,9 @@ int csync_statedb_get_below_path( CSYNC *ctx, const char *path ) { ctx->statedb.lastReturnValue = rc; do { - csync_file_stat_t *st = NULL; + std::unique_ptr st; - rc = _csync_file_stat_from_metadata_table( &st, stmt); + rc = _csync_file_stat_from_metadata_table(st, stmt); if( st ) { /* When selective sync is used, the database may have subtrees with a parent * whose etag (md5) is _invalid_. These are ignored and shall not appear in the @@ -476,21 +445,21 @@ int csync_statedb_get_below_path( CSYNC *ctx, const char *path ) { * _invalid_, but that is not a problem as the next discovery will retrieve * their correct etags again and we don't run into this case. */ - if( c_streq(st->etag, "_invalid_") ) { - CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "%s selective sync excluded", st->path); - char *skipbase = c_strdup(st->path); - skipbase[st->pathlen] = '/'; - int skiplen = st->pathlen + 1; + if( st->etag == "_invalid_") { + CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "%s selective sync excluded", st->path.constData()); + QByteArray skipbase = st->path; + skipbase += '/'; /* Skip over all entries with the same base path. Note that this depends * strongly on the ordering of the retrieved items. */ do { - csync_file_stat_free(st); - rc = _csync_file_stat_from_metadata_table( &st, stmt); - if( st && strncmp(st->path, skipbase, skiplen) != 0 ) { - break; + st.reset(); + rc = _csync_file_stat_from_metadata_table(st, stmt); + if( st ) { + if( st->path == skipbase ) + break; + CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "%s selective sync excluded because the parent is", st->path.constData()); } - CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "%s selective sync excluded because the parent is", st->path); } while( rc == SQLITE_ROW ); /* End of data? */ @@ -504,11 +473,11 @@ int csync_statedb_get_below_path( CSYNC *ctx, const char *path ) { * without a full remote discovery being triggered. */ CSYNC_EXCLUDE_TYPE excluded = csync_excluded_traversal(ctx->excludes, st->path, st->type); if (excluded != CSYNC_NOT_EXCLUDED) { - CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "%s excluded (%d)", st->path, excluded); + CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "%s excluded (%d)", st->path.constData(), excluded); if (excluded == CSYNC_FILE_EXCLUDE_AND_REMOVE || excluded == CSYNC_FILE_SILENTLY_EXCLUDED) { - csync_file_stat_free(st); + st.reset(); continue; } @@ -516,8 +485,8 @@ int csync_statedb_get_below_path( CSYNC *ctx, const char *path ) { } /* store into result list. */ - if (c_rbtree_insert(ctx->remote.tree, (void *) st) < 0) { - csync_file_stat_free(st); + if (c_rbtree_insert(ctx->remote.tree, (void *) st.release()) < 0) { + st.reset(); ctx->status_code = CSYNC_STATUS_TREE_ERROR; break; } diff --git a/src/csync/csync_statedb.h b/src/csync/csync_statedb.h index dc58a9b08..8ef2235af 100644 --- a/src/csync/csync_statedb.h +++ b/src/csync/csync_statedb.h @@ -56,11 +56,11 @@ OCSYNC_EXPORT int csync_statedb_load(CSYNC *ctx, const char *statedb, sqlite3 ** OCSYNC_EXPORT int csync_statedb_close(CSYNC *ctx); -OCSYNC_EXPORT csync_file_stat_t *csync_statedb_get_stat_by_hash(CSYNC *ctx, uint64_t phash); +OCSYNC_EXPORT std::unique_ptr csync_statedb_get_stat_by_hash(CSYNC *ctx, uint64_t phash); -OCSYNC_EXPORT csync_file_stat_t *csync_statedb_get_stat_by_inode(CSYNC *ctx, uint64_t inode); +OCSYNC_EXPORT std::unique_ptr csync_statedb_get_stat_by_inode(CSYNC *ctx, uint64_t inode); -OCSYNC_EXPORT csync_file_stat_t *csync_statedb_get_stat_by_file_id(CSYNC *ctx, const char *file_id); +OCSYNC_EXPORT std::unique_ptr csync_statedb_get_stat_by_file_id(CSYNC *ctx, const char *file_id); /** * @brief Query all files metadata inside and below a path. diff --git a/src/csync/csync_time.c b/src/csync/csync_time.c index 3092819a3..442263cee 100644 --- a/src/csync/csync_time.c +++ b/src/csync/csync_time.c @@ -29,7 +29,6 @@ #include #include "csync_time.h" -#include "vio/csync_vio.h" #ifndef _WIN32 #include diff --git a/src/csync/csync_time.h b/src/csync/csync_time.h index 55aa09841..14d355b32 100644 --- a/src/csync/csync_time.h +++ b/src/csync/csync_time.h @@ -27,8 +27,6 @@ extern "C" { #include -#include "csync_private.h" - int csync_gettime(struct timespec *tp); void csync_sleep(unsigned int msecs); diff --git a/src/csync/csync_update.cpp b/src/csync/csync_update.cpp index 927290b1a..e62e3be29 100644 --- a/src/csync/csync_update.cpp +++ b/src/csync/csync_update.cpp @@ -164,11 +164,9 @@ static bool _csync_mtime_equal(time_t a, time_t b) static int _csync_detect_update(CSYNC *ctx, const char *file, const csync_vio_file_stat_t *fs, enum csync_ftw_type_e type) { uint64_t h = 0; - size_t len = 0; - size_t size = 0; const char *path = NULL; - csync_file_stat_t *st = NULL; - csync_file_stat_t *tmp = NULL; + std::unique_ptr st; + std::unique_ptr tmp; CSYNC_EXCLUDE_TYPE excluded; if ((file == NULL) || (fs == NULL)) { @@ -186,8 +184,6 @@ static int _csync_detect_update(CSYNC *ctx, const char *file, path += strlen(ctx->local.uri) + 1; } - len = strlen(path); - if (type == CSYNC_FTW_TYPE_SKIP) { excluded =CSYNC_FILE_EXCLUDE_STAT_FAILED; } else { @@ -225,15 +221,9 @@ static int _csync_detect_update(CSYNC *ctx, const char *file, if( h == 0 ) { return -1; } - size = sizeof(csync_file_stat_t) + len + 1; - st = static_cast(c_malloc(size)); + st.reset(new csync_file_stat_t); - /* Set instruction by default to none */ - st->instruction = CSYNC_INSTRUCTION_NONE; - st->etag = NULL; - st->child_modified = 0; - st->has_ignored_files = 0; if (type == CSYNC_FTW_TYPE_FILE ) { if (fs->mtime == 0) { CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "file: %s - mtime is zero!", path); @@ -258,8 +248,6 @@ static int _csync_detect_update(CSYNC *ctx, const char *file, tmp = csync_statedb_get_stat_by_hash(ctx, h); if(_last_db_return_error(ctx)) { - csync_file_stat_free(st); - csync_file_stat_free(tmp); ctx->status_code = CSYNC_STATUS_UNSUCCESSFUL; return -1; } @@ -270,14 +258,14 @@ static int _csync_detect_update(CSYNC *ctx, const char *file, ", etag: %s <-> %s, inode: %" PRId64 " <-> %" PRId64 ", size: %" PRId64 " <-> %" PRId64 ", perms: %s <-> %s, ignore: %d", ((int64_t) fs->mtime), ((int64_t) tmp->modtime), - fs->etag, tmp->etag, (uint64_t) fs->inode, (uint64_t) tmp->inode, - (uint64_t) fs->size, (uint64_t) tmp->size, fs->remotePerm, tmp->remotePerm, tmp->has_ignored_files ); + fs->etag, tmp->etag.constData(), (uint64_t) fs->inode, (uint64_t) tmp->inode, + (uint64_t) fs->size, (uint64_t) tmp->size, fs->remotePerm, tmp->remotePerm.constData(), tmp->has_ignored_files ); if (ctx->current == REMOTE_REPLICA && !c_streq(fs->etag, tmp->etag)) { st->instruction = CSYNC_INSTRUCTION_EVAL; // Preserve the EVAL flag later on if the type has changed. - if (_csync_filetype_different(tmp, fs)) { - st->child_modified = 1; + if (_csync_filetype_different(tmp.get(), fs)) { + st->child_modified = true; } goto out; @@ -290,15 +278,15 @@ static int _csync_detect_update(CSYNC *ctx, const char *file, // Checksum comparison at this stage is only enabled for .eml files, // check #4754 #4755 bool isEmlFile = csync_fnmatch("*.eml", file, FNM_CASEFOLD) == 0; - if (isEmlFile && fs->size == tmp->size && tmp->checksumHeader) { + if (isEmlFile && fs->size == tmp->size && !tmp->checksumHeader.isEmpty()) { if (ctx->callbacks.checksum_hook) { st->checksumHeader = ctx->callbacks.checksum_hook( file, tmp->checksumHeader, ctx->callbacks.checksum_userdata); } bool checksumIdentical = false; - if (st->checksumHeader) { - checksumIdentical = strncmp(st->checksumHeader, tmp->checksumHeader, 1000) == 0; + if (!st->checksumHeader.isEmpty()) { + checksumIdentical = st->checksumHeader == tmp->checksumHeader; } if (checksumIdentical) { CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "NOTE: Checksums are identical, file did not actually change: %s", path); @@ -308,8 +296,8 @@ static int _csync_detect_update(CSYNC *ctx, const char *file, } // Preserve the EVAL flag later on if the type has changed. - if (_csync_filetype_different(tmp, fs)) { - st->child_modified = 1; + if (_csync_filetype_different(tmp.get(), fs)) { + st->child_modified = true; } st->instruction = CSYNC_INSTRUCTION_EVAL; @@ -344,9 +332,6 @@ static int _csync_detect_update(CSYNC *ctx, const char *file, } else { enum csync_vio_file_type_e tmp_vio_type = CSYNC_VIO_FILE_TYPE_UNKNOWN; - /* tmp might point to malloc mem, so free it here before reusing tmp */ - csync_file_stat_free(tmp); - /* check if it's a file and has been renamed */ if (ctx->current == LOCAL_REPLICA) { CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "Checking for rename based on inode # %" PRId64 "", (uint64_t) fs->inode); @@ -354,7 +339,6 @@ static int _csync_detect_update(CSYNC *ctx, const char *file, tmp = csync_statedb_get_stat_by_inode(ctx, fs->inode); if(_last_db_return_error(ctx)) { - csync_file_stat_free(st); ctx->status_code = CSYNC_STATUS_UNSUCCESSFUL; return -1; } @@ -383,14 +367,14 @@ static int _csync_detect_update(CSYNC *ctx, const char *file, // Verify the checksum where possible - if (isRename && tmp->checksumHeader && ctx->callbacks.checksum_hook + if (isRename && !tmp->checksumHeader.isEmpty() && ctx->callbacks.checksum_hook && fs->type == CSYNC_VIO_FILE_TYPE_REGULAR) { st->checksumHeader = ctx->callbacks.checksum_hook( file, tmp->checksumHeader, ctx->callbacks.checksum_userdata); - if (st->checksumHeader) { - CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "checking checksum of potential rename %s %s <-> %s", path, st->checksumHeader, tmp->checksumHeader); - isRename = strncmp(st->checksumHeader, tmp->checksumHeader, 1000) == 0; + if (!st->checksumHeader.isEmpty()) { + CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "checking checksum of potential rename %s %s <-> %s", path, st->checksumHeader.constData(), tmp->checksumHeader.constData()); + isRename = st->checksumHeader == tmp->checksumHeader; } } @@ -409,17 +393,16 @@ static int _csync_detect_update(CSYNC *ctx, const char *file, tmp = csync_statedb_get_stat_by_file_id(ctx, fs->file_id); if(_last_db_return_error(ctx)) { - csync_file_stat_free(st); ctx->status_code = CSYNC_STATUS_UNSUCCESSFUL; return -1; } if(tmp ) { /* tmp existing at all */ - if ( _csync_filetype_different(tmp, fs)) { + if ( _csync_filetype_different(tmp.get(), fs)) { CSYNC_LOG(CSYNC_LOG_PRIORITY_WARN, "file types different is not!"); st->instruction = CSYNC_INSTRUCTION_NEW; goto out; } - CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "remote rename detected based on fileid %s %s", tmp->path, file); + CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "remote rename detected based on fileid %s %s", tmp->path.constData(), file); st->instruction = CSYNC_INSTRUCTION_EVAL_RENAME; if (fs->type == CSYNC_VIO_FILE_TYPE_DIRECTORY) { csync_rename_record(ctx, tmp->path, path); @@ -438,7 +421,6 @@ static int _csync_detect_update(CSYNC *ctx, const char *file, if (fs->type == CSYNC_VIO_FILE_TYPE_DIRECTORY && ctx->current == REMOTE_REPLICA && ctx->callbacks.checkSelectiveSyncNewFolderHook) { if (ctx->callbacks.checkSelectiveSyncNewFolderHook(ctx->callbacks.update_callback_userdata, path, fs->remotePerm)) { - csync_file_stat_free(st); return 1; } } @@ -448,7 +430,6 @@ static int _csync_detect_update(CSYNC *ctx, const char *file, } } else { CSYNC_LOG(CSYNC_LOG_PRIORITY_WARN, "Unable to open statedb" ); - csync_file_stat_free(st); ctx->status_code = CSYNC_STATUS_UNSUCCESSFUL; return -1; } @@ -481,54 +462,49 @@ out: && st->instruction != CSYNC_INSTRUCTION_IGNORE && st->instruction != CSYNC_INSTRUCTION_UPDATE_METADATA && type != CSYNC_FTW_TYPE_DIR) { - st->child_modified = 1; + st->child_modified = true; } - ctx->current_fs = st; + ctx->current_fs = st.get(); - csync_file_stat_free(tmp); st->inode = fs->inode; - st->mode = fs->mode; st->size = fs->size; st->modtime = fs->mtime; st->type = type; st->etag = NULL; if( fs->etag ) { - SAFE_FREE(st->etag); - st->etag = c_strdup(fs->etag); + st->etag = fs->etag; } - csync_vio_set_file_id(st->file_id, fs->file_id); + st->file_id = fs->file_id; if (fs->fields & CSYNC_VIO_FILE_STAT_FIELDS_DIRECTDOWNLOADURL) { - SAFE_FREE(st->directDownloadUrl); - st->directDownloadUrl = c_strdup(fs->directDownloadUrl); + st->directDownloadUrl = fs->directDownloadUrl; } if (fs->fields & CSYNC_VIO_FILE_STAT_FIELDS_DIRECTDOWNLOADCOOKIES) { - SAFE_FREE(st->directDownloadCookies); - st->directDownloadCookies = c_strdup(fs->directDownloadCookies); + st->directDownloadCookies = fs->directDownloadCookies; } if (fs->fields & CSYNC_VIO_FILE_STAT_FIELDS_PERM) { - strncpy(st->remotePerm, fs->remotePerm, REMOTE_PERM_BUF_SIZE); + st->remotePerm = fs->remotePerm; } // For the remote: propagate the discovered checksum if (fs->checksumHeader && ctx->current == REMOTE_REPLICA) { - st->checksumHeader = c_strdup(fs->checksumHeader); + st->checksumHeader = fs->checksumHeader; } st->phash = h; - st->pathlen = len; - memcpy(st->path, (len ? path : ""), len + 1); + st->path = path; + + CSYNC_LOG(CSYNC_LOG_PRIORITY_INFO, "file: %s, instruction: %s <<=", st->path.constData(), + csync_instruction_str(st->instruction)); switch (ctx->current) { case LOCAL_REPLICA: - if (c_rbtree_insert(ctx->local.tree, (void *) st) < 0) { - csync_file_stat_free(st); + if (c_rbtree_insert(ctx->local.tree, (void *) st.release()) < 0) { ctx->status_code = CSYNC_STATUS_TREE_ERROR; return -1; } break; case REMOTE_REPLICA: - if (c_rbtree_insert(ctx->remote.tree, (void *) st) < 0) { - csync_file_stat_free(st); + if (c_rbtree_insert(ctx->remote.tree, (void *) st.release()) < 0) { ctx->status_code = CSYNC_STATUS_TREE_ERROR; return -1; } @@ -536,8 +512,6 @@ out: default: break; } - CSYNC_LOG(CSYNC_LOG_PRIORITY_INFO, "file: %s, instruction: %s <<=", st->path, - csync_instruction_str(st->instruction)); return 0; } @@ -546,7 +520,7 @@ int csync_walker(CSYNC *ctx, const char *file, const csync_vio_file_stat_t *fs, int flag) { int rc = -1; enum csync_ftw_type_e type = CSYNC_FTW_TYPE_SKIP; - csync_file_stat_t *st = NULL; + std::unique_ptr st; uint64_t h; if (ctx->abort) { @@ -587,8 +561,7 @@ int csync_walker(CSYNC *ctx, const char *file, const csync_vio_file_stat_t *fs, if( !st ) { return 0; } - csync_file_stat_free(st); - st = NULL; + st.reset(); type = CSYNC_FTW_TYPE_SKIP; break; diff --git a/src/libsync/checksums.cpp b/src/libsync/checksums.cpp index eaf36379c..448b5d477 100644 --- a/src/libsync/checksums.cpp +++ b/src/libsync/checksums.cpp @@ -230,23 +230,19 @@ CSyncChecksumHook::CSyncChecksumHook() { } -const char *CSyncChecksumHook::hook(const char *path, const char *otherChecksumHeader, void * /*this_obj*/) +QByteArray CSyncChecksumHook::hook(const QByteArray &path, const QByteArray &otherChecksumHeader, void * /*this_obj*/) { QByteArray type = parseChecksumHeaderType(QByteArray(otherChecksumHeader)); if (type.isEmpty()) return NULL; - QByteArray checksum = ComputeChecksum::computeNow(path, type); + QByteArray checksum = ComputeChecksum::computeNow(QString::fromUtf8(path), type); if (checksum.isNull()) { qCWarning(lcChecksums) << "Failed to compute checksum" << type << "for" << path; return NULL; } - QByteArray checksumHeader = makeChecksumHeader(type, checksum); - char *result = (char *)malloc(checksumHeader.size() + 1); - memcpy(result, checksumHeader.constData(), checksumHeader.size()); - result[checksumHeader.size()] = 0; - return result; + return makeChecksumHeader(type, checksum); } } diff --git a/src/libsync/checksums.h b/src/libsync/checksums.h index a7259f7ec..3c31c8c03 100644 --- a/src/libsync/checksums.h +++ b/src/libsync/checksums.h @@ -131,6 +131,6 @@ public: * to be set as userdata. * The return value will be owned by csync. */ - static const char *hook(const char *path, const char *otherChecksumHeader, void *this_obj); + static QByteArray hook(const QByteArray &path, const QByteArray &otherChecksumHeader, void *this_obj); }; } diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index d4afcc57c..6fff8e538 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -437,7 +437,7 @@ int SyncEngine::treewalkFile(TREE_WALK_FILE *file, bool remote) * files are often read from database rather than being pulled from remote. */ if (remote) { - item->_serverHasIgnoredFiles = (file->has_ignored_files > 0); + item->_serverHasIgnoredFiles = file->has_ignored_files; } // Sometimes the discovery computes checksums for local files diff --git a/test/csync/csync_tests/check_csync_statedb_query.cpp b/test/csync/csync_tests/check_csync_statedb_query.cpp index 843900ec2..d856d94ab 100644 --- a/test/csync/csync_tests/check_csync_statedb_query.cpp +++ b/test/csync/csync_tests/check_csync_statedb_query.cpp @@ -155,8 +155,8 @@ static void check_csync_statedb_insert_metadata(void **state) assert_int_equal(rc, 0); for (i = 0; i < 100; i++) { - st = (csync_file_stat_t*)c_malloc(sizeof(csync_file_stat_t) + 30 ); - snprintf(st->path, 29, "file_%d" , i ); + st = new csync_file_stat_t; + st->path = QString("file_%1").arg(i).toUtf8(); st->phash = i; rc = c_rbtree_insert(csync->local.tree, (void *) st); @@ -174,8 +174,8 @@ static void check_csync_statedb_write(void **state) int i, rc; for (i = 0; i < 100; i++) { - st = (csync_file_stat_t*)c_malloc(sizeof(csync_file_stat_t) + 30); - snprintf(st->path, 29, "file_%d" , i ); + st = new csync_file_stat_t; + st->path = QString("file_%1").arg(i).toUtf8(); st->phash = i; rc = c_rbtree_insert(csync->local.tree, (void *) st); @@ -190,22 +190,20 @@ static void check_csync_statedb_write(void **state) static void check_csync_statedb_get_stat_by_hash_not_found(void **state) { CSYNC *csync = (CSYNC*)*state; - csync_file_stat_t *tmp; + std::unique_ptr tmp; tmp = csync_statedb_get_stat_by_hash(csync, (uint64_t) 666); - assert_null(tmp); - - free(tmp); + assert_null(tmp.get()); } static void check_csync_statedb_get_stat_by_inode_not_found(void **state) { CSYNC *csync = (CSYNC*)*state; - csync_file_stat_t *tmp; + std::unique_ptr tmp; tmp = csync_statedb_get_stat_by_inode(csync, (ino_t) 666); - assert_null(tmp); + assert_null(tmp.get()); } int torture_run_tests(void) diff --git a/test/csync/std_tests/check_std_c_time.c b/test/csync/std_tests/check_std_c_time.c index 56fc6ad40..52c5fd808 100644 --- a/test/csync/std_tests/check_std_c_time.c +++ b/test/csync/std_tests/check_std_c_time.c @@ -23,6 +23,7 @@ #include "csync_time.h" #include "std/c_time.h" +#include static void check_c_tspecdiff(void **state) { diff --git a/test/testcsyncsqlite.cpp b/test/testcsyncsqlite.cpp index 09dc1528a..fb2767816 100644 --- a/test/testcsyncsqlite.cpp +++ b/test/testcsyncsqlite.cpp @@ -29,13 +29,11 @@ private slots: } void testFullResult() { - csync_file_stat_t *st = csync_statedb_get_stat_by_hash((CSYNC*)(&_ctx), 2081025720555645157 ); - QVERIFY(st); + std::unique_ptr st = csync_statedb_get_stat_by_hash((CSYNC*)(&_ctx), 2081025720555645157 ); + QVERIFY(st.get()); QCOMPARE( QString::number(st->phash), QString::number(2081025720555645157) ); - QCOMPARE( QString::number(st->pathlen), QString::number(13)); QCOMPARE( QString::fromUtf8(st->path), QLatin1String("test2/zu/zuzu") ); QCOMPARE( QString::number(st->inode), QString::number(1709554)); - QCOMPARE( QString::number(st->mode), QString::number(0)); QCOMPARE( QString::number(st->modtime), QString::number(1384415006)); QCOMPARE( QString::number(st->type), QString::number(2)); QCOMPARE( QString::fromUtf8(st->etag), QLatin1String("52847f2090665")); @@ -44,39 +42,33 @@ private slots: } void testByHash() { - csync_file_stat_t *st = csync_statedb_get_stat_by_hash((CSYNC*)(&_ctx), -7147279406142960289); - QVERIFY(st); + std::unique_ptr st = csync_statedb_get_stat_by_hash((CSYNC*)(&_ctx), -7147279406142960289); + QVERIFY(st.get()); QCOMPARE(QString::fromUtf8(st->path), QLatin1String("documents/c1")); - csync_file_stat_free(st); st = csync_statedb_get_stat_by_hash((CSYNC*)(&_ctx), 5426481156826978940); - QVERIFY(st); + QVERIFY(st.get()); QCOMPARE(QString::fromUtf8(st->path), QLatin1String("documents/c1/c2")); - csync_file_stat_free(st); } void testByInode() { - csync_file_stat_t *st = csync_statedb_get_stat_by_inode((CSYNC*)(&_ctx), 1709555); - QVERIFY(st); + std::unique_ptr st = csync_statedb_get_stat_by_inode((CSYNC*)(&_ctx), 1709555); + QVERIFY(st.get()); QCOMPARE(QString::fromUtf8(st->path), QLatin1String("test2/zu/zuzu/zuzuzu")); - csync_file_stat_free(st); st = csync_statedb_get_stat_by_inode((CSYNC*)(&_ctx), 1706571); - QVERIFY(st); + QVERIFY(st.get()); QCOMPARE(QString::fromUtf8(st->path), QLatin1String("Shared/for_kf/a2")); - csync_file_stat_free(st); } void testByFileId() { - csync_file_stat_t *st = csync_statedb_get_stat_by_file_id((CSYNC*)(&_ctx), "00000556525d5af3d9625"); - QVERIFY(st); + std::unique_ptr st = csync_statedb_get_stat_by_file_id((CSYNC*)(&_ctx), "00000556525d5af3d9625"); + QVERIFY(st.get()); QCOMPARE(QString::fromUtf8(st->path), QLatin1String("test2/zu")); - csync_file_stat_free(st); st = csync_statedb_get_stat_by_file_id((CSYNC*)(&_ctx), "-0000001525d5af3d9625"); - QVERIFY(st); + QVERIFY(st.get()); QCOMPARE(QString::fromUtf8(st->path), QLatin1String("Shared")); - csync_file_stat_free(st); } void cleanupTestCase() {