Skip to content

Commit d01978e

Browse files
bnoordhuisMylesBorins
authored andcommitted
src: replace manual memory mgmt with std::string
PR-URL: #15782 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Minwoo Jung <minwoo@nodesource.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent a15f33c commit d01978e

File tree

2 files changed

+13
-25
lines changed

2 files changed

+13
-25
lines changed

src/tls_wrap.cc

+9-22
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ TLSWrap::TLSWrap(Environment* env,
4747
started_(false),
4848
established_(false),
4949
shutdown_(false),
50-
error_(nullptr),
5150
cycle_depth_(0),
5251
eof_(false) {
5352
node::Wrap(object(), this);
@@ -84,8 +83,6 @@ TLSWrap::~TLSWrap() {
8483
#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
8584
sni_context_.Reset();
8685
#endif // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
87-
88-
ClearError();
8986
}
9087

9188

@@ -348,7 +345,7 @@ void TLSWrap::EncOutCb(WriteWrap* req_wrap, int status) {
348345
}
349346

350347

351-
Local<Value> TLSWrap::GetSSLError(int status, int* err, const char** msg) {
348+
Local<Value> TLSWrap::GetSSLError(int status, int* err, std::string* msg) {
352349
EscapableHandleScope scope(env()->isolate());
353350

354351
// ssl_ is already destroyed in reading EOF by close notify alert.
@@ -379,13 +376,9 @@ Local<Value> TLSWrap::GetSSLError(int status, int* err, const char** msg) {
379376
OneByteString(env()->isolate(), mem->data, mem->length);
380377
Local<Value> exception = Exception::Error(message);
381378

382-
if (msg != nullptr) {
383-
CHECK_EQ(*msg, nullptr);
384-
char* const buf = new char[mem->length + 1];
385-
memcpy(buf, mem->data, mem->length);
386-
buf[mem->length] = '\0';
387-
*msg = buf;
388-
}
379+
if (msg != nullptr)
380+
msg->assign(mem->data, mem->data + mem->length);
381+
389382
BIO_free_all(bio);
390383

391384
return scope.Escape(exception);
@@ -497,12 +490,11 @@ bool TLSWrap::ClearIn() {
497490

498491
// Error or partial write
499492
int err;
500-
const char* error_str = nullptr;
493+
std::string error_str;
501494
Local<Value> arg = GetSSLError(written, &err, &error_str);
502495
if (!arg.IsEmpty()) {
503496
MakePending();
504-
InvokeQueued(UV_EPROTO, error_str);
505-
delete[] error_str;
497+
InvokeQueued(UV_EPROTO, error_str.c_str());
506498
clear_in_->Reset();
507499
}
508500

@@ -551,13 +543,12 @@ int TLSWrap::ReadStop() {
551543

552544

553545
const char* TLSWrap::Error() const {
554-
return error_;
546+
return error_.empty() ? nullptr : error_.c_str();
555547
}
556548

557549

558550
void TLSWrap::ClearError() {
559-
delete[] error_;
560-
error_ = nullptr;
551+
error_.clear();
561552
}
562553

563554

@@ -605,11 +596,7 @@ int TLSWrap::DoWrite(WriteWrap* w,
605596

606597
if (ssl_ == nullptr) {
607598
ClearError();
608-
609-
static char msg[] = "Write after DestroySSL";
610-
char* tmp = new char[sizeof(msg)];
611-
memcpy(tmp, msg, sizeof(msg));
612-
error_ = tmp;
599+
error_ = "Write after DestroySSL";
613600
return UV_EPROTO;
614601
}
615602

src/tls_wrap.h

+4-3
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414

1515
#include <openssl/ssl.h>
1616

17+
#include <string>
18+
1719
namespace node {
1820

1921
// Forward-declarations
@@ -127,8 +129,7 @@ class TLSWrap : public AsyncWrap,
127129

128130
void DoRead(ssize_t nread, const uv_buf_t* buf, uv_handle_type pending);
129131

130-
// If |msg| is not nullptr, caller is responsible for calling `delete[] *msg`.
131-
v8::Local<v8::Value> GetSSLError(int status, int* err, const char** msg);
132+
v8::Local<v8::Value> GetSSLError(int status, int* err, std::string* msg);
132133

133134
static void OnClientHelloParseEnd(void* arg);
134135
static void Wrap(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -159,7 +160,7 @@ class TLSWrap : public AsyncWrap,
159160
bool started_;
160161
bool established_;
161162
bool shutdown_;
162-
const char* error_;
163+
std::string error_;
163164
int cycle_depth_;
164165

165166
// If true - delivered EOF to the js-land, either after `close_notify`, or

0 commit comments

Comments
 (0)