Implementing further comments by Yang.

include/grpcpp/security/tls_credentials_options.h

@@ -118,9 +118,9 @@ class TlsCredentialReloadArg {
 struct TlsCredentialReloadInterface {
   virtual ~TlsCredentialReloadInterface() = default;
   /** A callback that invokes the credential reload. **/
-  virtual int Schedule(TlsCredentialReloadArg* arg) { return 1; }
+  virtual int Schedule(std::unique_ptr<TlsCredentialReloadArg>& arg) = 0;
   /** A callback that cancels a credential reload request. **/
-  virtual void Cancel(TlsCredentialReloadArg* arg) {}
+  virtual void Cancel(std::unique_ptr<TlsCredentialReloadArg>& arg) {}
   /** A callback that cleans up any data associated to the
    * interface or the config. It will be called when the config is no longer
    * using the interface. **/
@@ -136,11 +136,11 @@ class TlsCredentialReloadConfig {
                                 credential_reload_interface);
   ~TlsCredentialReloadConfig();
 
-  int Schedule(TlsCredentialReloadArg* arg) const {
+  int Schedule(std::unique_ptr<TlsCredentialReloadArg>& arg) const {
     return credential_reload_interface_->Schedule(arg);
   }
 
-  void Cancel(TlsCredentialReloadArg* arg) const {
+  void Cancel(std::unique_ptr<TlsCredentialReloadArg>& arg) const {
     credential_reload_interface_->Cancel(arg);
   }
 
@@ -208,12 +208,12 @@ class TlsServerAuthorizationCheckArg {
 struct TlsServerAuthorizationCheckInterface {
   virtual ~TlsServerAuthorizationCheckInterface() = default;
   /** A callback that invokes the server authorization check. **/
-  virtual int Schedule(TlsServerAuthorizationCheckArg* arg) { return 1; }
+  virtual int Schedule(std::unique_ptr<TlsServerAuthorizationCheckArg>& arg) = 0;
   /** A callback that cancels a server authorization check request. **/
-  virtual void Cancel(TlsServerAuthorizationCheckArg* arg){};
+  virtual void Cancel(std::unique_ptr<TlsServerAuthorizationCheckArg>& arg) {}
   /** A callback that cleans up any data associated to the
    * interface or the config. **/
-  virtual void Release(){};
+  virtual void Release() {}
 };
 
 /** TLS server authorization check config, wraps
@@ -228,11 +228,11 @@ class TlsServerAuthorizationCheckConfig {
           server_authorization_check_interface);
   ~TlsServerAuthorizationCheckConfig();
 
-  int Schedule(TlsServerAuthorizationCheckArg* arg) const {
+  int Schedule(std::unique_ptr<TlsServerAuthorizationCheckArg>& arg) const {
     return server_authorization_check_interface_->Schedule(arg);
   }
 
-  void Cancel(TlsServerAuthorizationCheckArg* arg) const {
+  void Cancel(std::unique_ptr<TlsServerAuthorizationCheckArg>& arg) const {
     server_authorization_check_interface_->Cancel(arg);
   }
 

src/cpp/common/tls_credentials_options.cc

@@ -96,7 +96,9 @@ TlsCredentialReloadConfig::TlsCredentialReloadConfig(
 }
 
 TlsCredentialReloadConfig::~TlsCredentialReloadConfig() {
-  credential_reload_interface_->Release();
+  if (credential_reload_interface_ != nullptr) {
+    credential_reload_interface_->Release();
+  }
 }
 
 /** gRPC TLS server authorization check arg API implementation **/
@@ -175,7 +177,9 @@ TlsServerAuthorizationCheckConfig::TlsServerAuthorizationCheckConfig(
 }
 
 TlsServerAuthorizationCheckConfig::~TlsServerAuthorizationCheckConfig() {
-  server_authorization_check_interface_->Release();
+  if (server_authorization_check_interface_ != nullptr) {
+    server_authorization_check_interface_->Release();
+  }
 }
 
 /** gRPC TLS credential options API implementation **/
@@ -193,9 +197,11 @@ TlsCredentialsOptions::TlsCredentialsOptions(
   c_credentials_options_ = grpc_tls_credentials_options_create();
   grpc_tls_credentials_options_set_cert_request_type(c_credentials_options_,
                                                      cert_request_type_);
-  grpc_tls_credentials_options_set_key_materials_config(
-      c_credentials_options_,
-      ConvertToCKeyMaterialsConfig(key_materials_config_));
+  if (key_materials_config_ != nullptr) {
+    grpc_tls_credentials_options_set_key_materials_config(
+        c_credentials_options_,
+        ConvertToCKeyMaterialsConfig(key_materials_config_));
+  }
   if (credential_reload_config_ != nullptr) {
     grpc_tls_credentials_options_set_credential_reload_config(
         c_credentials_options_, credential_reload_config_->c_config());

src/cpp/common/tls_credentials_options_util.cc

@@ -92,8 +92,8 @@ int TlsCredentialReloadConfigCSchedule(void* config_user_data,
   }
   TlsCredentialReloadConfig* cpp_config =
       static_cast<TlsCredentialReloadConfig*>(arg->config->context());
-  TlsCredentialReloadArg cpp_arg(arg);
-  return cpp_config->Schedule(&cpp_arg);
+  std::unique_ptr<TlsCredentialReloadArg> cpp_arg(new TlsCredentialReloadArg(arg));
+  return cpp_config->Schedule(cpp_arg);
 }
 
 void TlsCredentialReloadConfigCCancel(void* config_user_data,
@@ -105,8 +105,8 @@ void TlsCredentialReloadConfigCCancel(void* config_user_data,
   }
   TlsCredentialReloadConfig* cpp_config =
       static_cast<TlsCredentialReloadConfig*>(arg->config->context());
-  TlsCredentialReloadArg cpp_arg(arg);
-  cpp_config->Cancel(&cpp_arg);
+  std::unique_ptr<TlsCredentialReloadArg> cpp_arg(new TlsCredentialReloadArg(arg));
+  cpp_config->Cancel(cpp_arg);
 }
 
 /** The C schedule and cancel functions for the server authorization check
@@ -122,8 +122,8 @@ int TlsServerAuthorizationCheckConfigCSchedule(
   }
   TlsServerAuthorizationCheckConfig* cpp_config =
       static_cast<TlsServerAuthorizationCheckConfig*>(arg->config->context());
-  TlsServerAuthorizationCheckArg cpp_arg(arg);
-  return cpp_config->Schedule(&cpp_arg);
+  std::unique_ptr<TlsServerAuthorizationCheckArg> cpp_arg(new TlsServerAuthorizationCheckArg(arg));
+  return cpp_config->Schedule(cpp_arg);
 }
 
 void TlsServerAuthorizationCheckConfigCCancel(
@@ -136,8 +136,8 @@ void TlsServerAuthorizationCheckConfigCCancel(
   }
   TlsServerAuthorizationCheckConfig* cpp_config =
       static_cast<TlsServerAuthorizationCheckConfig*>(arg->config->context());
-  TlsServerAuthorizationCheckArg cpp_arg(arg);
-  cpp_config->Cancel(&cpp_arg);
+  std::unique_ptr<TlsServerAuthorizationCheckArg> cpp_arg(new TlsServerAuthorizationCheckArg(arg));
+  cpp_config->Cancel(cpp_arg);
 }
 
 }  // namespace experimental

test/cpp/client/credentials_test.cc

@@ -50,8 +50,8 @@ static void tls_credential_reload_callback(
   arg->status = GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_UNCHANGED;
 }
 
-class TestTlsCredentialReloadInterface : public TlsCredentialReloadInterface {
-  int Schedule(TlsCredentialReloadArg* arg) override {
+class TestTlsCredentialReload : public TlsCredentialReloadInterface {
+  int Schedule(std::unique_ptr<TlsCredentialReloadArg>& arg) override {
     GPR_ASSERT(arg != nullptr);
     struct TlsKeyMaterialsConfig::PemKeyCertPair pair3 = {"private_key3",
                                                           "cert_chain3"};
@@ -69,7 +69,7 @@ class TestTlsCredentialReloadInterface : public TlsCredentialReloadInterface {
     return 0;
   }
 
-  void Cancel(TlsCredentialReloadArg* arg) override {
+  void Cancel(std::unique_ptr<TlsCredentialReloadArg>& arg) override {
     GPR_ASSERT(arg != nullptr);
     arg->set_status(GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_FAIL);
     arg->set_error_details("cancelled");
@@ -88,9 +88,9 @@ static void tls_server_authorization_check_callback(
   arg->error_details = gpr_strdup("callback_error_details");
 }
 
-class TestTlsServerAuthorizationCheckInterface
+class TestTlsServerAuthorizationCheck
     : public TlsServerAuthorizationCheckInterface {
-  int Schedule(TlsServerAuthorizationCheckArg* arg) override {
+  int Schedule(std::unique_ptr<TlsServerAuthorizationCheckArg>& arg) override {
     GPR_ASSERT(arg != nullptr);
     grpc::string cb_user_data = "cb_user_data";
     arg->set_cb_user_data(static_cast<void*>(gpr_strdup(cb_user_data.c_str())));
@@ -102,7 +102,7 @@ class TestTlsServerAuthorizationCheckInterface
     return 1;
   }
 
-  void Cancel(TlsServerAuthorizationCheckArg* arg) override {
+  void Cancel(std::unique_ptr<TlsServerAuthorizationCheckArg>& arg) override {
     GPR_ASSERT(arg != nullptr);
     arg->set_status(GRPC_STATUS_PERMISSION_DENIED);
     arg->set_error_details("cancelled");
@@ -344,12 +344,12 @@ TEST_F(CredentialsTest, TlsCredentialReloadArgCallback) {
 }
 
 TEST_F(CredentialsTest, TlsCredentialReloadConfigSchedule) {
-  std::unique_ptr<TestTlsCredentialReloadInterface> interface(
-      new TestTlsCredentialReloadInterface());
-  TlsCredentialReloadConfig config(std::move(interface));
+  std::unique_ptr<TestTlsCredentialReload> test_credential_reload(
+      new TestTlsCredentialReload());
+  TlsCredentialReloadConfig config(std::move(test_credential_reload));
   grpc_tls_credential_reload_arg c_arg;
-  TlsCredentialReloadArg arg(&c_arg);
-  arg.set_cb_user_data(static_cast<void*>(nullptr));
+  std::unique_ptr<TlsCredentialReloadArg> arg( new TlsCredentialReloadArg(&c_arg));
+  arg->set_cb_user_data(static_cast<void*>(nullptr));
   std::shared_ptr<TlsKeyMaterialsConfig> key_materials_config(
       new TlsKeyMaterialsConfig());
   struct TlsKeyMaterialsConfig::PemKeyCertPair pair1 = {"private_key1",
@@ -358,19 +358,20 @@ TEST_F(CredentialsTest, TlsCredentialReloadConfigSchedule) {
                                                         "cert_chain2"};
   std::vector<TlsKeyMaterialsConfig::PemKeyCertPair> pair_list = {pair1, pair2};
   key_materials_config->set_key_materials("pem_root_certs", pair_list);
-  arg.set_key_materials_config(key_materials_config);
-  arg.set_status(GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_NEW);
-  arg.set_error_details("error_details");
+  arg->set_key_materials_config(key_materials_config);
+  arg->set_status(GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_NEW);
+  arg->set_error_details("error_details");
   grpc_tls_key_materials_config* key_materials_config_before_schedule =
       c_arg.key_materials_config;
   const char* error_details_before_schedule = c_arg.error_details;
 
-  int schedule_output = config.Schedule(&arg);
+  int schedule_output = config.Schedule(arg);
+  GPR_ASSERT(arg != nullptr);
   EXPECT_EQ(schedule_output, 0);
-  EXPECT_EQ(arg.cb_user_data(), nullptr);
-  EXPECT_STREQ(arg.key_materials_config()->pem_root_certs().c_str(),
+  EXPECT_EQ(arg->cb_user_data(), nullptr);
+  EXPECT_STREQ(arg->key_materials_config()->pem_root_certs().c_str(),
                "new_pem_root_certs");
-  pair_list = arg.key_materials_config()->pem_key_cert_pair_list();
+  pair_list = arg->key_materials_config()->pem_key_cert_pair_list();
   EXPECT_EQ(static_cast<int>(pair_list.size()), 3);
   EXPECT_STREQ(pair_list[0].private_key.c_str(), "private_key01");
   EXPECT_STREQ(pair_list[0].cert_chain.c_str(), "cert_chain01");
@@ -378,8 +379,8 @@ TEST_F(CredentialsTest, TlsCredentialReloadConfigSchedule) {
   EXPECT_STREQ(pair_list[1].cert_chain.c_str(), "cert_chain2");
   EXPECT_STREQ(pair_list[2].private_key.c_str(), "private_key3");
   EXPECT_STREQ(pair_list[2].cert_chain.c_str(), "cert_chain3");
-  EXPECT_EQ(arg.status(), GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_NEW);
-  EXPECT_STREQ(arg.error_details().c_str(), "error_details");
+  EXPECT_EQ(arg->status(), GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_NEW);
+  EXPECT_STREQ(arg->error_details().c_str(), "error_details");
 
   // Cleanup.
   gpr_free(const_cast<char*>(error_details_before_schedule));
@@ -389,9 +390,9 @@ TEST_F(CredentialsTest, TlsCredentialReloadConfigSchedule) {
 }
 
 TEST_F(CredentialsTest, TlsCredentialReloadConfigCppToC) {
-  std::unique_ptr<TestTlsCredentialReloadInterface> interface(
-      new TestTlsCredentialReloadInterface());
-  TlsCredentialReloadConfig config(std::move(interface));
+  std::unique_ptr<TestTlsCredentialReload> test_credential_reload(
+      new TestTlsCredentialReload());
+  TlsCredentialReloadConfig config(std::move(test_credential_reload));
   grpc_tls_credential_reload_arg c_arg;
   c_arg.cb_user_data = static_cast<void*>(nullptr);
   grpc_tls_key_materials_config c_key_materials;
@@ -483,32 +484,33 @@ TEST_F(CredentialsTest, TlsServerAuthorizationCheckArgCallback) {
 }
 
 TEST_F(CredentialsTest, TlsServerAuthorizationCheckConfigSchedule) {
-  std::unique_ptr<TestTlsServerAuthorizationCheckInterface> interface(
-      new TestTlsServerAuthorizationCheckInterface());
-  TlsServerAuthorizationCheckConfig config(std::move(interface));
+  std::unique_ptr<TestTlsServerAuthorizationCheck>
+      test_server_authorization_check(new TestTlsServerAuthorizationCheck());
+  TlsServerAuthorizationCheckConfig config(
+      std::move(test_server_authorization_check));
   grpc_tls_server_authorization_check_arg c_arg;
-  TlsServerAuthorizationCheckArg arg(&c_arg);
-  arg.set_cb_user_data(nullptr);
-  arg.set_success(0);
-  arg.set_target_name("target_name");
-  arg.set_peer_cert("peer_cert");
-  arg.set_status(GRPC_STATUS_PERMISSION_DENIED);
-  arg.set_error_details("error_details");
+  std::unique_ptr<TlsServerAuthorizationCheckArg> arg(new TlsServerAuthorizationCheckArg(&c_arg));
+  arg->set_cb_user_data(nullptr);
+  arg->set_success(0);
+  arg->set_target_name("target_name");
+  arg->set_peer_cert("peer_cert");
+  arg->set_status(GRPC_STATUS_PERMISSION_DENIED);
+  arg->set_error_details("error_details");
   const char* target_name_before_schedule = c_arg.target_name;
   const char* peer_cert_before_schedule = c_arg.peer_cert;
   const char* error_details_before_schedule = c_arg.error_details;
 
-  int schedule_output = config.Schedule(&arg);
+  int schedule_output = config.Schedule(arg);
   EXPECT_EQ(schedule_output, 1);
-  EXPECT_STREQ(static_cast<char*>(arg.cb_user_data()), "cb_user_data");
-  EXPECT_EQ(arg.success(), 1);
-  EXPECT_STREQ(arg.target_name().c_str(), "sync_target_name");
-  EXPECT_STREQ(arg.peer_cert().c_str(), "sync_peer_cert");
-  EXPECT_EQ(arg.status(), GRPC_STATUS_OK);
-  EXPECT_STREQ(arg.error_details().c_str(), "sync_error_details");
+  EXPECT_STREQ(static_cast<char*>(arg->cb_user_data()), "cb_user_data");
+  EXPECT_EQ(arg->success(), 1);
+  EXPECT_STREQ(arg->target_name().c_str(), "sync_target_name");
+  EXPECT_STREQ(arg->peer_cert().c_str(), "sync_peer_cert");
+  EXPECT_EQ(arg->status(), GRPC_STATUS_OK);
+  EXPECT_STREQ(arg->error_details().c_str(), "sync_error_details");
 
   // Cleanup.
-  gpr_free(arg.cb_user_data());
+  gpr_free(arg->cb_user_data());
   gpr_free(const_cast<char*>(target_name_before_schedule));
   gpr_free(const_cast<char*>(peer_cert_before_schedule));
   gpr_free(const_cast<char*>(error_details_before_schedule));
@@ -519,9 +521,10 @@ TEST_F(CredentialsTest, TlsServerAuthorizationCheckConfigSchedule) {
 }
 
 TEST_F(CredentialsTest, TlsServerAuthorizationCheckConfigCppToC) {
-  std::unique_ptr<TestTlsServerAuthorizationCheckInterface> interface(
-      new TestTlsServerAuthorizationCheckInterface());
-  TlsServerAuthorizationCheckConfig config(std::move(interface));
+  std::unique_ptr<TestTlsServerAuthorizationCheck>
+      test_server_authorization_check(new TestTlsServerAuthorizationCheck());
+  TlsServerAuthorizationCheckConfig config(
+      std::move(test_server_authorization_check));
   grpc_tls_server_authorization_check_arg c_arg;
   c_arg.cb = tls_server_authorization_check_callback;
   c_arg.cb_user_data = nullptr;
@@ -568,17 +571,16 @@ TEST_F(CredentialsTest, TlsCredentialsOptionsCppToC) {
   std::vector<TlsKeyMaterialsConfig::PemKeyCertPair> pair_list = {pair};
   key_materials_config->set_key_materials("pem_root_certs", pair_list);
 
-  std::unique_ptr<TestTlsCredentialReloadInterface> credential_reload_interface(
-      new TestTlsCredentialReloadInterface());
+  std::unique_ptr<TestTlsCredentialReload> test_credential_reload(
+      new TestTlsCredentialReload());
   std::shared_ptr<TlsCredentialReloadConfig> credential_reload_config(
-      new TlsCredentialReloadConfig(std::move(credential_reload_interface)));
+      new TlsCredentialReloadConfig(std::move(test_credential_reload)));
 
-  std::unique_ptr<TestTlsServerAuthorizationCheckInterface>
-      server_authorization_check_interface(
-          new TestTlsServerAuthorizationCheckInterface());
+  std::unique_ptr<TestTlsServerAuthorizationCheck>
+      test_server_authorization_check(new TestTlsServerAuthorizationCheck());
   std::shared_ptr<TlsServerAuthorizationCheckConfig>
       server_authorization_check_config(new TlsServerAuthorizationCheckConfig(
-          std::move(server_authorization_check_interface)));
+          std::move(test_server_authorization_check)));
 
   TlsCredentialsOptions options = TlsCredentialsOptions(
       GRPC_SSL_REQUEST_CLIENT_CERTIFICATE_AND_VERIFY, key_materials_config,
@@ -666,17 +668,16 @@ TEST_F(CredentialsTest, TlsCredentialsOptionsCppToC) {
 
 // This test demonstrates how the SPIFFE credentials will be used.
 TEST_F(CredentialsTest, LoadSpiffeChannelCredentials) {
-  std::unique_ptr<TestTlsCredentialReloadInterface> credential_reload_interface(
-      new TestTlsCredentialReloadInterface());
+  std::unique_ptr<TestTlsCredentialReload> test_credential_reload(
+      new TestTlsCredentialReload());
   std::shared_ptr<TlsCredentialReloadConfig> credential_reload_config(
-      new TlsCredentialReloadConfig(std::move(credential_reload_interface)));
+      new TlsCredentialReloadConfig(std::move(test_credential_reload)));
 
-  std::unique_ptr<TestTlsServerAuthorizationCheckInterface>
-      server_authorization_check_interface(
-          new TestTlsServerAuthorizationCheckInterface());
+  std::unique_ptr<TestTlsServerAuthorizationCheck>
+      test_server_authorization_check(new TestTlsServerAuthorizationCheck());
   std::shared_ptr<TlsServerAuthorizationCheckConfig>
       server_authorization_check_config(new TlsServerAuthorizationCheckConfig(
-          std::move(server_authorization_check_interface)));
+          std::move(test_server_authorization_check)));
 
   TlsCredentialsOptions options = TlsCredentialsOptions(
       GRPC_SSL_REQUEST_CLIENT_CERTIFICATE_AND_VERIFY, nullptr,