From 93fc596a4ce62bc1166003b2f2550796cc783ba1 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 30 Sep 2018 13:41:57 -0400 Subject: [PATCH 1/3] src: clean up zlib write code Split the existing `Write()` method into one that takes care of translating the arguments from JS to C++, and a non-static method for the actual write operations, as well as some minor stylistic drive-by fixes. --- src/node_zlib.cc | 80 +++++++++++++++++++++++++----------------------- 1 file changed, 42 insertions(+), 38 deletions(-) diff --git a/src/node_zlib.cc b/src/node_zlib.cc index d0cc54c9436582..980089768dd27b 100644 --- a/src/node_zlib.cc +++ b/src/node_zlib.cc @@ -138,24 +138,15 @@ class ZCtx : public AsyncWrap, public ThreadPoolWork { // write(flush, in, in_off, in_len, out, out_off, out_len) template static void Write(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + Local context = env->context(); CHECK_EQ(args.Length(), 7); - ZCtx* ctx; - ASSIGN_OR_RETURN_UNWRAP(&ctx, args.Holder()); - CHECK(ctx->init_done_ && "write before init"); - CHECK(ctx->mode_ != NONE && "already finalized"); - - CHECK_EQ(false, ctx->write_in_progress_ && "write already in progress"); - CHECK_EQ(false, ctx->pending_close_ && "close is pending"); - ctx->write_in_progress_ = true; - ctx->Ref(); + uint32_t in_off, in_len, out_off, out_len, flush; + Bytef* in; + Bytef* out; CHECK_EQ(false, args[0]->IsUndefined() && "must provide flush value"); - - Environment* env = ctx->env(); - Local context = env->context(); - - unsigned int flush; if (!args[0]->Uint32Value(context).To(&flush)) return; if (flush != Z_NO_FLUSH && @@ -167,12 +158,6 @@ class ZCtx : public AsyncWrap, public ThreadPoolWork { CHECK(0 && "Invalid flush value"); } - AllocScope alloc_scope(ctx); - - Bytef* in; - Bytef* out; - uint32_t in_off, in_len, out_off, out_len; - if (args[1]->IsNull()) { // just a flush in = nullptr; @@ -180,43 +165,62 @@ class ZCtx : public AsyncWrap, public ThreadPoolWork { in_off = 0; } else { CHECK(Buffer::HasInstance(args[1])); - Local in_buf; - in_buf = args[1]->ToObject(context).ToLocalChecked(); + Local in_buf = args[1].As(); if (!args[2]->Uint32Value(context).To(&in_off)) return; if (!args[3]->Uint32Value(context).To(&in_len)) return; CHECK(Buffer::IsWithinBounds(in_off, in_len, Buffer::Length(in_buf))); - in = reinterpret_cast(Buffer::Data(in_buf) + in_off); + in = reinterpret_cast(Buffer::Data(in_buf) + in_off); } CHECK(Buffer::HasInstance(args[4])); - Local out_buf = args[4]->ToObject(context).ToLocalChecked(); + Local out_buf = args[4].As(); if (!args[5]->Uint32Value(context).To(&out_off)) return; if (!args[6]->Uint32Value(context).To(&out_len)) return; CHECK(Buffer::IsWithinBounds(out_off, out_len, Buffer::Length(out_buf))); - out = reinterpret_cast(Buffer::Data(out_buf) + out_off); + out = reinterpret_cast(Buffer::Data(out_buf) + out_off); + + ZCtx* ctx; + ASSIGN_OR_RETURN_UNWRAP(&ctx, args.Holder()); + + ctx->Write(flush, in, in_off, in_len, out, out_off, out_len); + } + + template + void Write(uint32_t flush, + Bytef* in, uint32_t in_off, uint32_t in_len, + Bytef* out, uint32_t out_off, uint32_t out_len) { + AllocScope alloc_scope(this); + + CHECK(init_done_ && "write before init"); + CHECK(mode_ != NONE && "already finalized"); + + CHECK_EQ(false, write_in_progress_); + CHECK_EQ(false, pending_close_); + write_in_progress_ = true; + Ref(); - ctx->strm_.avail_in = in_len; - ctx->strm_.next_in = in; - ctx->strm_.avail_out = out_len; - ctx->strm_.next_out = out; - ctx->flush_ = flush; + strm_.avail_in = in_len; + strm_.next_in = in; + strm_.avail_out = out_len; + strm_.next_out = out; + flush_ = flush; if (!async) { // sync version - env->PrintSyncTrace(); - ctx->DoThreadPoolWork(); - if (ctx->CheckError()) { - ctx->write_result_[0] = ctx->strm_.avail_out; - ctx->write_result_[1] = ctx->strm_.avail_in; - ctx->write_in_progress_ = false; + env()->PrintSyncTrace(); + DoThreadPoolWork(); + if (CheckError()) { + write_result_[0] = strm_.avail_out; + write_result_[1] = strm_.avail_in; + write_in_progress_ = false; } - ctx->Unref(); + Unref(); return; } // async version - ctx->ScheduleWork(); + ScheduleWork(); } // thread pool! From e84f6d8f99e04839f9c6e8fbf1d40947e752d5db Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 30 Sep 2018 13:58:15 -0400 Subject: [PATCH 2/3] fixup! src: clean up zlib write code --- src/node_zlib.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/node_zlib.cc b/src/node_zlib.cc index 980089768dd27b..1614321c0f24bb 100644 --- a/src/node_zlib.cc +++ b/src/node_zlib.cc @@ -183,13 +183,13 @@ class ZCtx : public AsyncWrap, public ThreadPoolWork { ZCtx* ctx; ASSIGN_OR_RETURN_UNWRAP(&ctx, args.Holder()); - ctx->Write(flush, in, in_off, in_len, out, out_off, out_len); + ctx->Write(flush, in, in_len, out, out_len); } template void Write(uint32_t flush, - Bytef* in, uint32_t in_off, uint32_t in_len, - Bytef* out, uint32_t out_off, uint32_t out_len) { + Bytef* in, uint32_t in_len, + Bytef* out, uint32_t out_len) { AllocScope alloc_scope(this); CHECK(init_done_ && "write before init"); From 592f48a819365a96df518ccf2a3fecd40aa7658f Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 30 Sep 2018 14:00:33 -0400 Subject: [PATCH 3/3] fixup! src: clean up zlib write code --- src/node_zlib.cc | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/node_zlib.cc b/src/node_zlib.cc index 1614321c0f24bb..80ebc1836f8ef6 100644 --- a/src/node_zlib.cc +++ b/src/node_zlib.cc @@ -143,8 +143,8 @@ class ZCtx : public AsyncWrap, public ThreadPoolWork { CHECK_EQ(args.Length(), 7); uint32_t in_off, in_len, out_off, out_len, flush; - Bytef* in; - Bytef* out; + char* in; + char* out; CHECK_EQ(false, args[0]->IsUndefined() && "must provide flush value"); if (!args[0]->Uint32Value(context).To(&flush)) return; @@ -170,7 +170,7 @@ class ZCtx : public AsyncWrap, public ThreadPoolWork { if (!args[3]->Uint32Value(context).To(&in_len)) return; CHECK(Buffer::IsWithinBounds(in_off, in_len, Buffer::Length(in_buf))); - in = reinterpret_cast(Buffer::Data(in_buf) + in_off); + in = Buffer::Data(in_buf) + in_off; } CHECK(Buffer::HasInstance(args[4])); @@ -178,7 +178,7 @@ class ZCtx : public AsyncWrap, public ThreadPoolWork { if (!args[5]->Uint32Value(context).To(&out_off)) return; if (!args[6]->Uint32Value(context).To(&out_len)) return; CHECK(Buffer::IsWithinBounds(out_off, out_len, Buffer::Length(out_buf))); - out = reinterpret_cast(Buffer::Data(out_buf) + out_off); + out = Buffer::Data(out_buf) + out_off; ZCtx* ctx; ASSIGN_OR_RETURN_UNWRAP(&ctx, args.Holder()); @@ -188,8 +188,8 @@ class ZCtx : public AsyncWrap, public ThreadPoolWork { template void Write(uint32_t flush, - Bytef* in, uint32_t in_len, - Bytef* out, uint32_t out_len) { + char* in, uint32_t in_len, + char* out, uint32_t out_len) { AllocScope alloc_scope(this); CHECK(init_done_ && "write before init"); @@ -201,9 +201,9 @@ class ZCtx : public AsyncWrap, public ThreadPoolWork { Ref(); strm_.avail_in = in_len; - strm_.next_in = in; + strm_.next_in = reinterpret_cast(in); strm_.avail_out = out_len; - strm_.next_out = out; + strm_.next_out = reinterpret_cast(out); flush_ = flush; if (!async) {