From 473e5532221e031f8723fc2c5b2267fc72c5d7cc Mon Sep 17 00:00:00 2001
From: florian <f.spreckelsen@inidscale.com>
Date: Thu, 12 Aug 2021 11:16:55 +0200
Subject: [PATCH] DRAFT: Include a `_deletable` flag
---
include/ccaosdb.h | 10 ++++++
src/ccaosdb.cpp | 83 ++++++++++++++++++++++++++++++++-----------
test/test_ccaosdb.cpp | 16 ++++-----
3 files changed, 80 insertions(+), 29 deletions(-)
diff --git a/include/ccaosdb.h b/include/ccaosdb.h
index a0643a5..027a102 100644
--- a/include/ccaosdb.h
+++ b/include/ccaosdb.h
@@ -61,6 +61,7 @@ const char *caosdb_constants_COMPATIBLE_SERVER_VERSION_PRE_RELEASE();
*/
typedef struct {
void *wrapped_connection;
+ bool _deletable = false;
} caosdb_connection_connection;
/**
@@ -72,6 +73,7 @@ typedef struct {
*/
typedef struct {
void *wrapped_connection_configuration;
+ bool _deletable = false;
} caosdb_connection_connection_configuration;
/**
@@ -91,10 +93,12 @@ typedef struct {
typedef struct {
void *wrapped_certificate_provider;
+ bool _deletable = false;
} caosdb_connection_certificate_provider;
typedef struct {
void *wrapped_authenticator;
+ bool _deletable = false;
} caosdb_authentication_authenticator;
/**
@@ -263,6 +267,7 @@ int caosdb_connection_connection_manager_get_connection(
// not sufficient yet.
typedef struct {
void *wrapped_transaction;
+ bool _deletable = false;
} caosdb_transaction_transaction;
/**
@@ -289,6 +294,7 @@ int caosdb_transaction_transaction_execute(
typedef struct {
void *wrapped_result_set;
+ bool _deletable = false;
} caosdb_transaction_result_set;
int caosdb_transaction_transaction_get_result_set(
@@ -300,6 +306,7 @@ int caosdb_transaction_transaction_get_count_result(
typedef struct {
void *wrapped_entity;
+ bool _deletable = false;
} caosdb_entity_entity;
int caosdb_transaction_result_set_at(caosdb_transaction_result_set *result_set,
@@ -309,12 +316,15 @@ int caosdb_transaction_result_set_size(
typedef struct {
void *wrapped_property;
+ bool _deletable = false;
} caosdb_entity_property;
typedef struct {
void *wrapped_parent;
+ bool _deletable = false;
} caosdb_entity_parent;
typedef struct {
void *wrapped_message;
+ bool _deletable = false;
} caosdb_entity_message;
// GETTERS FOR EVERYTHING
diff --git a/src/ccaosdb.cpp b/src/ccaosdb.cpp
index 4c9a4f0..23a4dc2 100644
--- a/src/ccaosdb.cpp
+++ b/src/ccaosdb.cpp
@@ -180,6 +180,7 @@ ERROR_RETURN_CODE(GENERIC_ERROR,
out->wrapped_certificate_provider =
new caosdb::configuration::PemFileCertificateProvider(
std::string(path));
+ out->_deletable = true;
return 0;
})
@@ -188,8 +189,11 @@ ERROR_RETURN_CODE(
int caosdb_connection_delete_certificate_provider(
caosdb_connection_certificate_provider *provider),
{
- delete static_cast<caosdb::configuration::CertificateProvider *>(
- provider->wrapped_certificate_provider);
+ if (provider->_deletable) {
+ delete static_cast<caosdb::configuration::CertificateProvider *>(
+ provider->wrapped_certificate_provider);
+ }
+ provider->_deletable = false;
return 0;
})
@@ -201,17 +205,22 @@ ERROR_RETURN_CODE(GENERIC_ERROR,
out->wrapped_authenticator =
new caosdb::authentication::PlainPasswordAuthenticator(
std::string(username), std::string(password));
+ out->_deletable = true;
return 0;
})
-ERROR_RETURN_CODE(GENERIC_ERROR,
- int caosdb_authentication_delete_authenticator(
- caosdb_authentication_authenticator *authenticator),
- {
- delete static_cast<caosdb::authentication::Authenticator *>(
- authenticator->wrapped_authenticator);
- return 0;
- })
+ERROR_RETURN_CODE(
+ GENERIC_ERROR,
+ int caosdb_authentication_delete_authenticator(
+ caosdb_authentication_authenticator *authenticator),
+ {
+ if (authenticator->_deletable) {
+ delete static_cast<caosdb::authentication::Authenticator *>(
+ authenticator->wrapped_authenticator);
+ }
+ authenticator->_deletable = false;
+ return 0;
+ })
ERROR_RETURN_CODE(
GENERIC_ERROR,
@@ -249,6 +258,7 @@ ERROR_RETURN_CODE(
out->wrapped_connection_configuration =
new caosdb::configuration::TlsConnectionConfiguration(host_str, port);
}
+ out->_deletable = true;
return 0;
})
@@ -260,6 +270,7 @@ ERROR_RETURN_CODE(
{
out->wrapped_connection_configuration =
new caosdb::configuration::InsecureConnectionConfiguration(host, port);
+ out->_deletable = true;
return 0;
})
@@ -268,8 +279,11 @@ ERROR_RETURN_CODE(
int caosdb_connection_delete_connection_configuration(
caosdb_connection_connection_configuration *configuration),
{
- delete static_cast<caosdb::configuration::ConnectionConfiguration *>(
- configuration->wrapped_connection_configuration);
+ if (configuration->_deletable) {
+ delete static_cast<caosdb::configuration::ConnectionConfiguration *>(
+ configuration->wrapped_connection_configuration);
+ }
+ configuration->_deletable = false;
return 0;
})
@@ -283,6 +297,7 @@ ERROR_RETURN_CODE(
static_cast<caosdb::configuration::ConnectionConfiguration *>(
configuration->wrapped_connection_configuration);
out->wrapped_connection = new caosdb::connection::Connection(*config);
+ out->_deletable = true;
return 0;
})
@@ -290,8 +305,11 @@ ERROR_RETURN_CODE(GENERIC_ERROR,
int caosdb_connection_delete_connection(
caosdb_connection_connection *connection),
{
- delete static_cast<caosdb::connection::Connection *>(
- connection->wrapped_connection);
+ if (connection->_deletable) {
+ delete static_cast<caosdb::connection::Connection *>(
+ connection->wrapped_connection);
+ }
+ connection->_deletable = false;
return 0;
})
@@ -347,6 +365,9 @@ ERROR_RETURN_CODE(GENERIC_ERROR,
caosdb::connection::ConnectionManager::GetConnection(
std::string(name))
.get();
+ // managed by the connection manager now, so not
+ // to be deleted manually
+ out->_deletable = false;
return 0;
})
@@ -363,6 +384,7 @@ ERROR_RETURN_CODE(GENERIC_ERROR,
connection->wrapped_connection);
out->wrapped_transaction =
wrapped_connection->CreateTransaction().release();
+ out->_deletable = true;
return 0;
})
@@ -370,8 +392,10 @@ ERROR_RETURN_CODE(GENERIC_ERROR,
int caosdb_transaction_delete_transaction(
caosdb_transaction_transaction *transaction),
{
- delete static_cast<caosdb::transaction::Transaction *>(
- transaction->wrapped_transaction);
+ if (transaction->_deletable) {
+ delete static_cast<caosdb::transaction::Transaction *>(
+ transaction->wrapped_transaction);
+ }
return 0;
})
@@ -476,13 +500,17 @@ ERROR_RETURN_CODE(GENERIC_ERROR,
ERROR_RETURN_CODE(GENERIC_ERROR,
int caosdb_entity_create_entity(caosdb_entity_entity *out), {
out->wrapped_entity = new caosdb::entity::Entity();
+ out->_deletable = true;
return 0;
})
ERROR_RETURN_CODE(GENERIC_ERROR,
int caosdb_entity_delete_entity(caosdb_entity_entity *out), {
- delete static_cast<caosdb::entity::Entity *>(
- out->wrapped_entity);
+ if (out->_deletable) {
+ delete static_cast<caosdb::entity::Entity *>(
+ out->wrapped_entity);
+ }
+ out->_deletable = false;
return 0;
})
@@ -490,26 +518,34 @@ ERROR_RETURN_CODE(
GENERIC_ERROR, int caosdb_entity_create_property(caosdb_entity_property *out),
{
out->wrapped_property = new caosdb::entity::Property();
+ out->_deletable = true;
return 0;
})
ERROR_RETURN_CODE(
GENERIC_ERROR, int caosdb_entity_delete_property(caosdb_entity_property *out),
{
- delete static_cast<caosdb::entity::Property *>(out->wrapped_property);
+ if (out->_deletable) {
+ delete static_cast<caosdb::entity::Property *>(out->wrapped_property);
+ }
+ out->_deletable = false;
return 0;
})
ERROR_RETURN_CODE(GENERIC_ERROR,
int caosdb_entity_create_parent(caosdb_entity_parent *out), {
out->wrapped_parent = new caosdb::entity::Parent();
+ out->_deletable = true;
return 0;
})
ERROR_RETURN_CODE(GENERIC_ERROR,
int caosdb_entity_delete_parent(caosdb_entity_parent *out), {
- delete static_cast<caosdb::entity::Parent *>(
- out->wrapped_parent);
+ if (out->_deletable) {
+ delete static_cast<caosdb::entity::Parent *>(
+ out->wrapped_parent);
+ }
+ out->_deletable = false;
return 0;
})
@@ -543,6 +579,7 @@ ERROR_RETURN_CODE(
auto *wrapped_entity =
static_cast<caosdb::entity::Entity *>(entity->wrapped_entity);
out->wrapped_message = wrapped_entity->GetErrors().mutable_at(index);
+ out->_deletable = false;
return 0;
})
@@ -565,6 +602,7 @@ ERROR_RETURN_CODE(
auto *wrapped_entity =
static_cast<caosdb::entity::Entity *>(entity->wrapped_entity);
out->wrapped_message = wrapped_entity->GetWarnings().mutable_at(index);
+ out->_deletable = false;
return 0;
})
@@ -587,6 +625,7 @@ ERROR_RETURN_CODE(
auto *wrapped_entity =
static_cast<caosdb::entity::Entity *>(entity->wrapped_entity);
out->wrapped_message = wrapped_entity->GetInfos().mutable_at(index);
+ out->_deletable = false;
return 0;
})
@@ -609,6 +648,7 @@ ERROR_RETURN_CODE(
auto *wrapped_entity =
static_cast<caosdb::entity::Entity *>(entity->wrapped_entity);
out->wrapped_property = wrapped_entity->GetProperties().mutable_at(index);
+ out->_deletable = false;
return 0;
})
@@ -631,6 +671,7 @@ ERROR_RETURN_CODE(
auto *wrapped_entity =
static_cast<caosdb::entity::Entity *>(entity->wrapped_entity);
out->wrapped_parent = wrapped_entity->GetParents().mutable_at(index);
+ out->_deletable = false;
return 0;
})
diff --git a/test/test_ccaosdb.cpp b/test/test_ccaosdb.cpp
index c957b3c..62ce930 100644
--- a/test/test_ccaosdb.cpp
+++ b/test/test_ccaosdb.cpp
@@ -324,13 +324,13 @@ TEST_F(test_ccaosdb, test_entity_with_parent_and_property) {
return_code = caosdb_entity_delete_entity(&entity);
EXPECT_EQ(return_code, 0);
- // TODO(fspreck) This fails, because the output_p{arent,roperty} is owned
- // by the entity (which is already deleted). Also, this shows that it is
- // dangerous to call the deletion of output_p{arent,roperty} before the
- // entity is being deleted, because it screws up the RepeatedPtrFieldWrapper.
+ // This tests the `_deletable` flag. The wrapped cpp objects of
+ // `output_parent` and `output_property` are owned by the entity, so
+ // they have been deleted together with the entity. With a wrong
+ // `_deletable` flag, the following would cause segfaults.
//
- // return_code = caosdb_entity_delete_parent(&output_parent);
- // EXPECT_EQ(return_code, 0);
- // return_code = caosdb_entity_delete_property(&output_property);
- // EXPECT_EQ(return_code, 0);
+ return_code = caosdb_entity_delete_parent(&output_parent);
+ EXPECT_EQ(return_code, 0);
+ return_code = caosdb_entity_delete_property(&output_property);
+ EXPECT_EQ(return_code, 0);
}
--
GitLab