mirrors/nextcloud-desktop
3
0
Fork 0
mirror of https://github.com/nextcloud/desktop.git synced 2025-10-26 11:17:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

csync: refactor csync_s::error_string to avoid valgrind error

Browse Source 
The problem here is that we were sometimes allocating the error_string with
qstrdup, which need to be released with delete[] and not free().

Simplify the code by using QString instead.

```
==7230== Mismatched free() / delete / delete []
==7230==    at 0x4C2E10B: free (vg_replace_malloc.c:530)
==7230==    by 0x57C2321: csync_s::reinitialize() (csync.cpp:247)
==7230==    by 0x548130F: OCC::SyncEngine::finalize(bool) (syncengine.cpp:1212)
==7230==    by 0x5481223: OCC::SyncEngine::handleSyncError(csync_s*, char const*) (syncengine.cpp:746)
==7230==    by 0x5483E78: OCC::SyncEngine::slotDiscoveryJobFinished(int) (syncengine.cpp:965)
==7230==    by 0x5495E34: QtPrivate::FunctorCall<QtPrivate::IndexesList<0>, QtPrivate::List<int>, void, void (OCC::SyncEngine::*)(int)>::call(void (OCC::SyncEngine::*)(int), OCC::SyncEngine*, void**) (qobjectdefs_impl.h:134)
==7230==    by 0x5495D92: void QtPrivate::FunctionPointer<void (OCC::SyncEngine::*)(int)>::call<QtPrivate::List<int>, void>(void (OCC::SyncEngine::*)(int), OCC::SyncEngine*, void**) (qobjectdefs_impl.h:167)
==7230==    by 0x5495CB5: QtPrivate::QSlotObject<void (OCC::SyncEngine::*)(int), QtPrivate::List<int>, void>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) (qobjectdefs_impl.h:396)
==7230==    by 0xA9BF2E1: QObject::event(QEvent*) (in /usr/lib/libQt5Core.so.5.11.0)
==7230==    by 0x64BE983: QApplicationPrivate::notify_helper(QObject*, QEvent*) (in /usr/lib/libQt5Widgets.so.5.11.0)
==7230==    by 0x64C625A: QApplication::notify(QObject*, QEvent*) (in /usr/lib/libQt5Widgets.so.5.11.0)
==7230==    by 0xA994BC8: QCoreApplication::notifyInternal2(QObject*, QEvent*) (in /usr/lib/libQt5Core.so.5.11.0)
==7230==  Address 0x225b2640 is 0 bytes inside a block of size 50 alloc'd
==7230==    at 0x4C2DC6F: operator new[](unsigned long) (vg_replace_malloc.c:423)
==7230==    by 0xA7E8FC8: qstrdup(char const*) (in /usr/lib/libQt5Core.so.5.11.0)
==7230==    by 0x53F5750: OCC::DiscoveryJob::remote_vio_opendir_hook(char const*, void*) (discoveryphase.cpp:666)
==7230==    by 0x57E1278: csync_vio_opendir(csync_s*, char const*) (csync_vio.cpp:39)
==7230==    by 0x57D718F: csync_ftw(csync_s*, char const*, int (*)(csync_s*, std::unique_ptr<csync_file_stat_s, std::default_delete<csync_file_stat_s> >), unsigned int) (csync_update.cpp:674)
==7230==    by 0x57C1B05: csync_update(csync_s*) (csync.cpp:109)
==7230==    by 0x53F5BCC: OCC::DiscoveryJob::start() (discoveryphase.cpp:718)
==7230==    by 0x54B8F74: OCC::DiscoveryJob::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (moc_discoveryphase.cpp:494)
==7230==    by 0xA9BF2E1: QObject::event(QEvent*) (in /usr/lib/libQt5Core.so.5.11.0)
==7230==    by 0x64BE983: QApplicationPrivate::notify_helper(QObject*, QEvent*) (in /usr/lib/libQt5Widgets.so.5.11.0)
==7230==    by 0x64C625A: QApplication::notify(QObject*, QEvent*) (in /usr/lib/libQt5Widgets.so.5.11.0)
==7230==    by 0xA994BC8: QCoreApplication::notifyInternal2(QObject*, QEvent*) (in /usr/lib/libQt5Core.so.5.11.0)
==7230==
```
This commit is contained in:
Olivier Goffart 2018-05-31 10:56:52 +02:00
parent 9e26b0fea3
commit 888f2aae2f
8 changed files with 11 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/csync/csync.cpp
View File

@ -244,7 +244,7 @@ int csync_s::reinitialize() {
renames.folder_renamed_to.clear();
status = CSYNC_STATUS_INIT;
SAFE_FREE(error_string);
error_string.clear();
rc = 0;
return rc;
@ -252,7 +252,6 @@ int csync_s::reinitialize() {
csync_s::~csync_s() {
SAFE_FREE(local.uri);
SAFE_FREE(error_string);
}
void *csync_get_userdata(CSYNC *ctx) {
@ -298,11 +297,6 @@ CSYNC_STATUS csync_get_status(CSYNC *ctx) {
return ctx->status_code;
}
const char *csync_get_status_string(CSYNC *ctx)
{
return csync_vio_get_status_string(ctx);
}
void csync_request_abort(CSYNC *ctx)
{
if (ctx != NULL) {

9
src/csync/csync.h
View File

@ -307,15 +307,6 @@ int OCSYNC_EXPORT csync_walk_local_tree(CSYNC *ctx, const csync_treewalk_visit_f
*/
int OCSYNC_EXPORT csync_walk_remote_tree(CSYNC *ctx, const csync_treewalk_visit_func &visitor);
/**
* @brief Get the csync status string.
*
* @param ctx The csync context.
*
* @return A const pointer to a string with more precise status info.
*/
const char OCSYNC_EXPORT *csync_get_status_string(CSYNC *ctx);
/**
* @brief Aborts the current sync run as soon as possible. Can be called from another thread.
*

5
src/csync/csync_private.h
View File

@ -180,7 +180,10 @@ struct OCSYNC_EXPORT csync_s {
/* csync error code */
enum csync_status_codes_e status_code = CSYNC_STATUS_OK;
char *error_string = nullptr;
/* Some additional string information which is added to the error message corresponding to the error code in errno.
* Usually a filename
*/
QString error_string;
int status = CSYNC_STATUS_INIT;
volatile bool abort = false;

6
src/csync/csync_update.cpp
View File

@ -677,7 +677,6 @@ int csync_ftw(CSYNC *ctx, const char *uri, csync_walker_fn fn,
ctx->status_code = CSYNC_STATUS_ABORTED;
goto error;
}
int asp = 0;
/* permission denied */
ctx->status_code = csync_errno_to_status(errno, CSYNC_STATUS_OPENDIR_ERROR);
if (errno == EACCES) {
@ -686,8 +685,7 @@ int csync_ftw(CSYNC *ctx, const char *uri, csync_walker_fn fn,
return 0;
}
} else if(errno == ENOENT) {
asp = asprintf( &ctx->error_string, "%s", uri);
ASSERT(asp >= 0);
ctx->error_string = QString::fromUtf8(uri);
}
// 403 Forbidden can be sent by the server if the file firewall is active.
// A file or directory should be ignored and sync must continue. See #3490
@ -718,7 +716,7 @@ int csync_ftw(CSYNC *ctx, const char *uri, csync_walker_fn fn,
/* Conversion error */
if (dirent->path.isEmpty() && !dirent->original_path.isEmpty()) {
ctx->status_code = CSYNC_STATUS_INVALID_CHARACTERS;
ctx->error_string = c_strdup(dirent->original_path);
ctx->error_string = QString::fromUtf8(dirent->original_path);
dirent->original_path.clear();
goto error;
}

6
src/csync/vio/csync_vio.cpp
View File

@ -90,9 +90,3 @@ std::unique_ptr<csync_file_stat_t> csync_vio_readdir(CSYNC *ctx, csync_vio_handl
return NULL;
}
char *csync_vio_get_status_string(CSYNC *ctx) {
if(ctx->error_string) {
return ctx->error_string;
}
return 0;
}

4
src/csync/vio/csync_vio.h
View File

@ -35,8 +35,4 @@ typedef struct fhandle_s {
csync_vio_handle_t *csync_vio_opendir(CSYNC *ctx, const char *name);
int csync_vio_closedir(CSYNC *ctx, csync_vio_handle_t *dhandle);
std::unique_ptr<csync_file_stat_t> csync_vio_readdir(CSYNC *ctx, csync_vio_handle_t *dhandle);
char *csync_vio_get_status_string(CSYNC *ctx);
#endif /* _CSYNC_VIO_H */

2
src/libsync/discoveryphase.cpp
View File

@ -663,7 +663,7 @@ csync_vio_handle_t *DiscoveryJob::remote_vio_opendir_hook(const char *url,
qCDebug(lcDiscovery) << directoryResult->code << "when opening" << url << "msg=" << directoryResult->msg;
errno = directoryResult->code;
// save the error string to the context
discoveryJob->_csync_ctx->error_string = qstrdup(directoryResult->msg.toUtf8().constData());
discoveryJob->_csync_ctx->error_string = directoryResult->msg;
return NULL;
}

6
src/libsync/syncengine.cpp
View File

@ -715,13 +715,13 @@ int SyncEngine::treewalkFile(csync_file_stat_t *file, csync_file_stat_t *other,
void SyncEngine::handleSyncError(CSYNC *ctx, const char *state)
{
CSYNC_STATUS err = csync_get_status(ctx);
const char *errMsg = csync_get_status_string(ctx);
QString errMsg = ctx->error_string;
QString errStr = csyncErrorToString(err);
if (errMsg) {
if (!errMsg.isEmpty()) {
if (!errStr.endsWith(" ")) {
errStr.append(" ");
}
errStr += QString::fromUtf8(errMsg);
errStr += errMsg;
}
// Special handling CSYNC_STATUS_INVALID_CHARACTERS
if (err == CSYNC_STATUS_INVALID_CHARACTERS) {