From c667dad53565e7e039c11ec820f6ccb3c28d8eb0 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Tue, 7 Jan 2025 17:34:14 +0100 Subject: [PATCH] fixup! src: refactor --trace-env to reuse option selection and handling --- src/node_credentials.cc | 2 +- src/node_env_var.cc | 58 ++++++++++++++++++--------------- src/node_internals.h | 8 +++-- test/parallel/test-trace-env.js | 2 +- 4 files changed, 38 insertions(+), 32 deletions(-) diff --git a/src/node_credentials.cc b/src/node_credentials.cc index 50ae6cd1cc5802..de6e48a90552c7 100644 --- a/src/node_credentials.cc +++ b/src/node_credentials.cc @@ -99,7 +99,7 @@ bool SafeGetenv(const char* key, std::string* text, Environment* env) { *text = value.value(); } - TraceEnv(env, "get", key); + TraceEnvVar(env, "get", key); return has_env; } diff --git a/src/node_env_var.cc b/src/node_env_var.cc index 600697bd064d5a..675011736892a5 100644 --- a/src/node_env_var.cc +++ b/src/node_env_var.cc @@ -337,17 +337,17 @@ Maybe KVStore::AssignToObject(v8::Isolate* isolate, return JustVoid(); } -struct TraceEnvOptions { +struct TraceEnvVarOptions { bool print_message : 1 = 0; bool print_js_stack : 1 = 0; bool print_native_stack : 1 = 0; }; template -inline void TraceEnvImpl(Environment* env, - TraceEnvOptions options, - const char* format, - Args&&... args) { +inline void TraceEnvVarImpl(Environment* env, + TraceEnvVarOptions options, + const char* format, + Args&&... args) { if (options.print_message) { fprintf(stderr, format, std::forward(args)...); } @@ -359,8 +359,8 @@ inline void TraceEnvImpl(Environment* env, } } -TraceEnvOptions GetTraceEnvOptions(Environment* env) { - TraceEnvOptions options; +TraceEnvVarOptions GetTraceEnvVarOptions(Environment* env) { + TraceEnvVarOptions options; auto cli_options = env != nullptr ? env->options() : per_process::cli_options->per_isolate->per_env; @@ -376,27 +376,31 @@ TraceEnvOptions GetTraceEnvOptions(Environment* env) { return options; } -void TraceEnv(Environment* env, const char* message) { - TraceEnvImpl(env, GetTraceEnvOptions(env), "[--trace-env] %s\n", message); +void TraceEnvVar(Environment* env, const char* message) { + TraceEnvVarImpl( + env, GetTraceEnvVarOptions(env), "[--trace-env] %s\n", message); } -void TraceEnv(Environment* env, const char* message, const char* key) { - TraceEnvImpl( - env, GetTraceEnvOptions(env), "[--trace-env] %s \"%s\"\n", message, key); +void TraceEnvVar(Environment* env, const char* message, const char* key) { + TraceEnvVarImpl(env, + GetTraceEnvVarOptions(env), + "[--trace-env] %s \"%s\"\n", + message, + key); } -void TraceEnv(Environment* env, - const char* message, - v8::Local key) { - TraceEnvOptions options = GetTraceEnvOptions(env); +void TraceEnvVar(Environment* env, + const char* message, + v8::Local key) { + TraceEnvVarOptions options = GetTraceEnvVarOptions(env); if (options.print_message) { Utf8Value key_utf8(env->isolate(), key); - TraceEnvImpl(env, - options, - "[--trace-env] %s \"%.*s\"\n", - message, - static_cast(key_utf8.length()), - key_utf8.out()); + TraceEnvVarImpl(env, + options, + "[--trace-env] %s \"%.*s\"\n", + message, + static_cast(key_utf8.length()), + key_utf8.out()); } } @@ -413,7 +417,7 @@ static Intercepted EnvGetter(Local property, env->env_vars()->Get(env->isolate(), property.As()); bool has_env = !value_string.IsEmpty(); - TraceEnv(env, "get", property.As()); + TraceEnvVar(env, "get", property.As()); if (has_env) { info.GetReturnValue().Set(value_string.ToLocalChecked()); @@ -453,7 +457,7 @@ static Intercepted EnvSetter(Local property, } env->env_vars()->Set(env->isolate(), key, value_string); - TraceEnv(env, "set", key); + TraceEnvVar(env, "set", key); return Intercepted::kYes; } @@ -465,7 +469,7 @@ static Intercepted EnvQuery(Local property, if (property->IsString()) { int32_t rc = env->env_vars()->Query(env->isolate(), property.As()); bool has_env = (rc != -1); - TraceEnv(env, "query", property.As()); + TraceEnvVar(env, "query", property.As()); if (has_env) { // Return attributes for the property. info.GetReturnValue().Set(v8::None); @@ -482,7 +486,7 @@ static Intercepted EnvDeleter(Local property, if (property->IsString()) { env->env_vars()->Delete(env->isolate(), property.As()); - TraceEnv(env, "delete", property.As()); + TraceEnvVar(env, "delete", property.As()); } // process.env never has non-configurable properties, so always @@ -495,7 +499,7 @@ static void EnvEnumerator(const PropertyCallbackInfo& info) { Environment* env = Environment::GetCurrent(info); CHECK(env->has_run_bootstrapping_code()); - TraceEnv(env, "enumerate environment variables"); + TraceEnvVar(env, "enumerate environment variables"); info.GetReturnValue().Set( env->env_vars()->Enumerate(env->isolate())); diff --git a/src/node_internals.h b/src/node_internals.h index 52e3e5dfc6ba48..382df89a2312f7 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -324,9 +324,11 @@ namespace credentials { bool SafeGetenv(const char* key, std::string* text, Environment* env = nullptr); } // namespace credentials -void TraceEnv(Environment* env, const char* message); -void TraceEnv(Environment* env, const char* message, const char* key); -void TraceEnv(Environment* env, const char* message, v8::Local key); +void TraceEnvVar(Environment* env, const char* message); +void TraceEnvVar(Environment* env, const char* message, const char* key); +void TraceEnvVar(Environment* env, + const char* message, + v8::Local key); void DefineZlibConstants(v8::Local target); v8::Isolate* NewIsolate(v8::Isolate::CreateParams* params, diff --git a/test/parallel/test-trace-env.js b/test/parallel/test-trace-env.js index 6c72ec0209d18b..ba08e0af2aad1d 100644 --- a/test/parallel/test-trace-env.js +++ b/test/parallel/test-trace-env.js @@ -9,7 +9,7 @@ const fixtures = require('../common/fixtures'); // Check reads from the Node.js internals. spawnSyncAndAssert(process.execPath, ['--trace-env', fixtures.path('empty.js')], { stderr(output) { - // This is a non-exhaustive list of thes checked + // This is a non-exhaustive list of the environment variables checked // during startup of an empty script at the time this test was written. // If the internals remove any one of them, the checks here can be updated // accordingly.