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

src: pass only Isolate* and env_vars to EnabledDebugList::Parse() · nodejs/node@5db0c8f · GitHub

/ node Public

Commit 5db0c8f

Browse files
RaisinTen
authored and
danielleadams
committed
src: pass only Isolate* and env_vars to EnabledDebugList::Parse()
The function doesn't require access to the entire Environment object, so this change just passes what it needs. Addresses this TODO - https://github.com/nodejs/node/blob/71691e53d601a4cf3eab3b33cebe4cc9f50c8470/src/env.cc#L372-L374 Signed-off-by: Darshan Sen <raisinten@gmail.com> PR-URL: #43668 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
1 parent 2493655 commit 5db0c8f

8 files changed

Lines changed: 56 additions & 49 deletions

File tree

‎src/debug_utils.cc‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include "debug_utils-inl.h" // NOLINT(build/include)
22
#include "env-inl.h"
33
#include "node_internals.h"
4+
#include "util.h"
45

56
#ifdef __POSIX__
67
#if defined(__linux__)
@@ -58,9 +59,10 @@ namespace per_process {
5859
EnabledDebugList enabled_debug_list;
5960
}
6061

61-
void EnabledDebugList::Parse(Environment* env) {
62+
void EnabledDebugList::Parse(std::shared_ptr<KVStore> env_vars,
63+
v8::Isolate* isolate) {
6264
std::string cats;
63-
credentials::SafeGetenv("NODE_DEBUG_NATIVE", &cats, env);
65+
credentials::SafeGetenv("NODE_DEBUG_NATIVE", &cats, env_vars, isolate);
6466
Parse(cats, true);
6567
}
6668

‎src/debug_utils.h‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
55

66
#include "async_wrap.h"
7+
#include "util.h"
78

89
#include <algorithm>
910
#include <sstream>
@@ -66,10 +67,11 @@ class NODE_EXTERN_PRIVATE EnabledDebugList {
6667
return enabled_[static_cast<int>(category)];
6768
}
6869

69-
// Uses NODE_DEBUG_NATIVE to initialize the categories. When env is not a
70-
// nullptr, the environment variables set in the Environment are used.
71-
// Otherwise the system environment variables are used.
72-
void Parse(Environment* env);
70+
// Uses NODE_DEBUG_NATIVE to initialize the categories. The env_vars variable
71+
// is parsed if it is not a nullptr, otherwise the system environment
72+
// variables are parsed.
73+
void Parse(std::shared_ptr<KVStore> env_vars = nullptr,
74+
v8::Isolate* isolate = nullptr);
7375

7476
private:
7577
// Set all categories matching cats to the value of enabled.

‎src/env.cc‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -760,10 +760,7 @@ Environment::Environment(IsolateData* isolate_data,
760760
}
761761

762762
set_env_vars(per_process::system_environment);
763-
// TODO(joyeecheung): pass Isolate* and env_vars to it instead of the entire
764-
// env, when the recursive dependency inclusion in "debug-utils.h" is
765-
// resolved.
766-
enabled_debug_list_.Parse(this);
763+
enabled_debug_list_.Parse(env_vars(), isolate);
767764

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

‎src/env.h‎

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -657,34 +657,6 @@ struct ContextInfo {
657657

658658
class EnabledDebugList;
659659

660-
class KVStore {
661-
public:
662-
KVStore() = default;
663-
virtual ~KVStore() = default;
664-
KVStore(const KVStore&) = delete;
665-
KVStore& operator=(const KVStore&) = delete;
666-
KVStore(KVStore&&) = delete;
667-
KVStore& operator=(KVStore&&) = delete;
668-
669-
virtual v8::MaybeLocal<v8::String> Get(v8::Isolate* isolate,
670-
v8::Local<v8::String> key) const = 0;
671-
virtual v8::Maybe<std::string> Get(const char* key) const = 0;
672-
virtual void Set(v8::Isolate* isolate,
673-
v8::Local<v8::String> key,
674-
v8::Local<v8::String> value) = 0;
675-
virtual int32_t Query(v8::Isolate* isolate,
676-
v8::Local<v8::String> key) const = 0;
677-
virtual int32_t Query(const char* key) const = 0;
678-
virtual void Delete(v8::Isolate* isolate, v8::Local<v8::String> key) = 0;
679-
virtual v8::Local<v8::Array> Enumerate(v8::Isolate* isolate) const = 0;
680-
681-
virtual std::shared_ptr<KVStore> Clone(v8::Isolate* isolate) const;
682-
virtual v8::Maybe<bool> AssignFromObject(v8::Local<v8::Context> context,
683-
v8::Local<v8::Object> entries);
684-
685-
static std::shared_ptr<KVStore> CreateMapKVStore();
686-
};
687-
688660
namespace per_process {
689661
extern std::shared_ptr<KVStore> system_environment;
690662
}

‎src/node.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1008,7 +1008,7 @@ InitializationResult InitializeOncePerProcess(
10081008

10091009
// Initialized the enabled list for Debug() calls with system
10101010
// environment variables.
1011-
per_process::enabled_debug_list.Parse(nullptr);
1011+
per_process::enabled_debug_list.Parse();
10121012

10131013
atexit(ResetStdio);
10141014

‎src/node_credentials.cc‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,10 @@ bool HasOnly(int capability) {
6464
// process only has the capability CAP_NET_BIND_SERVICE set. If the current
6565
// process does not have any capabilities set and the process is running as
6666
// setuid root then lookup will not be allowed.
67-
bool SafeGetenv(const char* key, std::string* text, Environment* env) {
67+
bool SafeGetenv(const char* key,
68+
std::string* text,
69+
std::shared_ptr<KVStore> env_vars,
70+
v8::Isolate* isolate) {
6871
#if !defined(__CloudABI__) && !defined(_WIN32)
6972
#if defined(__linux__)
7073
if ((!HasOnly(CAP_NET_BIND_SERVICE) && per_process::linux_at_secure) ||
@@ -76,15 +79,15 @@ bool SafeGetenv(const char* key, std::string* text, Environment* env) {
7679
goto fail;
7780
#endif
7881

79-
if (env != nullptr) {
80-
HandleScope handle_scope(env->isolate());
81-
TryCatch ignore_errors(env->isolate());
82-
MaybeLocal<String> maybe_value = env->env_vars()->Get(
83-
env->isolate(),
84-
String::NewFromUtf8(env->isolate(), key).ToLocalChecked());
82+
if (env_vars != nullptr) {
83+
DCHECK_NOT_NULL(isolate);
84+
HandleScope handle_scope(isolate);
85+
TryCatch ignore_errors(isolate);
86+
MaybeLocal<String> maybe_value = env_vars->Get(
87+
isolate, String::NewFromUtf8(isolate, key).ToLocalChecked());
8588
Local<String> value;
8689
if (!maybe_value.ToLocal(&value)) goto fail;
87-
String::Utf8Value utf8_value(env->isolate(), value);
90+
String::Utf8Value utf8_value(isolate, value);
8891
if (*utf8_value == nullptr) goto fail;
8992
*text = std::string(*utf8_value, utf8_value.length());
9093
return true;
@@ -121,7 +124,7 @@ static void SafeGetenv(const FunctionCallbackInfo<Value>& args) {
121124
Isolate* isolate = env->isolate();
122125
Utf8Value strenvtag(isolate, args[0]);
123126
std::string text;
124-
if (!SafeGetenv(*strenvtag, &text, env)) return;
127+
if (!SafeGetenv(*strenvtag, &text, env->env_vars(), isolate)) return;
125128
Local<Value> result =
126129
ToV8Value(isolate->GetCurrentContext(), text).ToLocalChecked();
127130
args.GetReturnValue().Set(result);

‎src/node_internals.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,10 @@ class ThreadPoolWork {
290290
#endif // defined(__POSIX__) && !defined(__ANDROID__) && !defined(__CloudABI__)
291291

292292
namespace credentials {
293-
bool SafeGetenv(const char* key, std::string* text, Environment* env = nullptr);
293+
bool SafeGetenv(const char* key,
294+
std::string* text,
295+
std::shared_ptr<KVStore> env_vars = nullptr,
296+
v8::Isolate* isolate = nullptr);
294297
} // namespace credentials
295298

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

‎src/util.h‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,34 @@ template <typename Inner, typename Outer>
273273
constexpr ContainerOfHelper<Inner, Outer> ContainerOf(Inner Outer::*field,
274274
Inner* pointer);
275275

276+
class KVStore {
277+
public:
278+
KVStore() = default;
279+
virtual ~KVStore() = default;
280+
KVStore(const KVStore&) = delete;
281+
KVStore& operator=(const KVStore&) = delete;
282+
KVStore(KVStore&&) = delete;
283+
KVStore& operator=(KVStore&&) = delete;
284+
285+
virtual v8::MaybeLocal<v8::String> Get(v8::Isolate* isolate,
286+
v8::Local<v8::String> key) const = 0;
287+
virtual v8::Maybe<std::string> Get(const char* key) const = 0;
288+
virtual void Set(v8::Isolate* isolate,
289+
v8::Local<v8::String> key,
290+
v8::Local<v8::String> value) = 0;
291+
virtual int32_t Query(v8::Isolate* isolate,
292+
v8::Local<v8::String> key) const = 0;
293+
virtual int32_t Query(const char* key) const = 0;
294+
virtual void Delete(v8::Isolate* isolate, v8::Local<v8::String> key) = 0;
295+
virtual v8::Local<v8::Array> Enumerate(v8::Isolate* isolate) const = 0;
296+
297+
virtual std::shared_ptr<KVStore> Clone(v8::Isolate* isolate) const;
298+
virtual v8::Maybe<bool> AssignFromObject(v8::Local<v8::Context> context,
299+
v8::Local<v8::Object> entries);
300+
301+
static std::shared_ptr<KVStore> CreateMapKVStore();
302+
};
303+
276304
// Convenience wrapper around v8::String::NewFromOneByte().
277305
inline v8::Local<v8::String> OneByteString(v8::Isolate* isolate,
278306
const char* data,

0 commit comments

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