FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: use RAII for uv_process_options_t · nodejs/node@415fff2 · GitHub

/ node Public

Commit 415fff2

Browse files
iknoom
authored and
targos
committed
src: use RAII for uv_process_options_t
PR-URL: #59945 Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent 66d90b8 commit 415fff2

3 files changed

Lines changed: 64 additions & 73 deletions

File tree

‎src/process_wrap.cc‎

Lines 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

‎src/spawn_sync.cc‎

Lines 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);

‎src/spawn_sync.h‎

Lines 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)
Back | FazBrowse Home | New Git URL