src: make ownership of stdio_pipes explicit

Use smart pointers instead of raw pointers for StdioPipes. Also, use a smart pointer for the array holding them. PR-URL: #17030 Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
nodejs · Dec 11, 2017 · a1a9957 · a1a9957
1 parent 7329537
commit a1a9957
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 25 deletions.
diff --git a/src/spawn_sync.cc b/src/spawn_sync.cc
@@ -417,14 +417,7 @@ SyncProcessRunner::SyncProcessRunner(Environment* env)
 SyncProcessRunner::~SyncProcessRunner() {
   CHECK_EQ(lifecycle_, kHandlesClosed);
 
-  if (stdio_pipes_ != nullptr) {
-    for (size_t i = 0; i < stdio_count_; i++) {
-      if (stdio_pipes_[i] != nullptr)
-        delete stdio_pipes_[i];
-    }
-  }
-
-  delete[] stdio_pipes_;
+  stdio_pipes_.reset();
   delete[] file_buffer_;
   delete[] args_buffer_;
   delete[] cwd_buffer_;
@@ -495,7 +488,7 @@ void SyncProcessRunner::TryInitializeAndRunLoop(Local<Value> options) {
   uv_process_.data = this;
 
   for (uint32_t i = 0; i < stdio_count_; i++) {
-    SyncProcessStdioPipe* h = stdio_pipes_[i];
+    SyncProcessStdioPipe* h = stdio_pipes_[i].get();
     if (h != nullptr) {
       r = h->Start();
       if (r < 0)
@@ -554,11 +547,11 @@ void SyncProcessRunner::CloseStdioPipes() {
   CHECK_LT(lifecycle_, kHandlesClosed);
 
   if (stdio_pipes_initialized_) {
-    CHECK_NE(stdio_pipes_, nullptr);
+    CHECK(stdio_pipes_);
     CHECK_NE(uv_loop_, nullptr);
 
     for (uint32_t i = 0; i < stdio_count_; i++) {
-      if (stdio_pipes_[i] != nullptr)
+      if (stdio_pipes_[i])
         stdio_pipes_[i]->Close();
     }
 
@@ -711,14 +704,14 @@ Local<Object> SyncProcessRunner::BuildResultObject() {
 
 Local<Array> SyncProcessRunner::BuildOutputArray() {
   CHECK_GE(lifecycle_, kInitialized);
-  CHECK_NE(stdio_pipes_, nullptr);
+  CHECK(stdio_pipes_);
 
   EscapableHandleScope scope(env()->isolate());
   Local<Context> context = env()->context();
   Local<Array> js_output = Array::New(env()->isolate(), stdio_count_);
 
   for (uint32_t i = 0; i < stdio_count_; i++) {
-    SyncProcessStdioPipe* h = stdio_pipes_[i];
+    SyncProcessStdioPipe* h = stdio_pipes_[i].get();
     if (h != nullptr && h->writable())
       js_output->Set(context, i, h->GetOutputAsBuffer(env())).FromJust();
     else
@@ -851,7 +844,8 @@ int SyncProcessRunner::ParseStdioOptions(Local<Value> js_value) {
   stdio_count_ = js_stdio_options->Length();
   uv_stdio_containers_ = new uv_stdio_container_t[stdio_count_];
 
-  stdio_pipes_ = new SyncProcessStdioPipe*[stdio_count_]();
+  stdio_pipes_.reset(
+      new std::unique_ptr<SyncProcessStdioPipe>[stdio_count_]());
   stdio_pipes_initialized_ = true;
 
   for (uint32_t i = 0; i < stdio_count_; i++) {
@@ -925,7 +919,7 @@ int SyncProcessRunner::ParseStdioOption(int child_fd,
 
 int SyncProcessRunner::AddStdioIgnore(uint32_t child_fd) {
   CHECK_LT(child_fd, stdio_count_);
-  CHECK_EQ(stdio_pipes_[child_fd], nullptr);
+  CHECK(!stdio_pipes_[child_fd]);
 
   uv_stdio_containers_[child_fd].flags = UV_IGNORE;
 
@@ -938,31 +932,29 @@ int SyncProcessRunner::AddStdioPipe(uint32_t child_fd,
                                     bool writable,
                                     uv_buf_t input_buffer) {
   CHECK_LT(child_fd, stdio_count_);
-  CHECK_EQ(stdio_pipes_[child_fd], nullptr);
+  CHECK(!stdio_pipes_[child_fd]);
 
-  SyncProcessStdioPipe* h = new SyncProcessStdioPipe(this,
-                                                     readable,
-                                                     writable,
-                                                     input_buffer);
+  std::unique_ptr<SyncProcessStdioPipe> h(
+      new SyncProcessStdioPipe(this, readable, writable, input_buffer));
 
   int r = h->Initialize(uv_loop_);
   if (r < 0) {
-    delete h;
+    h.reset();
     return r;
   }
 
-  stdio_pipes_[child_fd] = h;
-
   uv_stdio_containers_[child_fd].flags = h->uv_flags();
   uv_stdio_containers_[child_fd].data.stream = h->uv_stream();
 
+  stdio_pipes_[child_fd] = std::move(h);
+
   return 0;
 }
 
 
 int SyncProcessRunner::AddStdioInheritFD(uint32_t child_fd, int inherit_fd) {
   CHECK_LT(child_fd, stdio_count_);
-  CHECK_EQ(stdio_pipes_[child_fd], nullptr);
+  CHECK(!stdio_pipes_[child_fd]);
 
   uv_stdio_containers_[child_fd].flags = UV_INHERIT_FD;
   uv_stdio_containers_[child_fd].data.fd = inherit_fd;

diff --git a/src/spawn_sync.h b/src/spawn_sync.h
@@ -213,7 +213,7 @@ class SyncProcessRunner {
 
   uint32_t stdio_count_;
   uv_stdio_container_t* uv_stdio_containers_;
-  SyncProcessStdioPipe** stdio_pipes_;
+  std::unique_ptr<std::unique_ptr<SyncProcessStdioPipe>[]> stdio_pipes_;
   bool stdio_pipes_initialized_;
 
   uv_process_options_t uv_process_options_;