Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor/code optimization #12187

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/async-wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ void AsyncWrap::DestroyIdsCb(uv_idle_t* handle) {
// Want each callback to be cleaned up after itself, instead of cleaning
// them all up after the while() loop completes.
HandleScope scope(env->isolate());
Local<Value> argv = Number::New(env->isolate(), current_id);
Local<Value> argv = Number::New(env->isolate(), static_cast<double>(current_id));
MaybeLocal<Value> ret = fn->Call(
env->context(), Undefined(env->isolate()), 1, &argv);

Expand Down Expand Up @@ -260,7 +260,7 @@ AsyncWrap::AsyncWrap(Environment* env,
CHECK_GE(object->InternalFieldCount(), 1);

// Shift provider value over to prevent id collision.
persistent().SetWrapperClassId(NODE_ASYNC_ID_OFFSET + provider);
persistent().SetWrapperClassId(static_cast<uint16_t>(NODE_ASYNC_ID_OFFSET + provider));

Local<Function> init_fn = env->async_hooks_init_function();

Expand All @@ -277,14 +277,14 @@ AsyncWrap::AsyncWrap(Environment* env,
HandleScope scope(env->isolate());

Local<Value> argv[] = {
Number::New(env->isolate(), get_uid()),
Number::New(env->isolate(), static_cast<double>(get_uid())),
Int32::New(env->isolate(), provider),
Null(env->isolate()),
Null(env->isolate())
};

if (parent != nullptr) {
argv[2] = Number::New(env->isolate(), parent->get_uid());
argv[2] = Number::New(env->isolate(), static_cast<double>(parent->get_uid()));
argv[3] = parent->object();
}

Expand Down Expand Up @@ -320,7 +320,7 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,

Local<Function> pre_fn = env()->async_hooks_pre_function();
Local<Function> post_fn = env()->async_hooks_post_function();
Local<Value> uid = Number::New(env()->isolate(), get_uid());
Local<Value> uid = Number::New(env()->isolate(), static_cast<double>(get_uid()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would recommend against changing these things here, there would be quite a few conflicts with #11883

Local<Object> context = object();
Local<Object> domain;
bool has_domain = false;
Expand Down
4 changes: 2 additions & 2 deletions src/cares_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1150,7 +1150,7 @@ static void GetAddrInfo(const FunctionCallbackInfo<Value>& args) {
node::Utf8Value hostname(env->isolate(), args[1]);

int32_t flags = (args[3]->IsInt32()) ? args[3]->Int32Value() : 0;
int family;
int family = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems confusing for the same reason as @mscdex’ comment below … :/


switch (args[2]->Int32Value()) {
case 0:
Expand Down Expand Up @@ -1263,7 +1263,7 @@ static void SetServers(const FunctionCallbackInfo<Value>& args) {
ares_addr_node* servers = new ares_addr_node[len];
ares_addr_node* last = nullptr;

int err;
int err = 0;

for (uint32_t i = 0; i < len; i++) {
CHECK(arr->Get(i)->IsArray());
Expand Down
2 changes: 1 addition & 1 deletion src/js_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ void JSStream::New(const FunctionCallbackInfo<Value>& args) {
// normal function.
CHECK(args.IsConstructCall());
Environment* env = Environment::GetCurrent(args);
JSStream* wrap;
JSStream* wrap = NULL;
Copy link
Contributor

@mscdex mscdex Apr 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't necessary, either the variable will already be assigned below or the process aborts because of an unreachable condition. Also, nullptr should be used anyway for C++.


if (args.Length() == 0) {
wrap = new JSStream(env, args.This(), nullptr);
Expand Down
8 changes: 4 additions & 4 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2303,10 +2303,10 @@ void MemoryUsage(const FunctionCallbackInfo<Value>& args) {
Local<ArrayBuffer> ab = array->Buffer();
double* fields = static_cast<double*>(ab->GetContents().Data());

fields[0] = rss;
fields[1] = v8_heap_stats.total_heap_size();
fields[2] = v8_heap_stats.used_heap_size();
fields[3] = isolate->AdjustAmountOfExternalAllocatedMemory(0);
fields[0] = static_cast<double>(rss);
fields[1] = static_cast<double>(v8_heap_stats.total_heap_size());
fields[2] = static_cast<double>(v8_heap_stats.used_heap_size());
fields[3] = static_cast<double>(isolate->AdjustAmountOfExternalAllocatedMemory(0)); //possible loss data
}


Expand Down
8 changes: 4 additions & 4 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3371,7 +3371,7 @@ void CipherBase::Init(const FunctionCallbackInfo<Value>& args) {
const node::Utf8Value cipher_type(args.GetIsolate(), args[0]);
const char* key_buf = Buffer::Data(args[1]);
ssize_t key_buf_len = Buffer::Length(args[1]);
cipher->Init(*cipher_type, key_buf, key_buf_len);
cipher->Init(*cipher_type, key_buf, static_cast<int>(key_buf_len));
}


Expand Down Expand Up @@ -3437,7 +3437,7 @@ void CipherBase::InitIv(const FunctionCallbackInfo<Value>& args) {
const char* key_buf = Buffer::Data(args[1]);
ssize_t iv_len = Buffer::Length(args[2]);
const char* iv_buf = Buffer::Data(args[2]);
cipher->InitIv(*cipher_type, key_buf, key_len, iv_buf, iv_len);
cipher->InitIv(*cipher_type, key_buf, static_cast<int>(key_len), iv_buf, static_cast<int>(iv_len));
}


Expand Down Expand Up @@ -4378,7 +4378,7 @@ void Verify::VerifyFinal(const FunctionCallbackInfo<Value>& args) {
}

bool verify_result;
Error err = verify->VerifyFinal(kbuf, klen, hbuf, hlen, &verify_result);
Error err = verify->VerifyFinal(kbuf, klen, hbuf, static_cast<int>(hlen), &verify_result);
if (args[1]->IsString())
delete[] hbuf;
if (err != kSignOk)
Expand Down Expand Up @@ -4508,7 +4508,7 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo<Value>& args) {
reinterpret_cast<const unsigned char*>(buf),
len,
&out_value,
&out_len);
static_cast<int>(&out_len));

if (out_len == 0 || !r) {
delete[] out_value;
Expand Down
2 changes: 1 addition & 1 deletion src/node_crypto_clienthello-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ inline void ClientHelloParser::Reset() {
extension_offset_ = 0;
session_size_ = 0;
session_id_ = nullptr;
tls_ticket_size_ = -1;
tls_ticket_size_ = static_cast<uint16_t>(-1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 , this does increase readability a bit

tls_ticket_ = nullptr;
servername_size_ = 0;
servername_ = nullptr;
Expand Down
22 changes: 11 additions & 11 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -224,11 +224,11 @@ static void After(uv_fs_t *req) {
break;

case UV_FS_OPEN:
argv[1] = Integer::New(env->isolate(), req->result);
argv[1] = Integer::New(env->isolate(), static_cast<int32_t>(req->result)); //possible los of data
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: los (also: practically speaking there’s absolutely nothing to worry about, you could just leave the comment away)

break;

case UV_FS_WRITE:
argv[1] = Integer::New(env->isolate(), req->result);
argv[1] = Integer::New(env->isolate(), static_cast<int32_t>(req->result));
break;

case UV_FS_MKDTEMP:
Expand Down Expand Up @@ -281,7 +281,7 @@ static void After(uv_fs_t *req) {

case UV_FS_READ:
// Buffer interface
argv[1] = Integer::New(env->isolate(), req->result);
argv[1] = Integer::New(env->isolate(), static_cast<int32_t>(req->result));
break;

case UV_FS_SCANDIR:
Expand Down Expand Up @@ -442,19 +442,19 @@ static void Close(const FunctionCallbackInfo<Value>& args) {


void FillStatsArray(double* fields, const uv_stat_t* s) {
fields[0] = s->st_dev;
fields[1] = s->st_mode;
fields[2] = s->st_nlink;
fields[3] = s->st_uid;
fields[4] = s->st_gid;
fields[5] = s->st_rdev;
fields[0] = static_cast<double>(s->st_dev);
fields[1] = static_cast<double>(s->st_mode);
fields[2] = static_cast<double>(s->st_nlink);
fields[3] = static_cast<double>(s->st_uid);
fields[4] = static_cast<double>(s->st_gid);
fields[5] = static_cast<double>(s->st_rdev);
#if defined(__POSIX__)
fields[6] = s->st_blksize;
#else
fields[6] = -1;
#endif
fields[7] = s->st_ino;
fields[8] = s->st_size;
fields[7] = static_cast<double>(s->st_ino);
fields[8] = static_cast<double>(s->st_size);
#if defined(__POSIX__)
fields[9] = s->st_blocks;
#else
Expand Down
4 changes: 2 additions & 2 deletions src/node_http_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ class Parser : public AsyncWrap {
return -1;
}

return head_response->IntegerValue();
return static_cast<int>(head_response->IntegerValue());
}


Expand All @@ -339,7 +339,7 @@ class Parser : public AsyncWrap {

Local<Value> argv[3] = {
current_buffer_,
Integer::NewFromUnsigned(env()->isolate(), at - current_buffer_data_),
Integer::NewFromUnsigned(env()->isolate(), static_cast<uint32_t>(at - current_buffer_data_)),
Integer::NewFromUnsigned(env()->isolate(), length)
};

Expand Down
16 changes: 8 additions & 8 deletions src/node_os.cc
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,12 @@ static void GetCPUInfo(const FunctionCallbackInfo<Value>& args) {
for (i = 0, field_idx = 0; i < count; i++) {
uv_cpu_info_t* ci = cpu_infos + i;

fields[field_idx++] = ci->speed;
fields[field_idx++] = ci->cpu_times.user;
fields[field_idx++] = ci->cpu_times.nice;
fields[field_idx++] = ci->cpu_times.sys;
fields[field_idx++] = ci->cpu_times.idle;
fields[field_idx++] = ci->cpu_times.irq;
fields[field_idx++] = static_cast<double>(ci->speed);
fields[field_idx++] = static_cast<double>(ci->cpu_times.user);
fields[field_idx++] = static_cast<double>(ci->cpu_times.nice);
fields[field_idx++] = static_cast<double>(ci->cpu_times.sys);
fields[field_idx++] = static_cast<double>(ci->cpu_times.idle);
fields[field_idx++] = static_cast<double>(ci->cpu_times.irq);
model_argv[model_idx++] = OneByteString(env->isolate(), ci->model);

if (model_idx >= NODE_PUSH_VAL_TO_ARRAY_MAX) {
Expand All @@ -193,15 +193,15 @@ static void GetCPUInfo(const FunctionCallbackInfo<Value>& args) {


static void GetFreeMemory(const FunctionCallbackInfo<Value>& args) {
double amount = uv_get_free_memory();
double amount = static_cast<double>(uv_get_free_memory());
if (amount < 0)
return;
args.GetReturnValue().Set(amount);
}


static void GetTotalMemory(const FunctionCallbackInfo<Value>& args) {
double amount = uv_get_total_memory();
double amount = static_cast<double>(uv_get_total_memory());
if (amount < 0)
return;
args.GetReturnValue().Set(amount);
Expand Down
12 changes: 6 additions & 6 deletions src/node_url.cc
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ namespace url {
}
if (dots < 3 && ch != '.')
goto end;
*piece_pointer = *piece_pointer * 0x100 + value;
*piece_pointer = static_cast<uint16_t>(*piece_pointer * 0x100 + value); // possible loss data
if (dots & 0x1)
piece_pointer++;
if (ch != kEOL) {
Expand All @@ -190,12 +190,12 @@ namespace url {
default:
goto end;
}
*piece_pointer = value;
*piece_pointer = static_cast<uint16_t>(value);
piece_pointer++;
}

if (compress_pointer != nullptr) {
swaps = piece_pointer - compress_pointer;
swaps = static_cast<int>(piece_pointer - compress_pointer);
piece_pointer = last_piece - 1;
while (piece_pointer != &host->value.ipv6[0] && swaps > 0) {
uint16_t temp = *piece_pointer;
Expand Down Expand Up @@ -266,7 +266,7 @@ namespace url {

while (pointer <= end) {
const char ch = pointer < end ? pointer[0] : kEOL;
const int remaining = end - pointer - 1;
const int remaining = static_cast<const int>(end - pointer - 1);
if (ch == '.' || ch == kEOL) {
if (++parts > 4)
goto end;
Expand Down Expand Up @@ -299,10 +299,10 @@ namespace url {
}

type = HOST_TYPE_IPV4;
val = numbers[parts - 1];
val = static_cast<uint32_t>(numbers[parts - 1]);
for (int n = 0; n < parts - 1; n++) {
double b = 3 - n;
val += numbers[n] * pow(256, b);
val += static_cast<uint32_t>(numbers[n] * pow(256, b));
}

host->value.ipv4 = val;
Expand Down
2 changes: 1 addition & 1 deletion src/stream_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ void StreamBase::EmitData(ssize_t nread,
Environment* env = env_;

Local<Value> argv[] = {
Integer::New(env->isolate(), nread),
Integer::New(env->isolate(), static_cast<int32_t>(nread)),
buf,
handle
};
Expand Down
4 changes: 2 additions & 2 deletions src/stream_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -251,9 +251,9 @@ void StreamWrap::OnReadCommon(uv_stream_t* handle,

if (nread > 0) {
if (wrap->is_tcp()) {
NODE_COUNT_NET_BYTES_RECV(nread);
NODE_COUNT_NET_BYTES_RECV(static_cast<int>(nread));
} else if (wrap->is_named_pipe()) {
NODE_COUNT_PIPE_BYTES_RECV(nread);
NODE_COUNT_PIPE_BYTES_RECV(static_cast<int>(nread));
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/string_bytes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ size_t hex_decode(char* buf,
unsigned b = unhex(src[i * 2 + 1]);
if (!~a || !~b)
return i;
buf[i] = (a << 4) | b;
buf[i] = static_cast<char>((a << 4) | b);
}

return i;
Expand Down Expand Up @@ -515,7 +515,7 @@ static bool contains_non_ascii(const char* src, size_t len) {


#if defined(_WIN64) || defined(_LP64)
const uintptr_t mask = 0x8080808080808080ll;
const uintptr_t mask = static_cast<uintptr_t>(0x8080808080808080ll);
#else
const uintptr_t mask = 0x80808080l;
#endif
Expand Down
2 changes: 1 addition & 1 deletion src/string_search.h
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ size_t StringSearch<Char>::BoyerMooreSearch(
static_cast<Char>(last_char));
} else {
int gs_shift = good_suffix_shift[j + 1];
int bc_occ = CharOccurrence(bad_char_occurrence, c);
int bc_occ = CharOccurrence(bad_char_occurrence, static_cast<uint8_t>(c));
int shift = j - bc_occ;
if (gs_shift > shift) {
shift = gs_shift;
Expand Down