From d74b5fc6f83703d040283f5d93e03043a9cde35d Mon Sep 17 00:00:00 2001
From: Sergey Yakubov <sergey.yakubov@desy.de>
Date: Wed, 23 Jun 2021 14:53:51 +0200
Subject: [PATCH] refactor and fix memleak
---
consumer/api/cpp/src/consumer_c_glue.cpp | 79 +++++++++----------
.../api/cpp/include/asapo/producer/producer.h | 2 +-
.../consumer/consumer_api/consumer_api.c | 3 +-
3 files changed, 41 insertions(+), 43 deletions(-)
diff --git a/consumer/api/cpp/src/consumer_c_glue.cpp b/consumer/api/cpp/src/consumer_c_glue.cpp
index 25278ff0b..4a618a16f 100644
--- a/consumer/api/cpp/src/consumer_c_glue.cpp
+++ b/consumer/api/cpp/src/consumer_c_glue.cpp
@@ -121,6 +121,19 @@ int process_error(AsapoErrorHandle* error, asapo::Error err,
return retval;
}
+AsapoHandle* handle_or_null(AsapoHandle* handle, AsapoErrorHandle* error, asapo::Error err,
+ const asapo::ErrorTemplateInterface* p_exclude_err_template = nullptr) {
+ if (process_error(error, std::move(err),p_exclude_err_template) < 0) {
+ if (handle!=nullptr) {
+ delete handle;
+ }
+ return nullptr;
+ } else {
+ return handle;
+ }
+}
+
+
#define dataGetterStart \
asapo::MessageData d; \
asapo::MessageMeta* fi = info ? new asapo::MessageMeta : nullptr; \
@@ -214,8 +227,8 @@ extern "C" {
has_filesysytem,
*(source->handle),
&err);
- process_error(error, std::move(err));
- return new AsapoHandlerHolder<asapo::Consumer>(c.release());
+ auto retval = new AsapoHandlerHolder<asapo::Consumer>(c.release());
+ return static_cast<AsapoConsumerHandle>(handle_or_null(retval, error, std::move(err)));
}
//! wraps asapo::Consumer::GenerateNewGroupId()
@@ -227,10 +240,8 @@ extern "C" {
AsapoErrorHandle* error) {
asapo::Error err;
auto result = new std::string(consumer->handle->GenerateNewGroupId(&err));
- if (process_error(error, std::move(err)) < 0) {
- return nullptr;
- }
- return new AsapoHandlerHolder<std::string> {result};
+ auto retval = new AsapoHandlerHolder<std::string> {result};
+ return static_cast<AsapoStringHandle>(handle_or_null(retval, error, std::move(err)));
}
//! give a pointer to the content of the asapoString
@@ -323,10 +334,8 @@ extern "C" {
from_id, to_id,
stream,
&err));
- if (process_error(error, std::move(err)) < 0) {
- return nullptr;
- }
- return new AsapoHandlerHolder<asapo::IdList> {list};
+ auto retval = new AsapoHandlerHolder<asapo::IdList> {list};
+ return static_cast<AsapoIdListHandle>(handle_or_null(retval, error, std::move(err)));
}
//! give number of items in an id list
@@ -368,10 +377,8 @@ extern "C" {
auto info = new asapo::StreamInfos(consumer->handle->GetStreamList(from,
static_cast<asapo::StreamFilter>(filter),
&err));
- if (process_error(error, std::move(err)) < 0) {
- return nullptr;
- }
- return new AsapoHandlerHolder<asapo::StreamInfos> {info};
+ auto retval = new AsapoHandlerHolder<asapo::StreamInfos> {info};
+ return static_cast<AsapoStreamInfosHandle>(handle_or_null(retval, error, std::move(err)));
}
//! get one stream info from a stream infos handle
@@ -443,12 +450,10 @@ extern "C" {
AsapoStringHandle asapo_consumer_get_beamtime_meta(AsapoConsumerHandle consumer,
AsapoErrorHandle* error) {
asapo::Error err;
- auto retval = new std::string(consumer->handle->GetBeamtimeMeta(&err));
- if (process_error(error, std::move(err)) < 0) {
- return nullptr;
- }
- return new AsapoHandlerHolder<std::string> {retval};
- }
+ auto result = new std::string(consumer->handle->GetBeamtimeMeta(&err));
+ auto retval = new AsapoHandlerHolder<std::string> {result};
+ return static_cast<AsapoStringHandle>(handle_or_null(retval, error, std::move(err)));
+}
//! wraps asapo::Consumer::RetrieveData()
/// \copydoc asapo::Consumer::RetrieveData()
@@ -479,11 +484,9 @@ extern "C" {
const char* stream,
AsapoErrorHandle* error) {
asapo::Error err;
- auto retval = new asapo::DataSet(consumer->handle->GetNextDataset(*group_id->handle, min_size, stream, &err));
- if (process_error(error, std::move(err), &asapo::ConsumerErrorTemplates::kPartialData) < 0) {
- return nullptr;
- }
- return new AsapoHandlerHolder<asapo::DataSet> {retval};
+ auto result = new asapo::DataSet(consumer->handle->GetNextDataset(*group_id->handle, min_size, stream, &err));
+ auto retval = new AsapoHandlerHolder<asapo::DataSet> {result};
+ return static_cast<AsapoDataSetHandle>(handle_or_null(retval, error, std::move(err),&asapo::ConsumerErrorTemplates::kPartialData));
}
//! wraps asapo::Consumer::GetLastDataset()
@@ -495,12 +498,10 @@ extern "C" {
const char* stream,
AsapoErrorHandle* error) {
asapo::Error err;
- auto retval = new asapo::DataSet(consumer->handle->GetLastDataset(min_size, stream, &err));
- if (process_error(error, std::move(err), &asapo::ConsumerErrorTemplates::kPartialData) < 0) {
- return nullptr;
- }
- return new AsapoHandlerHolder<asapo::DataSet> {retval};
- }
+ auto result = new asapo::DataSet(consumer->handle->GetLastDataset(min_size, stream, &err));
+ auto retval = new AsapoHandlerHolder<asapo::DataSet> {result};
+ return static_cast<AsapoDataSetHandle>(handle_or_null(retval, error, std::move(err),&asapo::ConsumerErrorTemplates::kPartialData));
+}
//! wraps asapo::Consumer::GetLastAcknowledgedMessage()
/// \copydoc asapo::Consumer::GetLastAcknowledgedMessage()
@@ -528,11 +529,9 @@ extern "C" {
const char* stream,
AsapoErrorHandle* error) {
asapo::Error err;
- auto retval = new asapo::DataSet(consumer->handle->GetDatasetById(id, min_size, stream, &err));
- if (process_error(error, std::move(err), &asapo::ConsumerErrorTemplates::kPartialData) < 0) {
- return nullptr;
- }
- return new AsapoHandlerHolder<asapo::DataSet> {retval};
+ auto result = new asapo::DataSet(consumer->handle->GetDatasetById(id, min_size, stream, &err));
+ auto retval = new AsapoHandlerHolder<asapo::DataSet> {result};
+ return static_cast<AsapoDataSetHandle>(handle_or_null(retval, error, std::move(err),&asapo::ConsumerErrorTemplates::kPartialData));
}
//! wraps aspao::Consumer::GetById()
@@ -592,11 +591,9 @@ extern "C" {
const char* stream,
AsapoErrorHandle* error) {
asapo::Error err;
- auto retval = new asapo::MessageMetas(consumer->handle->QueryMessages(query, stream, &err));
- if (process_error(error, std::move(err)) < 0) {
- return nullptr;
- }
- return new AsapoHandlerHolder<asapo::MessageMetas> {retval};
+ auto result = new asapo::MessageMetas(consumer->handle->QueryMessages(query, stream, &err));
+ auto retval = new AsapoHandlerHolder<asapo::MessageMetas> {result};
+ return static_cast<AsapoMessageMetasHandle>(handle_or_null(retval, error, std::move(err)));
}
//! wraps aspao::Consumer::SetResendNacs()
diff --git a/producer/api/cpp/include/asapo/producer/producer.h b/producer/api/cpp/include/asapo/producer/producer.h
index 6e35e8b82..faa23467f 100644
--- a/producer/api/cpp/include/asapo/producer/producer.h
+++ b/producer/api/cpp/include/asapo/producer/producer.h
@@ -69,7 +69,7 @@ class Producer {
RequestCallback callback) = 0;
- //! Sends data to the receiver - same as Send - memory should not be freed until send is finished
+ //! Sends data to the receiver - same as Send - memory should not be freed after send is finished
//! used e.g. for Python bindings
virtual Error Send__(const MessageHeader& message_header,
void* data,
diff --git a/tests/automatic/consumer/consumer_api/consumer_api.c b/tests/automatic/consumer/consumer_api/consumer_api.c
index 8bac3bdda..1e2bb1115 100644
--- a/tests/automatic/consumer/consumer_api/consumer_api.c
+++ b/tests/automatic/consumer/consumer_api/consumer_api.c
@@ -92,7 +92,8 @@ void test_datasets(AsapoConsumerHandle consumer, AsapoStringHandle group_id) {
asapo_free_handle(&dataset);
// get last incomplete datasets without min_size
- asapo_consumer_get_last_dataset(consumer, 0, "incomplete", &err);
+ AsapoDataSetHandle ds = asapo_consumer_get_last_dataset(consumer, 0, "incomplete", &err);
+ ASSERT_TRUE(ds == NULL,"returns null in case of error");
ASSERT_TRUE(asapo_error_get_type(err) == kEndOfStream,"incomplete dataset end of stream error");
// get dataset by id incomplete datasets without min_size
--
GitLab