Skip to content

Commit

Permalink
src: Modified src/node_env_var.cc as per Joyee Cheung’s review comments
Browse files Browse the repository at this point in the history
Below review comments are taken care in this submission:
1. followed snake case naming convention for local variables
2. dropped sizeof(char) since it is obvious value 1
3. Used MaybeLocal<String> instead of Local<String> to check
for
   empty strings and throw exception if empty.

Fixes: nodejs#27211
Refs: https://v8docs.nodesource.com/node-4.8/annotated.html
  • Loading branch information
devasci authored and addaleax committed Aug 20, 2019
1 parent eef9b8b commit 7db9f03
Showing 1 changed file with 14 additions and 10 deletions.
24 changes: 14 additions & 10 deletions src/node_env_var.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,19 +85,19 @@ Local<String> RealEnvStore::Get(Isolate* isolate,

node::Utf8Value key(isolate, property);
char* val = nullptr;
size_t initSz = 256;
size_t init_sz = 256;

// Allocate 256 bytes initially, if not enough reallocate.
val = Malloc(sizeof(char) * initSz);
val = Malloc(init_sz);

int ret = uv_os_getenv(*key, val, &initSz);
int ret = uv_os_getenv(*key, val, &init_sz);

if (UV_ENOBUFS == ret) {
// Buffer is not large enough, reallocate to the updated initSz
// Buffer is not large enough, reallocate to the updated init_sz
// and fetch env value again.
val = Realloc(val, sizeof(char) * initSz);
val = Realloc(val, init_sz);

ret = uv_os_getenv(*key, val, &initSz);
ret = uv_os_getenv(*key, val, &init_sz);

// Still failed to fetch env value return emptry string.
if (UV_ENOBUFS == ret || UV_ENOENT == ret) {
Expand All @@ -107,13 +107,17 @@ Local<String> RealEnvStore::Get(Isolate* isolate,
return Local<String>();
}

Local<String> valueString =
String::NewFromUtf8(isolate, val, NewStringType::kNormal)
.ToLocalChecked();
MaybeLocal<String> value_string =
String::NewFromUtf8(isolate, val, NewStringType::kNormal);

if (value_string.IsEmpty()) {
isolate->ThrowException(ERR_STRING_TOO_LONG(isolate));
return Local<String>();
}

if (nullptr != val) free(val);

return valueString;
return value_string.ToLocalChecked();
}

void RealEnvStore::Set(Isolate* isolate,
Expand Down

0 comments on commit 7db9f03

Please sign in to comment.