From 4a86761fc29f12eb6857c1797f65def6c6218e51 Mon Sep 17 00:00:00 2001 From: Ronan Abhamon <ronan.abhamon@belledonne-communications.com> Date: Tue, 4 Sep 2018 14:54:06 +0200 Subject: [PATCH] fix(c-chat-message): avoid copy in linphone_chat_message_get_contents and leak --- include/linphone/api/c-api.h | 4 ++-- include/linphone/api/c-chat-message.h | 2 +- src/c-wrapper/api/c-chat-message.cpp | 14 ++++++++++++-- src/c-wrapper/internal/c-tools.h | 7 ++----- tester/multipart-tester.cpp | 11 +++++------ 5 files changed, 22 insertions(+), 16 deletions(-) diff --git a/include/linphone/api/c-api.h b/include/linphone/api/c-api.h index 3cae0b5c26..f57d544cfd 100644 --- a/include/linphone/api/c-api.h +++ b/include/linphone/api/c-api.h @@ -34,9 +34,9 @@ #include "linphone/api/c-content.h" #include "linphone/api/c-dial-plan.h" #include "linphone/api/c-event-log.h" -#include "linphone/api/c-participant.h" -#include "linphone/api/c-participant-imdn-state.h" #include "linphone/api/c-magic-search.h" +#include "linphone/api/c-participant-imdn-state.h" +#include "linphone/api/c-participant.h" #include "linphone/api/c-search-result.h" #include "linphone/api/c-types.h" diff --git a/include/linphone/api/c-chat-message.h b/include/linphone/api/c-chat-message.h index 0f47383200..1d6a2fb5c0 100644 --- a/include/linphone/api/c-chat-message.h +++ b/include/linphone/api/c-chat-message.h @@ -388,7 +388,7 @@ LINPHONE_PUBLIC void linphone_chat_message_remove_content (LinphoneChatMessage * * @param[in] msg #LinphoneChatMessage object. * @return \bctbx_list{LinphoneContent} the list of #LinphoneContent. */ -LINPHONE_PUBLIC bctbx_list_t* linphone_chat_message_get_contents(const LinphoneChatMessage *msg); +LINPHONE_PUBLIC const bctbx_list_t *linphone_chat_message_get_contents(const LinphoneChatMessage *msg); /** * Returns true if the chat message has a text content diff --git a/src/c-wrapper/api/c-chat-message.cpp b/src/c-wrapper/api/c-chat-message.cpp index 3ed7781edd..ba5902da8b 100644 --- a/src/c-wrapper/api/c-chat-message.cpp +++ b/src/c-wrapper/api/c-chat-message.cpp @@ -53,9 +53,16 @@ L_DECLARE_C_OBJECT_IMPL_WITH_XTORS(ChatMessage, void *message_state_changed_user_data; struct Cache { + ~Cache () { + if (contents) + bctbx_list_free(contents); + } + string contentType; string textContentBody; string customHeaderValue; + + bctbx_list_t *contents = nullptr; } mutable cache; ) @@ -250,8 +257,11 @@ void linphone_chat_message_remove_content (LinphoneChatMessage *msg, LinphoneCon L_GET_CPP_PTR_FROM_C_OBJECT(msg)->removeContent(L_GET_CPP_PTR_FROM_C_OBJECT(content)); } -bctbx_list_t* linphone_chat_message_get_contents(const LinphoneChatMessage *msg) { - return L_GET_RESOLVED_C_LIST_FROM_CPP_LIST(L_GET_CPP_PTR_FROM_C_OBJECT(msg)->getContents()); +const bctbx_list_t *linphone_chat_message_get_contents (const LinphoneChatMessage *msg) { + if (msg->cache.contents) + bctbx_free(msg->cache.contents); + msg->cache.contents = L_GET_RESOLVED_C_LIST_FROM_CPP_LIST(L_GET_CPP_PTR_FROM_C_OBJECT(msg)->getContents()); + return msg->cache.contents; } bool_t linphone_chat_message_has_text_content (const LinphoneChatMessage *msg) { diff --git a/src/c-wrapper/internal/c-tools.h b/src/c-wrapper/internal/c-tools.h index 4841aaa8d4..167f164c3d 100644 --- a/src/c-wrapper/internal/c-tools.h +++ b/src/c-wrapper/internal/c-tools.h @@ -583,11 +583,8 @@ public: > static inline bctbx_list_t *getResolvedCListFromCppList (const std::list<CppType *> &cppList) { bctbx_list_t *result = nullptr; - for (const auto &value : cppList) { - auto cValue = getCBackPtr(value->clone()); - reinterpret_cast<WrappedClonableObject<CppType> *>(cValue)->owner = WrappedObjectOwner::External; - result = bctbx_list_append(result, cValue); - } + for (const auto &value : cppList) + result = bctbx_list_append(result, getCBackPtr(value)); return result; } diff --git a/tester/multipart-tester.cpp b/tester/multipart-tester.cpp index 606d1e4a01..179446836e 100644 --- a/tester/multipart-tester.cpp +++ b/tester/multipart-tester.cpp @@ -36,7 +36,7 @@ using namespace std; using namespace LinphonePrivate; -static void check_contents(bctbx_list_t *contents, bool first_file_transfer, bool second_file_transfer, bool third_content) { +static void check_contents(const bctbx_list_t *contents, bool first_file_transfer, bool second_file_transfer, bool third_content) { BC_ASSERT_PTR_NOT_NULL(contents); if (third_content) BC_ASSERT_EQUAL(bctbx_list_size(contents), 3, int, "%d"); @@ -46,10 +46,10 @@ static void check_contents(bctbx_list_t *contents, bool first_file_transfer, boo int textContentCount = 0; int fileTransferContentCount = 0; int unexpectedContentCount = 0; - bctbx_list_t *it; - + const bctbx_list_t *it; + for (it = contents; it != NULL; it = bctbx_list_next(it)) { - LinphoneContent *content = (LinphoneContent *) bctbx_list_get_data(it); + const LinphoneContent *content = (LinphoneContent *)bctbx_list_get_data(it); BC_ASSERT_PTR_NOT_NULL(content); if (linphone_content_is_file_transfer(content)) { @@ -186,11 +186,10 @@ static void chat_message_multipart_modifier_base(bool first_file_transfer, bool BC_ASSERT_TRUE(wait_for(pauline->lc,marie->lc,&pauline->stat.number_of_LinphoneMessageReceived,1)); BC_ASSERT_PTR_NOT_NULL(pauline->stat.last_received_chat_message); - bctbx_list_t *contents = linphone_chat_message_get_contents(pauline->stat.last_received_chat_message); + const bctbx_list_t *contents = linphone_chat_message_get_contents(pauline->stat.last_received_chat_message); check_contents(contents, first_file_transfer, second_file_transfer, third_content); marieRoom.reset(); // Avoid bad weak ptr when the core is destroyed below this line. - bctbx_list_free_with_data(contents, (void (*)(void *))linphone_content_unref); linphone_core_manager_destroy(marie); linphone_core_manager_destroy(pauline); -- GitLab