You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2020/06/16 09:04:04 UTC

[GitHub] [nifi-minifi-cpp] hunyadi-dev opened a new pull request #816: MINIFICPP-1261 - Refactor non-trivial usages of ScopeGuard class

hunyadi-dev opened a new pull request #816:
URL: https://github.com/apache/nifi-minifi-cpp/pull/816


   Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
   
   In order to streamline the review of the contribution we ask you
   to ensure the following steps have been taken:
   
   ### For all changes:
   - [ ] Is there a JIRA ticket associated with this PR? Is it referenced
        in the commit message?
   
   - [ ] Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
   
   - [ ] Has your PR been rebased against the latest commit within the target branch (typically master)?
   
   - [ ] Is your initial contribution a single, squashed commit?
   
   ### For code changes:
   - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
   - [ ] If applicable, have you updated the LICENSE file?
   - [ ] If applicable, have you updated the NOTICE file?
   
   ### For documentation related changes:
   - [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
   
   ### Note:
   Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #816: MINIFICPP-1261 - Refactor non-trivial usages of ScopeGuard class

Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #816:
URL: https://github.com/apache/nifi-minifi-cpp/pull/816#discussion_r446966421



##########
File path: libminifi/include/utils/ScopeGuard.h
##########
@@ -33,10 +33,6 @@ namespace utils {
 
 struct ScopeGuard : ::gsl::final_action<std::function<void()>> {
   using ::gsl::final_action<std::function<void()>>::final_action;
-
-  void disable() noexcept {
-    dismiss();
-  }

Review comment:
       Ok. I didn't think this was harmful, but if you think so, then deprecation sounds like a good idea.




----------------------------------------------------------------
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:
users@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

Posted by GitBox <gi...@apache.org>.
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<std::function<void()>> {
   using ::gsl::final_action<std::function<void()>>::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:
users@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

Posted by GitBox <gi...@apache.org>.
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([&stored_database_]() {
-    delete stored_database_;
-  });
+  std::unique_ptr<rocksdb::DB> 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, &stored_database_);
+    rocksdb::Status status = rocksdb::DB::OpenForReadOnly(options, FLOWFILE_CHECKPOINT_DIRECTORY, &used_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:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] szaszm commented on pull request #816: MINIFICPP-1261 - Refactor non-trivial usages of ScopeGuard class

Posted by GitBox <gi...@apache.org>.
szaszm commented on pull request #816:
URL: https://github.com/apache/nifi-minifi-cpp/pull/816#issuecomment-644736318


   The CI failures look legit and relevant


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] arpadboda closed pull request #816: MINIFICPP-1261 - Refactor non-trivial usages of ScopeGuard class

Posted by GitBox <gi...@apache.org>.
arpadboda closed pull request #816:
URL: https://github.com/apache/nifi-minifi-cpp/pull/816


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #816: MINIFICPP-1261 - Refactor non-trivial usages of ScopeGuard class

Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #816:
URL: https://github.com/apache/nifi-minifi-cpp/pull/816#discussion_r446965308



##########
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<logging::Logger> logger_;
   std::shared_ptr<Configure> configure_;
   std::shared_ptr<minifi::controllers::SSLContextService> ssl_service_;
-  SSL_CTX *ctx;
+  using Context = std::unique_ptr<SSL_CTX, decltype(&deleteContext)>;
+  Context ctx;

Review comment:
       Not a big deal, just my opinion/preference. I prefer to use convenience aliases only if they bring a large benefit. This is because in the past I was working with a codebase that really overused type aliases and I had no idea what's what while reading the code.




----------------------------------------------------------------
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:
users@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

Posted by GitBox <gi...@apache.org>.
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, &passphrase);
+      SSL_CTX_set_default_passwd_cb(local_context.get(), io::tls::pemPassWordCb);
+      SSL_CTX_set_default_passwd_cb_userdata(local_context.get(), &passphrase);
     }
 
-    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:
users@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

Posted by GitBox <gi...@apache.org>.
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<std::function<void()>> {
   using ::gsl::final_action<std::function<void()>>::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:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] arpadboda commented on a change in pull request #816: MINIFICPP-1261 - Refactor non-trivial usages of ScopeGuard class

Posted by GitBox <gi...@apache.org>.
arpadboda commented on a change in pull request #816:
URL: https://github.com/apache/nifi-minifi-cpp/pull/816#discussion_r443518040



##########
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([&stored_database_]() {
-    delete stored_database_;
-  });
+  std::unique_ptr<rocksdb::DB> 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, &stored_database_);
+    rocksdb::Status status = rocksdb::DB::OpenForReadOnly(options, FLOWFILE_CHECKPOINT_DIRECTORY, &used_database);
+    stored_database.reset(used_database);

Review comment:
       This makes few sense to me to do here in case we release 4 lines later. 
   I would suggest doing it only when status is ok. Might be a bit more lines, but increases readability.




----------------------------------------------------------------
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:
users@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

Posted by GitBox <gi...@apache.org>.
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<logging::Logger> logger_;
   std::shared_ptr<Configure> configure_;
   std::shared_ptr<minifi::controllers::SSLContextService> ssl_service_;
-  SSL_CTX *ctx;
+  using Context = std::unique_ptr<SSL_CTX, decltype(&deleteContext)>;
+  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:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #816: MINIFICPP-1261 - Refactor non-trivial usages of ScopeGuard class

Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #816:
URL: https://github.com/apache/nifi-minifi-cpp/pull/816#discussion_r446315793



##########
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<logging::Logger> logger_;
   std::shared_ptr<Configure> configure_;
   std::shared_ptr<minifi::controllers::SSLContextService> ssl_service_;
-  SSL_CTX *ctx;
+  using Context = std::unique_ptr<SSL_CTX, decltype(&deleteContext)>;
+  Context ctx;

Review comment:
       I would localize the context type alias to the implementation, so that pointer semantics are obvious when looking at the declaration. This is probably very subjective because no guidelines come to my mind to support this.
   ```suggestion
     std::unique_ptr<SSL_CTX, decltype(&deleteContext)> ctx;
   ```

##########
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:
       I think a move assignment would be more clear and would expose potential lifetime issues earlier. (Unless it's implemented in terms of `swap`.)
   ```suggestion
         ctx = std::move(local_context);
   ```

##########
File path: libminifi/include/utils/ScopeGuard.h
##########
@@ -33,10 +33,6 @@ namespace utils {
 
 struct ScopeGuard : ::gsl::final_action<std::function<void()>> {
   using ::gsl::final_action<std::function<void()>>::final_action;
-
-  void disable() noexcept {
-    dismiss();
-  }

Review comment:
       I wouldn't remove this just for the sake of removing it, because external code might depend on it, and it's not a huge maintenance burden to keep it.

##########
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, &passphrase);
+      SSL_CTX_set_default_passwd_cb(local_context.get(), io::tls::pemPassWordCb);
+      SSL_CTX_set_default_passwd_cb_userdata(local_context.get(), &passphrase);
     }
 
-    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:
       Same as above, move assignment feels like it would be a more accurate description of intent.
   ```suggestion
     ctx = std::move(local_context);
   ```

##########
File path: libminifi/src/controllers/SSLContextService.cpp
##########
@@ -56,30 +56,30 @@ void SSLContextService::initialize() {
 bool SSLContextService::configure_ssl_context(SSL_CTX *ctx) {
   if (!IsNullOrEmpty(certificate)) {
     if (isFileTypeP12(certificate)) {
-      BIO* fp = BIO_new(BIO_s_file());
+      auto fp_deleter = [](BIO* ptr) { BIO_free(ptr); };
+      std::unique_ptr<BIO, decltype(fp_deleter)> fp(BIO_new(BIO_s_file()), fp_deleter);
       if (fp == nullptr) {
         logging::LOG_ERROR(logger_) << "Failed create new file BIO, " << getLatestOpenSSLErrorString();
         return false;
       }
-      utils::ScopeGuard fp_guard([fp]() { BIO_free(fp); });
-      if (BIO_read_filename(fp, certificate.c_str()) <= 0) {
+      if (BIO_read_filename(fp.get(), certificate.c_str()) <= 0) {
         logging::LOG_ERROR(logger_) << "Failed to read certificate file " << certificate << ", " << getLatestOpenSSLErrorString();
         return false;
       }
-      PKCS12* p12 = d2i_PKCS12_bio(fp, nullptr);
+      auto p12_deleter = [](PKCS12* ptr) { PKCS12_free(ptr); };
+      std::unique_ptr<PKCS12, decltype(p12_deleter)> p12(d2i_PKCS12_bio(fp.get(), nullptr), p12_deleter);
       if (p12 == nullptr) {
         logging::LOG_ERROR(logger_) << "Failed to DER decode certificate file " << certificate << ", " << getLatestOpenSSLErrorString();
         return false;
       }
-      utils::ScopeGuard p12_guard([p12]() { PKCS12_free(p12); });
       EVP_PKEY* pkey = nullptr;
       X509* cert = nullptr;
       STACK_OF(X509)* ca = nullptr;
-      if (!PKCS12_parse(p12, passphrase_.c_str(), &pkey, &cert, &ca)) {
+      if (!PKCS12_parse(p12.get(), passphrase_.c_str(), &pkey, &cert, &ca)) {
         logging::LOG_ERROR(logger_) << "Failed to parse certificate file " << certificate << " as PKCS#12, " << getLatestOpenSSLErrorString();
         return false;
       }
-      utils::ScopeGuard certs_guard([pkey, cert, ca]() {
+      ::gsl::final_action<std::function<void()>> certs_guard([pkey, cert, ca]() {

Review comment:
       Please use `auto` and `gsl::finally`. It's not recommended (by gsl-lite) to depend on the global `gsl` namespace, as they want to eventually move to the `gsl_lite` namespace. `ScopeGuard` has this dependency to greatly simplify the implementation, but here it doesn't provide a benefit.
   ```suggestion
         const auto certs_guard = gsl::finally([pkey, cert, ca]() {
   ```




----------------------------------------------------------------
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:
users@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

Posted by GitBox <gi...@apache.org>.
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<logging::Logger> logger_;
   std::shared_ptr<Configure> configure_;
   std::shared_ptr<minifi::controllers::SSLContextService> ssl_service_;
-  SSL_CTX *ctx;
+  using Context = std::unique_ptr<SSL_CTX, decltype(&deleteContext)>;
+  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:
users@infra.apache.org