[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #816: MINIFICPP-1261 - Refactor non-trivial usages of ScopeGuard class
hunyadi-dev commented on a change in pull request #816:
URL: https://github.com/apache/nifi-minifi-cpp/pull/816#discussion_r446841984
##
File path: libminifi/include/io/tls/TLSSocket.h
##
@@ -80,10 +77,13 @@ class TLSContext : public SocketContext {
int16_t initialize(bool server_method = false);
private:
+ static void deleteContext(SSL_CTX* ptr) { SSL_CTX_free(ptr); }
+
std::shared_ptr logger_;
std::shared_ptr configure_;
std::shared_ptr ssl_service_;
- SSL_CTX *ctx;
+ using Context = std::unique_ptr;
+ Context ctx;
Review comment:
I am using this for declaring a local context in
`TLSContext::initialize` that is why I aliased it. Will type it out instead, if
you prefer that.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #816: MINIFICPP-1261 - Refactor non-trivial usages of ScopeGuard class
hunyadi-dev commented on a change in pull request #816:
URL: https://github.com/apache/nifi-minifi-cpp/pull/816#discussion_r446854170
##
File path: libminifi/src/io/tls/TLSSocket.cpp
##
@@ -120,35 +115,35 @@ int16_t TLSContext::initialize(bool server_method) {
file.close();
passphrase = password;
}
- SSL_CTX_set_default_passwd_cb(ctx, io::tls::pemPassWordCb);
- SSL_CTX_set_default_passwd_cb_userdata(ctx, );
+ SSL_CTX_set_default_passwd_cb(local_context.get(),
io::tls::pemPassWordCb);
+ SSL_CTX_set_default_passwd_cb_userdata(local_context.get(), );
}
-int retp = SSL_CTX_use_PrivateKey_file(ctx, privatekey.c_str(),
SSL_FILETYPE_PEM);
+int retp = SSL_CTX_use_PrivateKey_file(local_context.get(),
privatekey.c_str(), SSL_FILETYPE_PEM);
if (retp != 1) {
logger_->log_error("Could not create load private key,%i on %s error :
%s", retp, privatekey, std::strerror(errno));
error_value = TLS_ERROR_KEY_ERROR;
return error_value;
}
// verify private key
-if (!SSL_CTX_check_private_key(ctx)) {
+if (!SSL_CTX_check_private_key(local_context.get())) {
logger_->log_error("Private key does not match the public certificate,
error : %s", std::strerror(errno));
error_value = TLS_ERROR_KEY_ERROR;
return error_value;
}
// load CA certificates
if (ssl_service_ != nullptr ||
configure_->get(Configure::nifi_security_client_ca_certificate, caCertificate))
{
- retp = SSL_CTX_load_verify_locations(ctx, caCertificate.c_str(), 0);
+ retp = SSL_CTX_load_verify_locations(local_context.get(),
caCertificate.c_str(), 0);
if (retp == 0) {
logger_->log_error("Can not load CA certificate, Exiting, error : %s",
std::strerror(errno));
error_value = TLS_ERROR_CERT_ERROR;
return error_value;
}
}
-logger_->log_debug("Load/Verify Client Certificate OK. for %X and %X",
this, ctx);
+logger_->log_debug("Load/Verify Client Certificate OK. for %X and %X",
this, local_context.get());
}
- ctxGuard.disable();
+ ctx.swap(local_context);
Review comment:
Adding manually, as I do not want to split the change into two commits.
The other suggestion has been applied.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #816: MINIFICPP-1261 - Refactor non-trivial usages of ScopeGuard class
hunyadi-dev commented on a change in pull request #816:
URL: https://github.com/apache/nifi-minifi-cpp/pull/816#discussion_r446848748
##
File path: libminifi/include/utils/ScopeGuard.h
##
@@ -33,10 +33,6 @@ namespace utils {
struct ScopeGuard : ::gsl::final_action> {
using ::gsl::final_action>::final_action;
-
- void disable() noexcept {
-dismiss();
- }
Review comment:
I do not like this pattern being present in the codebase at all.
Restored, but marked for depracation ASAP (1.0).
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #816: MINIFICPP-1261 - Refactor non-trivial usages of ScopeGuard class
hunyadi-dev commented on a change in pull request #816:
URL: https://github.com/apache/nifi-minifi-cpp/pull/816#discussion_r446848748
##
File path: libminifi/include/utils/ScopeGuard.h
##
@@ -33,10 +33,6 @@ namespace utils {
struct ScopeGuard : ::gsl::final_action> {
using ::gsl::final_action>::final_action;
-
- void disable() noexcept {
-dismiss();
- }
Review comment:
I do not like this pattern being present in the codebase at all.
Restored, but marked for depracation ASAP (to be removed in 1.0).
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #816: MINIFICPP-1261 - Refactor non-trivial usages of ScopeGuard class
hunyadi-dev commented on a change in pull request #816:
URL: https://github.com/apache/nifi-minifi-cpp/pull/816#discussion_r446841976
##
File path: libminifi/src/io/tls/TLSSocket.cpp
##
@@ -72,30 +72,25 @@ int16_t TLSContext::initialize(bool server_method) {
}
const SSL_METHOD *method;
method = server_method ? TLSv1_2_server_method() : TLSv1_2_client_method();
- ctx = SSL_CTX_new(method);
- if (ctx == nullptr) {
+ Context local_context = Context(SSL_CTX_new(method), deleteContext);
+ if (local_context == nullptr) {
logger_->log_error("Could not create SSL context, error: %s.",
std::strerror(errno));
error_value = TLS_ERROR_CONTEXT;
return error_value;
}
- utils::ScopeGuard ctxGuard([this]() {
-SSL_CTX_free(ctx);
-ctx = nullptr;
- });
-
if (needClientCert) {
std::string certificate;
std::string privatekey;
std::string passphrase;
std::string caCertificate;
if (ssl_service_ != nullptr) {
- if (!ssl_service_->configure_ssl_context(ctx)) {
+ if (!ssl_service_->configure_ssl_context(local_context.get())) {
error_value = TLS_ERROR_CERT_ERROR;
return error_value;
}
- ctxGuard.disable();
+ ctx.swap(local_context);
Review comment:
Adding manually, as I do not want to split the change into two commits.
The other suggestion has been applied.
##
File path: libminifi/include/io/tls/TLSSocket.h
##
@@ -80,10 +77,13 @@ class TLSContext : public SocketContext {
int16_t initialize(bool server_method = false);
private:
+ static void deleteContext(SSL_CTX* ptr) { SSL_CTX_free(ptr); }
+
std::shared_ptr logger_;
std::shared_ptr configure_;
std::shared_ptr ssl_service_;
- SSL_CTX *ctx;
+ using Context = std::unique_ptr;
+ Context ctx;
Review comment:
Adding manually, as I do not want to split the change into two commits.
The other suggestion has been applied.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #816: MINIFICPP-1261 - Refactor non-trivial usages of ScopeGuard class
hunyadi-dev commented on a change in pull request #816:
URL: https://github.com/apache/nifi-minifi-cpp/pull/816#discussion_r446055811
##
File path: extensions/rocksdb-repos/FlowFileRepository.cpp
##
@@ -122,27 +122,26 @@ void FlowFileRepository::run() {
}
void FlowFileRepository::prune_stored_flowfiles() {
- rocksdb::DB* stored_database_ = nullptr;
- utils::ScopeGuard db_guard([_database_]() {
-delete stored_database_;
- });
+ std::unique_ptr stored_database;
+ rocksdb::DB* used_database;
bool corrupt_checkpoint = false;
if (nullptr != checkpoint_) {
rocksdb::Options options;
options.create_if_missing = true;
options.use_direct_io_for_flush_and_compaction = true;
options.use_direct_reads = true;
-rocksdb::Status status = rocksdb::DB::OpenForReadOnly(options,
FLOWFILE_CHECKPOINT_DIRECTORY, _database_);
+rocksdb::Status status = rocksdb::DB::OpenForReadOnly(options,
FLOWFILE_CHECKPOINT_DIRECTORY, _database);
+stored_database.reset(used_database);
Review comment:
Changed as requested.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
