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

src: simplify direct queries of env vars in C++ land · nodejs/node@306c1d3 · GitHub

/ node Public

Commit 306c1d3

Browse files
joyeecheung
authored and
richardlau
committed
src: simplify direct queries of env vars in C++ land
In many cases we can query the environment variable directly instead of encoding it into UTF8 first and then decoding it again from UTF8 via an intermediate V8 string. Drive-by: pass per_process::system_environment explicitly when the real environment variables are supposed to be used instead of relying on fallback with defaults. PR-URL: #51829 Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent a74c373 commit 306c1d3

8 files changed

Lines changed: 19 additions & 62 deletions

File tree

‎src/debug_utils.cc‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,9 @@ EnabledDebugList enabled_debug_list;
6262
using v8::Local;
6363
using v8::StackTrace;
6464

65-
void EnabledDebugList::Parse(std::shared_ptr<KVStore> env_vars,
66-
v8::Isolate* isolate) {
65+
void EnabledDebugList::Parse(std::shared_ptr<KVStore> env_vars) {
6766
std::string cats;
68-
credentials::SafeGetenv("NODE_DEBUG_NATIVE", &cats, env_vars, isolate);
67+
credentials::SafeGetenv("NODE_DEBUG_NATIVE", &cats, env_vars);
6968
Parse(cats);
7069
}
7170

‎src/debug_utils.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,7 @@ class NODE_EXTERN_PRIVATE EnabledDebugList {
7474
// Uses NODE_DEBUG_NATIVE to initialize the categories. The env_vars variable
7575
// is parsed if it is not a nullptr, otherwise the system environment
7676
// variables are parsed.
77-
void Parse(std::shared_ptr<KVStore> env_vars = nullptr,
78-
v8::Isolate* isolate = nullptr);
77+
void Parse(std::shared_ptr<KVStore> env_vars);
7978

8079
private:
8180
// Enable all categories matching cats.

‎src/env.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -848,7 +848,7 @@ Environment::Environment(IsolateData* isolate_data,
848848
}
849849

850850
set_env_vars(per_process::system_environment);
851-
enabled_debug_list_.Parse(env_vars(), isolate);
851+
enabled_debug_list_.Parse(env_vars());
852852

853853
// We create new copies of the per-Environment option sets, so that it is
854854
// easier to modify them after Environment creation. The defaults are

‎src/inspector_profiler.cc‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -418,12 +418,9 @@ void StartProfilers(Environment* env) {
418418
EndStartedProfilers(static_cast<Environment*>(env));
419419
}, env);
420420

421-
Isolate* isolate = env->isolate();
422-
Local<String> coverage_str = env->env_vars()->Get(
423-
isolate, FIXED_ONE_BYTE_STRING(isolate, "NODE_V8_COVERAGE"))
424-
.FromMaybe(Local<String>());
425-
if ((!coverage_str.IsEmpty() && coverage_str->Length() > 0) ||
426-
env->options()->test_runner_coverage) {
421+
std::string coverage_str =
422+
env->env_vars()->Get("NODE_V8_COVERAGE").FromMaybe(std::string());
423+
if (!coverage_str.empty() || env->options()->test_runner_coverage) {
427424
CHECK_NULL(env->coverage_connection());
428425
env->set_coverage_connection(std::make_unique<V8CoverageConnection>(env));
429426
env->coverage_connection()->Start();

‎src/node.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1030,7 +1030,7 @@ InitializeOncePerProcessInternal(const std::vector<std::string>& args,
10301030
if (!(flags & ProcessInitializationFlags::kNoParseGlobalDebugVariables)) {
10311031
// Initialized the enabled list for Debug() calls with system
10321032
// environment variables.
1033-
per_process::enabled_debug_list.Parse();
1033+
per_process::enabled_debug_list.Parse(per_process::system_environment);
10341034
}
10351035

10361036
PlatformInit(flags);

‎src/node_credentials.cc‎

Lines changed: 8 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,10 @@ namespace node {
2424
using v8::Array;
2525
using v8::Context;
2626
using v8::FunctionCallbackInfo;
27-
using v8::HandleScope;
2827
using v8::Isolate;
2928
using v8::Local;
3029
using v8::MaybeLocal;
3130
using v8::Object;
32-
using v8::String;
33-
using v8::TryCatch;
3431
using v8::Uint32;
3532
using v8::Value;
3633

@@ -77,54 +74,24 @@ static bool HasOnly(int capability) {
7774
// setuid root then lookup will not be allowed.
7875
bool SafeGetenv(const char* key,
7976
std::string* text,
80-
std::shared_ptr<KVStore> env_vars,
81-
v8::Isolate* isolate) {
77+
std::shared_ptr<KVStore> env_vars) {
8278
#if !defined(__CloudABI__) && !defined(_WIN32)
8379
#if defined(__linux__)
8480
if ((!HasOnly(CAP_NET_BIND_SERVICE) && linux_at_secure()) ||
8581
getuid() != geteuid() || getgid() != getegid())
8682
#else
8783
if (linux_at_secure() || getuid() != geteuid() || getgid() != getegid())
8884
#endif
89-
goto fail;
85+
return false;
9086
#endif
9187

92-
if (env_vars != nullptr) {
93-
DCHECK_NOT_NULL(isolate);
94-
HandleScope handle_scope(isolate);
95-
TryCatch ignore_errors(isolate);
96-
MaybeLocal<String> maybe_value = env_vars->Get(
97-
isolate, String::NewFromUtf8(isolate, key).ToLocalChecked());
98-
Local<String> value;
99-
if (!maybe_value.ToLocal(&value)) goto fail;
100-
String::Utf8Value utf8_value(isolate, value);
101-
if (*utf8_value == nullptr) goto fail;
102-
*text = std::string(*utf8_value, utf8_value.length());
103-
return true;
104-
}
105-
106-
{
107-
Mutex::ScopedLock lock(per_process::env_var_mutex);
108-
109-
size_t init_sz = 256;
110-
MaybeStackBuffer<char, 256> val;
111-
int ret = uv_os_getenv(key, *val, &init_sz);
112-
113-
if (ret == UV_ENOBUFS) {
114-
// Buffer is not large enough, reallocate to the updated init_sz
115-
// and fetch env value again.
116-
val.AllocateSufficientStorage(init_sz);
117-
ret = uv_os_getenv(key, *val, &init_sz);
118-
}
119-
120-
if (ret == 0) { // Env key value fetch success.
121-
*text = *val;
122-
return true;
123-
}
88+
// Fallback to system environment which reads the real environment variable
89+
// through uv_os_getenv.
90+
if (env_vars == nullptr) {
91+
env_vars = per_process::system_environment;
12492
}
12593

126-
fail:
127-
return false;
94+
return env_vars->Get(key).To(text);
12895
}
12996

13097
static void SafeGetenv(const FunctionCallbackInfo<Value>& args) {
@@ -133,7 +100,7 @@ static void SafeGetenv(const FunctionCallbackInfo<Value>& args) {
133100
Isolate* isolate = env->isolate();
134101
Utf8Value strenvtag(isolate, args[0]);
135102
std::string text;
136-
if (!SafeGetenv(*strenvtag, &text, env->env_vars(), isolate)) return;
103+
if (!SafeGetenv(*strenvtag, &text, env->env_vars())) return;
137104
Local<Value> result =
138105
ToV8Value(isolate->GetCurrentContext(), text).ToLocalChecked();
139106
args.GetReturnValue().Set(result);

‎src/node_internals.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -308,8 +308,7 @@ class ThreadPoolWork {
308308
namespace credentials {
309309
bool SafeGetenv(const char* key,
310310
std::string* text,
311-
std::shared_ptr<KVStore> env_vars = nullptr,
312-
v8::Isolate* isolate = nullptr);
311+
std::shared_ptr<KVStore> env_vars = nullptr);
313312
} // namespace credentials
314313

315314
void DefineZlibConstants(v8::Local<v8::Object> target);

‎src/node_worker.cc‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ using v8::Isolate;
3232
using v8::Local;
3333
using v8::Locker;
3434
using v8::Maybe;
35-
using v8::MaybeLocal;
3635
using v8::Null;
3736
using v8::Number;
3837
using v8::Object;
@@ -537,11 +536,8 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
537536
});
538537

539538
#ifndef NODE_WITHOUT_NODE_OPTIONS
540-
MaybeLocal<String> maybe_node_opts =
541-
env_vars->Get(isolate, OneByteString(isolate, "NODE_OPTIONS"));
542-
Local<String> node_opts;
543-
if (maybe_node_opts.ToLocal(&node_opts)) {
544-
std::string node_options(*String::Utf8Value(isolate, node_opts));
539+
std::string node_options;
540+
if (env_vars->Get("NODE_OPTIONS").To(&node_options)) {
545541
std::vector<std::string> errors{};
546542
std::vector<std::string> env_argv =
547543
ParseNodeOptionsEnvVar(node_options, &errors);

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL