src: replace manual memory mgmt with std::string

PR-URL: nodejs#15782 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: James M Snell <[email protected]>
kysnm · Oct 9, 2017 · fca31be · fca31be
1 parent 84579b1
commit fca31be
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 25 deletions.
diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
@@ -66,7 +66,6 @@ TLSWrap::TLSWrap(Environment* env,
       started_(false),
       established_(false),
       shutdown_(false),
-      error_(nullptr),
       cycle_depth_(0),
       eof_(false) {
   node::Wrap(object(), this);
@@ -103,8 +102,6 @@ TLSWrap::~TLSWrap() {
 #ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
   sni_context_.Reset();
 #endif  // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
-
-  ClearError();
 }
 
 
@@ -367,7 +364,7 @@ void TLSWrap::EncOutCb(WriteWrap* req_wrap, int status) {
 }
 
 
-Local<Value> TLSWrap::GetSSLError(int status, int* err, const char** msg) {
+Local<Value> TLSWrap::GetSSLError(int status, int* err, std::string* msg) {
   EscapableHandleScope scope(env()->isolate());
 
   // ssl_ is already destroyed in reading EOF by close notify alert.
@@ -398,13 +395,9 @@ Local<Value> TLSWrap::GetSSLError(int status, int* err, const char** msg) {
             OneByteString(env()->isolate(), mem->data, mem->length);
         Local<Value> exception = Exception::Error(message);
 
-        if (msg != nullptr) {
-          CHECK_EQ(*msg, nullptr);
-          char* const buf = new char[mem->length + 1];
-          memcpy(buf, mem->data, mem->length);
-          buf[mem->length] = '\0';
-          *msg = buf;
-        }
+        if (msg != nullptr)
+          msg->assign(mem->data, mem->data + mem->length);
+
         BIO_free_all(bio);
 
         return scope.Escape(exception);
@@ -516,12 +509,11 @@ bool TLSWrap::ClearIn() {
 
   // Error or partial write
   int err;
-  const char* error_str = nullptr;
+  std::string error_str;
   Local<Value> arg = GetSSLError(written, &err, &error_str);
   if (!arg.IsEmpty()) {
     MakePending();
-    InvokeQueued(UV_EPROTO, error_str);
-    delete[] error_str;
+    InvokeQueued(UV_EPROTO, error_str.c_str());
     clear_in_->Reset();
   }
 
@@ -570,13 +562,12 @@ int TLSWrap::ReadStop() {
 
 
 const char* TLSWrap::Error() const {
-  return error_;
+  return error_.empty() ? nullptr : error_.c_str();
 }
 
 
 void TLSWrap::ClearError() {
-  delete[] error_;
-  error_ = nullptr;
+  error_.clear();
 }
 
 
@@ -624,11 +615,7 @@ int TLSWrap::DoWrite(WriteWrap* w,
 
   if (ssl_ == nullptr) {
     ClearError();
-
-    static char msg[] = "Write after DestroySSL";
-    char* tmp = new char[sizeof(msg)];
-    memcpy(tmp, msg, sizeof(msg));
-    error_ = tmp;
+    error_ = "Write after DestroySSL";
     return UV_EPROTO;
   }
 

diff --git a/src/tls_wrap.h b/src/tls_wrap.h
@@ -35,6 +35,8 @@
 
 #include <openssl/ssl.h>
 
+#include <string>
+
 namespace node {
 
 // Forward-declarations
@@ -148,8 +150,7 @@ class TLSWrap : public AsyncWrap,
 
   void DoRead(ssize_t nread, const uv_buf_t* buf, uv_handle_type pending);
 
-  // If |msg| is not nullptr, caller is responsible for calling `delete[] *msg`.
-  v8::Local<v8::Value> GetSSLError(int status, int* err, const char** msg);
+  v8::Local<v8::Value> GetSSLError(int status, int* err, std::string* msg);
 
   static void OnClientHelloParseEnd(void* arg);
   static void Wrap(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -180,7 +181,7 @@ class TLSWrap : public AsyncWrap,
   bool started_;
   bool established_;
   bool shutdown_;
-  const char* error_;
+  std::string error_;
   int cycle_depth_;
 
   // If true - delivered EOF to the js-land, either after `close_notify`, or