Commit cdba989
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 2409db6 commit cdba989
2 files changed
Lines changed: 13 additions & 25 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -47,7 +47,6 @@ TLSWrap::TLSWrap(Environment* env, | |||
| 47 | 47 | started_(false), | |
| 48 | 48 | established_(false), | |
| 49 | 49 | shutdown_(false), | |
| 50 | - error_(nullptr), | ||
| 51 | 50 | cycle_depth_(0), | |
| 52 | 51 | eof_(false) { | |
| 53 | 52 | node::Wrap(object(), this); | |
@@ -84,8 +83,6 @@ TLSWrap::~TLSWrap() { | |||
| 84 | 83 | #ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB | |
| 85 | 84 | sni_context_.Reset(); | |
| 86 | 85 | #endif // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB | |
| 87 | - | ||
| 88 | - ClearError(); | ||
| 89 | 86 | } | |
| 90 | 87 | ||
| 91 | 88 | ||
@@ -348,7 +345,7 @@ void TLSWrap::EncOutCb(WriteWrap* req_wrap, int status) { | |||
| 348 | 345 | } | |
| 349 | 346 | ||
| 350 | 347 | ||
| 351 | - Local<Value> TLSWrap::GetSSLError(int status, int* err, const char** msg) { | ||
| 348 | + Local<Value> TLSWrap::GetSSLError(int status, int* err, std::string* msg) { | ||
| 352 | 349 | EscapableHandleScope scope(env()->isolate()); | |
| 353 | 350 | ||
| 354 | 351 | // 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) { | |||
| 379 | 376 | OneByteString(env()->isolate(), mem->data, mem->length); | |
| 380 | 377 | Local<Value> exception = Exception::Error(message); | |
| 381 | 378 | ||
| 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 | + | ||
| 389 | 382 | BIO_free_all(bio); | |
| 390 | 383 | ||
| 391 | 384 | return scope.Escape(exception); | |
@@ -497,12 +490,11 @@ bool TLSWrap::ClearIn() { | |||
| 497 | 490 | ||
| 498 | 491 | // Error or partial write | |
| 499 | 492 | int err; | |
| 500 | - const char* error_str = nullptr; | ||
| 493 | + std::string error_str; | ||
| 501 | 494 | Local<Value> arg = GetSSLError(written, &err, &error_str); | |
| 502 | 495 | if (!arg.IsEmpty()) { | |
| 503 | 496 | MakePending(); | |
| 504 | - InvokeQueued(UV_EPROTO, error_str); | ||
| 505 | - delete[] error_str; | ||
| 497 | + InvokeQueued(UV_EPROTO, error_str.c_str()); | ||
| 506 | 498 | clear_in_->Reset(); | |
| 507 | 499 | } | |
| 508 | 500 | ||
@@ -551,13 +543,12 @@ int TLSWrap::ReadStop() { | |||
| 551 | 543 | ||
| 552 | 544 | ||
| 553 | 545 | const char* TLSWrap::Error() const { | |
| 554 | - return error_; | ||
| 546 | + return error_.empty() ? nullptr : error_.c_str(); | ||
| 555 | 547 | } | |
| 556 | 548 | ||
| 557 | 549 | ||
| 558 | 550 | void TLSWrap::ClearError() { | |
| 559 | - delete[] error_; | ||
| 560 | - error_ = nullptr; | ||
| 551 | + error_.clear(); | ||
| 561 | 552 | } | |
| 562 | 553 | ||
| 563 | 554 | ||
@@ -605,11 +596,7 @@ int TLSWrap::DoWrite(WriteWrap* w, | |||
| 605 | 596 | ||
| 606 | 597 | if (ssl_ == nullptr) { | |
| 607 | 598 | 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"; | ||
| 613 | 600 | return UV_EPROTO; | |
| 614 | 601 | } | |
| 615 | 602 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -14,6 +14,8 @@ | |||
| 14 | 14 | ||
| 15 | 15 | #include <openssl/ssl.h> | |
| 16 | 16 | ||
| 17 | + #include <string> | ||
| 18 | + | ||
| 17 | 19 | namespace node { | |
| 18 | 20 | ||
| 19 | 21 | // Forward-declarations | |
@@ -127,8 +129,7 @@ class TLSWrap : public AsyncWrap, | |||
| 127 | 129 | ||
| 128 | 130 | void DoRead(ssize_t nread, const uv_buf_t* buf, uv_handle_type pending); | |
| 129 | 131 | ||
| 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); | ||
| 132 | 133 | ||
| 133 | 134 | static void OnClientHelloParseEnd(void* arg); | |
| 134 | 135 | static void Wrap(const v8::FunctionCallbackInfo<v8::Value>& args); | |
@@ -159,7 +160,7 @@ class TLSWrap : public AsyncWrap, | |||
| 159 | 160 | bool started_; | |
| 160 | 161 | bool established_; | |
| 161 | 162 | bool shutdown_; | |
| 162 | - const char* error_; | ||
| 163 | + std::string error_; | ||
| 163 | 164 | int cycle_depth_; | |
| 164 | 165 | ||
| 165 | 166 | // If true - delivered EOF to the js-land, either after `close_notify`, or | |
0 commit comments