Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit c7c597e

Browse filesBrowse files
iknoomaduh95
iknoom
authored and
aduh95
committed
src: use RAII for uv_process_options_t
PR-URL: #59945 Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent af33e8e commit c7c597e
Copy full SHA for c7c597e

File tree

Expand file treeCollapse file tree

3 files changed

+64
-73
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+64
-73
lines changed
Open diff view settings
Collapse file

‎src/process_wrap.cc‎

Copy file name to clipboardExpand all lines: src/process_wrap.cc
+40-43Lines changed: 40 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,10 @@ class ProcessWrap : public HandleWrap {
116116
return Just(stream);
117117
}
118118

119-
static Maybe<void> ParseStdioOptions(Environment* env,
120-
Local<Object> js_options,
121-
uv_process_options_t* options) {
119+
static Maybe<void> ParseStdioOptions(
120+
Environment* env,
121+
Local<Object> js_options,
122+
std::vector<uv_stdio_container_t>* options_stdio) {
122123
Local<Context> context = env->context();
123124
Local<String> stdio_key = env->stdio_string();
124125
Local<Value> stdios_val;
@@ -132,8 +133,7 @@ class ProcessWrap : public HandleWrap {
132133
Local<Array> stdios = stdios_val.As<Array>();
133134

134135
uint32_t len = stdios->Length();
135-
options->stdio = new uv_stdio_container_t[len];
136-
options->stdio_count = len;
136+
options_stdio->resize(len);
137137

138138
for (uint32_t i = 0; i < len; i++) {
139139
Local<Value> val;
@@ -147,23 +147,23 @@ class ProcessWrap : public HandleWrap {
147147
}
148148

149149
if (type->StrictEquals(env->ignore_string())) {
150-
options->stdio[i].flags = UV_IGNORE;
150+
(*options_stdio)[i].flags = UV_IGNORE;
151151
} else if (type->StrictEquals(env->pipe_string())) {
152-
options->stdio[i].flags = static_cast<uv_stdio_flags>(
152+
(*options_stdio)[i].flags = static_cast<uv_stdio_flags>(
153153
UV_CREATE_PIPE | UV_READABLE_PIPE | UV_WRITABLE_PIPE);
154-
if (!StreamForWrap(env, stdio).To(&options->stdio[i].data.stream)) {
154+
if (!StreamForWrap(env, stdio).To(&(*options_stdio)[i].data.stream)) {
155155
return Nothing<void>();
156156
}
157157
} else if (type->StrictEquals(env->overlapped_string())) {
158-
options->stdio[i].flags = static_cast<uv_stdio_flags>(
159-
UV_CREATE_PIPE | UV_READABLE_PIPE | UV_WRITABLE_PIPE |
160-
UV_OVERLAPPED_PIPE);
161-
if (!StreamForWrap(env, stdio).To(&options->stdio[i].data.stream)) {
158+
(*options_stdio)[i].flags =
159+
static_cast<uv_stdio_flags>(UV_CREATE_PIPE | UV_READABLE_PIPE |
160+
UV_WRITABLE_PIPE | UV_OVERLAPPED_PIPE);
161+
if (!StreamForWrap(env, stdio).To(&(*options_stdio)[i].data.stream)) {
162162
return Nothing<void>();
163163
}
164164
} else if (type->StrictEquals(env->wrap_string())) {
165-
options->stdio[i].flags = UV_INHERIT_STREAM;
166-
if (!StreamForWrap(env, stdio).To(&options->stdio[i].data.stream)) {
165+
(*options_stdio)[i].flags = UV_INHERIT_STREAM;
166+
if (!StreamForWrap(env, stdio).To(&(*options_stdio)[i].data.stream)) {
167167
return Nothing<void>();
168168
}
169169
} else {
@@ -174,8 +174,8 @@ class ProcessWrap : public HandleWrap {
174174
}
175175
CHECK(fd_value->IsNumber());
176176
int fd = FromV8Value<int>(fd_value);
177-
options->stdio[i].flags = UV_INHERIT_FD;
178-
options->stdio[i].data.fd = fd;
177+
(*options_stdio)[i].flags = UV_INHERIT_FD;
178+
(*options_stdio)[i].data.fd = fd;
179179
}
180180
}
181181
return JustVoid();
@@ -199,8 +199,6 @@ class ProcessWrap : public HandleWrap {
199199

200200
options.exit_cb = OnExit;
201201

202-
// TODO(bnoordhuis) is this possible to do without mallocing ?
203-
204202
// options.file
205203
Local<Value> file_v;
206204
if (!js_options->Get(context, env->file_string()).ToLocal(&file_v)) {
@@ -251,23 +249,26 @@ class ProcessWrap : public HandleWrap {
251249
if (!js_options->Get(context, env->args_string()).ToLocal(&argv_v)) {
252250
return;
253251
}
254-
if (!argv_v.IsEmpty() && argv_v->IsArray()) {
252+
std::vector<char*> options_args;
253+
std::vector<std::string> args_vals;
254+
if (argv_v->IsArray()) {
255255
Local<Array> js_argv = argv_v.As<Array>();
256256
int argc = js_argv->Length();
257257
CHECK_LT(argc, INT_MAX); // Check for overflow.
258-
259-
// Heap allocate to detect errors. +1 is for nullptr.
260-
options.args = new char*[argc + 1];
258+
args_vals.reserve(argc);
259+
options_args.resize(argc + 1);
261260
for (int i = 0; i < argc; i++) {
262261
Local<Value> val;
263262
if (!js_argv->Get(context, i).ToLocal(&val)) {
264263
return;
265264
}
266265
node::Utf8Value arg(env->isolate(), val);
267-
options.args[i] = strdup(*arg);
268-
CHECK_NOT_NULL(options.args[i]);
266+
args_vals.emplace_back(arg.ToString());
267+
options_args[i] = const_cast<char*>(args_vals.back().c_str());
268+
CHECK_NOT_NULL(options_args[i]);
269269
}
270-
options.args[argc] = nullptr;
270+
options_args[argc] = nullptr;
271+
options.args = options_args.data();
271272
}
272273

273274
// options.cwd
@@ -286,27 +287,35 @@ class ProcessWrap : public HandleWrap {
286287
if (!js_options->Get(context, env->env_pairs_string()).ToLocal(&env_v)) {
287288
return;
288289
}
289-
if (!env_v.IsEmpty() && env_v->IsArray()) {
290+
std::vector<char*> options_env;
291+
std::vector<std::string> env_vals;
292+
if (env_v->IsArray()) {
290293
Local<Array> env_opt = env_v.As<Array>();
291294
int envc = env_opt->Length();
292295
CHECK_LT(envc, INT_MAX); // Check for overflow.
293-
options.env = new char*[envc + 1]; // Heap allocated to detect errors.
296+
env_vals.reserve(envc);
297+
options_env.resize(envc + 1);
294298
for (int i = 0; i < envc; i++) {
295299
Local<Value> val;
296300
if (!env_opt->Get(context, i).ToLocal(&val)) {
297301
return;
298302
}
299303
node::Utf8Value pair(env->isolate(), val);
300-
options.env[i] = strdup(*pair);
301-
CHECK_NOT_NULL(options.env[i]);
304+
env_vals.emplace_back(pair.ToString());
305+
options_env[i] = const_cast<char*>(env_vals.back().c_str());
306+
CHECK_NOT_NULL(options_env[i]);
302307
}
303-
options.env[envc] = nullptr;
308+
options_env[envc] = nullptr;
309+
options.env = options_env.data();
304310
}
305311

306312
// options.stdio
307-
if (ParseStdioOptions(env, js_options, &options).IsNothing()) {
313+
std::vector<uv_stdio_container_t> options_stdio;
314+
if (ParseStdioOptions(env, js_options, &options_stdio).IsNothing()) {
308315
return;
309316
}
317+
options.stdio = options_stdio.data();
318+
options.stdio_count = options_stdio.size();
310319

311320
// options.windowsHide
312321
Local<Value> hide_v;
@@ -361,18 +370,6 @@ class ProcessWrap : public HandleWrap {
361370
}
362371
}
363372

364-
if (options.args) {
365-
for (int i = 0; options.args[i]; i++) free(options.args[i]);
366-
delete [] options.args;
367-
}
368-
369-
if (options.env) {
370-
for (int i = 0; options.env[i]; i++) free(options.env[i]);
371-
delete [] options.env;
372-
}
373-
374-
delete[] options.stdio;
375-
376373
args.GetReturnValue().Set(err);
377374
}
378375

Collapse file

‎src/spawn_sync.cc‎

Copy file name to clipboardExpand all lines: src/spawn_sync.cc
+19-25Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,6 @@ void SyncProcessRunner::Spawn(const FunctionCallbackInfo<Value>& args) {
403403
args.GetReturnValue().Set(result);
404404
}
405405

406-
407406
SyncProcessRunner::SyncProcessRunner(Environment* env)
408407
: max_buffer_(0),
409408
timeout_(0),
@@ -412,14 +411,9 @@ SyncProcessRunner::SyncProcessRunner(Environment* env)
412411
uv_loop_(nullptr),
413412

414413
stdio_count_(0),
415-
uv_stdio_containers_(nullptr),
416414
stdio_pipes_initialized_(false),
417415

418416
uv_process_options_(),
419-
file_buffer_(nullptr),
420-
args_buffer_(nullptr),
421-
env_buffer_(nullptr),
422-
cwd_buffer_(nullptr),
423417

424418
uv_process_(),
425419
killed_(false),
@@ -436,19 +430,12 @@ SyncProcessRunner::SyncProcessRunner(Environment* env)
436430

437431
lifecycle_(kUninitialized),
438432

439-
env_(env) {
440-
}
441-
433+
env_(env) {}
442434

443435
SyncProcessRunner::~SyncProcessRunner() {
444436
CHECK_EQ(lifecycle_, kHandlesClosed);
445437

446438
stdio_pipes_.clear();
447-
delete[] file_buffer_;
448-
delete[] args_buffer_;
449-
delete[] cwd_buffer_;
450-
delete[] env_buffer_;
451-
delete[] uv_stdio_containers_;
452439
}
453440

454441

@@ -808,12 +795,14 @@ Maybe<int> SyncProcessRunner::ParseOptions(Local<Value> js_value) {
808795
Local<Object> js_options = js_value.As<Object>();
809796

810797
Local<Value> js_file;
798+
const char* file_buffer;
811799
if (!js_options->Get(context, env()->file_string()).ToLocal(&js_file) ||
812-
!CopyJsString(js_file, &file_buffer_).To(&r)) {
800+
!CopyJsString(js_file, &file_buffer).To(&r)) {
813801
return Nothing<int>();
814802
}
815803
if (r < 0) return Just(r);
816-
uv_process_options_.file = file_buffer_;
804+
file_buffer_.reset(file_buffer);
805+
uv_process_options_.file = file_buffer_.get();
817806

818807
// Undocumented feature of Win32 CreateProcess API allows spawning
819808
// batch files directly but is potentially insecure because arguments
@@ -825,23 +814,27 @@ Maybe<int> SyncProcessRunner::ParseOptions(Local<Value> js_value) {
825814
#endif
826815

827816
Local<Value> js_args;
817+
char* args_buffer;
828818
if (!js_options->Get(context, env()->args_string()).ToLocal(&js_args) ||
829-
!CopyJsStringArray(js_args, &args_buffer_).To(&r)) {
819+
!CopyJsStringArray(js_args, &args_buffer).To(&r)) {
830820
return Nothing<int>();
831821
}
832822
if (r < 0) return Just(r);
833-
uv_process_options_.args = reinterpret_cast<char**>(args_buffer_);
823+
args_buffer_.reset(args_buffer);
824+
uv_process_options_.args = reinterpret_cast<char**>(args_buffer_.get());
834825

835826
Local<Value> js_cwd;
836827
if (!js_options->Get(context, env()->cwd_string()).ToLocal(&js_cwd)) {
837828
return Nothing<int>();
838829
}
839830
if (!js_cwd->IsNullOrUndefined()) {
840-
if (!CopyJsString(js_cwd, &cwd_buffer_).To(&r)) {
831+
const char* cwd_buffer;
832+
if (!CopyJsString(js_cwd, &cwd_buffer).To(&r)) {
841833
return Nothing<int>();
842834
}
843835
if (r < 0) return Just(r);
844-
uv_process_options_.cwd = cwd_buffer_;
836+
cwd_buffer_.reset(cwd_buffer);
837+
uv_process_options_.cwd = cwd_buffer_.get();
845838
}
846839

847840
Local<Value> js_env_pairs;
@@ -850,12 +843,13 @@ Maybe<int> SyncProcessRunner::ParseOptions(Local<Value> js_value) {
850843
return Nothing<int>();
851844
}
852845
if (!js_env_pairs->IsNullOrUndefined()) {
853-
if (!CopyJsStringArray(js_env_pairs, &env_buffer_).To(&r)) {
846+
char* env_buffer;
847+
if (!CopyJsStringArray(js_env_pairs, &env_buffer).To(&r)) {
854848
return Nothing<int>();
855849
}
856850
if (r < 0) return Just(r);
857-
858-
uv_process_options_.env = reinterpret_cast<char**>(env_buffer_);
851+
env_buffer_.reset(env_buffer);
852+
uv_process_options_.env = reinterpret_cast<char**>(env_buffer_.get());
859853
}
860854
Local<Value> js_uid;
861855
if (!js_options->Get(context, env()->uid_string()).ToLocal(&js_uid)) {
@@ -982,7 +976,7 @@ Maybe<int> SyncProcessRunner::ParseStdioOptions(Local<Value> js_value) {
982976
js_stdio_options = js_value.As<Array>();
983977

984978
stdio_count_ = js_stdio_options->Length();
985-
uv_stdio_containers_ = new uv_stdio_container_t[stdio_count_];
979+
uv_stdio_containers_.resize(stdio_count_);
986980

987981
stdio_pipes_.clear();
988982
stdio_pipes_.resize(stdio_count_);
@@ -1007,7 +1001,7 @@ Maybe<int> SyncProcessRunner::ParseStdioOptions(Local<Value> js_value) {
10071001
}
10081002
}
10091003

1010-
uv_process_options_.stdio = uv_stdio_containers_;
1004+
uv_process_options_.stdio = uv_stdio_containers_.data();
10111005
uv_process_options_.stdio_count = stdio_count_;
10121006

10131007
return Just<int>(0);
Collapse file

‎src/spawn_sync.h‎

Copy file name to clipboardExpand all lines: src/spawn_sync.h
+5-5Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -205,15 +205,15 @@ class SyncProcessRunner {
205205
uv_loop_t* uv_loop_;
206206

207207
uint32_t stdio_count_;
208-
uv_stdio_container_t* uv_stdio_containers_;
208+
std::vector<uv_stdio_container_t> uv_stdio_containers_;
209209
std::vector<std::unique_ptr<SyncProcessStdioPipe>> stdio_pipes_;
210210
bool stdio_pipes_initialized_;
211211

212212
uv_process_options_t uv_process_options_;
213-
const char* file_buffer_;
214-
char* args_buffer_;
215-
char* env_buffer_;
216-
const char* cwd_buffer_;
213+
std::unique_ptr<const char[]> file_buffer_;
214+
std::unique_ptr<char[]> args_buffer_;
215+
std::unique_ptr<char[]> env_buffer_;
216+
std::unique_ptr<const char[]> cwd_buffer_;
217217

218218
uv_process_t uv_process_;
219219
bool killed_;

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.