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.

Sign up for GitHub

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

Jump to bottom

zlib: reduce number of static internal methods #20674

Closed
wants to merge 2 commits into from
Closed

zlib: reduce number of static internal methods #20674

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ace4f0e
zlib: reduce number of static internal methods
addaleax May 11, 2018
2e92688
[squash] refer to node_errors.h in TODO comment
addaleax May 13, 2018
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
162 changes: 74 additions & 88 deletions src/node_zlib.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ class ZCtx : public AsyncWrap, public ThreadPoolWork {
// sync version
env->PrintSyncTrace();
ctx->DoThreadPoolWork();
if (CheckError(ctx)) {
if (ctx->CheckError()) {
ctx->write_result_[0] = ctx->strm_.avail_out;
ctx->write_result_[1] = ctx->strm_.avail_in;
ctx->write_in_progress_ = false;
Expand All @@ -215,53 +215,43 @@ class ZCtx : public AsyncWrap, public ThreadPoolWork {
ctx->ScheduleWork();
}

// TODO(addaleax): Make these methods non-static. It's a significant bunch
// of churn that's better left for a separate PR.
void DoThreadPoolWork() override {
Process(this);
}

void AfterThreadPoolWork(int status) override {
After(this, status);
}

// thread pool!
// This function may be called multiple times on the uv_work pool
// for a single write() call, until all of the input bytes have
// been consumed.
static void Process(ZCtx* ctx) {
void DoThreadPoolWork() {
const Bytef* next_expected_header_byte = nullptr;

// If the avail_out is left at 0, then it means that it ran out
// of room. If there was avail_out left over, then it means
// that all of the input was consumed.
switch (ctx->mode_) {
switch (mode_) {
case DEFLATE:
case GZIP:
case DEFLATERAW:
ctx->err_ = deflate(&ctx->strm_, ctx->flush_);
err_ = deflate(&strm_, flush_);
break;
case UNZIP:
if (ctx->strm_.avail_in > 0) {
next_expected_header_byte = ctx->strm_.next_in;
if (strm_.avail_in > 0) {
next_expected_header_byte = strm_.next_in;
}

switch (ctx->gzip_id_bytes_read_) {
switch (gzip_id_bytes_read_) {
case 0:
if (next_expected_header_byte == nullptr) {
break;
}

if (*next_expected_header_byte == GZIP_HEADER_ID1) {
ctx->gzip_id_bytes_read_ = 1;
gzip_id_bytes_read_ = 1;
next_expected_header_byte++;

if (ctx->strm_.avail_in == 1) {
if (strm_.avail_in == 1) {
// The only available byte was already read.
break;
}
} else {
ctx->mode_ = INFLATE;
mode_ = INFLATE;
break;
}

Expand All @@ -272,12 +262,12 @@ class ZCtx : public AsyncWrap, public ThreadPoolWork {
}

if (*next_expected_header_byte == GZIP_HEADER_ID2) {
ctx->gzip_id_bytes_read_ = 2;
ctx->mode_ = GUNZIP;
gzip_id_bytes_read_ = 2;
mode_ = GUNZIP;
} else {
// There is no actual difference between INFLATE and INFLATERAW
// (after initialization).
ctx->mode_ = INFLATE;
mode_ = INFLATE;
}

break;
Expand All @@ -289,39 +279,37 @@ class ZCtx : public AsyncWrap, public ThreadPoolWork {
case INFLATE:
case GUNZIP:
case INFLATERAW:
ctx->err_ = inflate(&ctx->strm_, ctx->flush_);
err_ = inflate(&strm_, flush_);

// If data was encoded with dictionary (INFLATERAW will have it set in
// SetDictionary, don't repeat that here)
if (ctx->mode_ != INFLATERAW &&
ctx->err_ == Z_NEED_DICT &&
ctx->dictionary_ != nullptr) {
if (mode_ != INFLATERAW &&
err_ == Z_NEED_DICT &&
dictionary_ != nullptr) {
// Load it
ctx->err_ = inflateSetDictionary(&ctx->strm_,
ctx->dictionary_,
ctx->dictionary_len_);
if (ctx->err_ == Z_OK) {
err_ = inflateSetDictionary(&strm_, dictionary_, dictionary_len_);
if (err_ == Z_OK) {
// And try to decode again
ctx->err_ = inflate(&ctx->strm_, ctx->flush_);
} else if (ctx->err_ == Z_DATA_ERROR) {
err_ = inflate(&strm_, flush_);
} else if (err_ == Z_DATA_ERROR) {
// Both inflateSetDictionary() and inflate() return Z_DATA_ERROR.
// Make it possible for After() to tell a bad dictionary from bad
// input.
ctx->err_ = Z_NEED_DICT;
err_ = Z_NEED_DICT;
}
}

while (ctx->strm_.avail_in > 0 &&
ctx->mode_ == GUNZIP &&
ctx->err_ == Z_STREAM_END &&
ctx->strm_.next_in[0] != 0x00) {
while (strm_.avail_in > 0 &&
mode_ == GUNZIP &&
err_ == Z_STREAM_END &&
strm_.next_in[0] != 0x00) {
// Bytes remain in input buffer. Perhaps this is another compressed
// member in the same archive, or just trailing garbage.
// Trailing zero bytes are okay, though, since they are frequently
// used for padding.

Reset(ctx);
ctx->err_ = inflate(&ctx->strm_, ctx->flush_);
Reset();
err_ = inflate(&strm_, flush_);
}
break;
default:
Expand All @@ -336,27 +324,27 @@ class ZCtx : public AsyncWrap, public ThreadPoolWork {
}


static bool CheckError(ZCtx* ctx) {
bool CheckError() {
// Acceptable error states depend on the type of zlib stream.
switch (ctx->err_) {
switch (err_) {
case Z_OK:
case Z_BUF_ERROR:
if (ctx->strm_.avail_out != 0 && ctx->flush_ == Z_FINISH) {
ZCtx::Error(ctx, "unexpected end of file");
if (strm_.avail_out != 0 && flush_ == Z_FINISH) {
Error("unexpected end of file");
return false;
}
case Z_STREAM_END:
// normal statuses, not fatal
break;
case Z_NEED_DICT:
if (ctx->dictionary_ == nullptr)
ZCtx::Error(ctx, "Missing dictionary");
if (dictionary_ == nullptr)
Error("Missing dictionary");
else
ZCtx::Error(ctx, "Bad dictionary");
Error("Bad dictionary");
return false;
default:
// something else.
ZCtx::Error(ctx, "Zlib error");
Error("Zlib error");
return false;
}

Expand All @@ -365,59 +353,57 @@ class ZCtx : public AsyncWrap, public ThreadPoolWork {


// v8 land!
static void After(ZCtx* ctx, int status) {
Environment* env = ctx->env();
ctx->write_in_progress_ = false;
void AfterThreadPoolWork(int status) {
write_in_progress_ = false;

if (status == UV_ECANCELED) {
ctx->Close();
Close();
return;
}

CHECK_EQ(status, 0);

HandleScope handle_scope(env->isolate());
Context::Scope context_scope(env->context());
HandleScope handle_scope(env()->isolate());
Context::Scope context_scope(env()->context());

if (!CheckError(ctx))
if (!CheckError())
return;

ctx->write_result_[0] = ctx->strm_.avail_out;
ctx->write_result_[1] = ctx->strm_.avail_in;
write_result_[0] = strm_.avail_out;
write_result_[1] = strm_.avail_in;

// call the write() cb
Local<Function> cb = PersistentToLocal(env->isolate(),
ctx->write_js_callback_);
ctx->MakeCallback(cb, 0, nullptr);
Local<Function> cb = PersistentToLocal(env()->isolate(),
write_js_callback_);
MakeCallback(cb, 0, nullptr);

ctx->Unref();
if (ctx->pending_close_)
ctx->Close();
Unref();
if (pending_close_)
Close();
}

static void Error(ZCtx* ctx, const char* message) {
Environment* env = ctx->env();

// TODO(addaleax): Switch to modern error system (node_errors.h).
void Error(const char* message) {
// If you hit this assertion, you forgot to enter the v8::Context first.
CHECK_EQ(env->context(), env->isolate()->GetCurrentContext());
CHECK_EQ(env()->context(), env()->isolate()->GetCurrentContext());

if (ctx->strm_.msg != nullptr) {
message = ctx->strm_.msg;
if (strm_.msg != nullptr) {
message = strm_.msg;
}

HandleScope scope(env->isolate());
HandleScope scope(env()->isolate());
Local<Value> args[2] = {
OneByteString(env->isolate(), message),
Number::New(env->isolate(), ctx->err_)
OneByteString(env()->isolate(), message),
Number::New(env()->isolate(), err_)
};
ctx->MakeCallback(env->onerror_string(), arraysize(args), args);
MakeCallback(env()->onerror_string(), arraysize(args), args);

// no hope of rescue.
if (ctx->write_in_progress_)
ctx->Unref();
ctx->write_in_progress_ = false;
if (ctx->pending_close_)
ctx->Close();
if (write_in_progress_)
Unref();
write_in_progress_ = false;
if (pending_close_)
Close();
}

static void New(const FunctionCallbackInfo<Value>& args) {
Expand Down Expand Up @@ -510,7 +496,7 @@ class ZCtx : public AsyncWrap, public ThreadPoolWork {
static void Reset(const FunctionCallbackInfo<Value> &args) {
ZCtx* ctx;
ASSIGN_OR_RETURN_UNWRAP(&ctx, args.Holder());
Reset(ctx);
ctx->Reset();
SetDictionary(ctx);
}

Expand Down Expand Up @@ -613,7 +599,7 @@ class ZCtx : public AsyncWrap, public ThreadPoolWork {
}

if (ctx->err_ != Z_OK) {
ZCtx::Error(ctx, "Failed to set dictionary");
ctx->Error("Failed to set dictionary");
}
}

Expand All @@ -630,30 +616,30 @@ class ZCtx : public AsyncWrap, public ThreadPoolWork {
}

if (ctx->err_ != Z_OK && ctx->err_ != Z_BUF_ERROR) {
ZCtx::Error(ctx, "Failed to set parameters");
ctx->Error("Failed to set parameters");
}
}

static void Reset(ZCtx* ctx) {
ctx->err_ = Z_OK;
void Reset() {
err_ = Z_OK;

switch (ctx->mode_) {
switch (mode_) {
case DEFLATE:
case DEFLATERAW:
case GZIP:
ctx->err_ = deflateReset(&ctx->strm_);
err_ = deflateReset(&strm_);
break;
case INFLATE:
case INFLATERAW:
case GUNZIP:
ctx->err_ = inflateReset(&ctx->strm_);
err_ = inflateReset(&strm_);
break;
default:
break;
}

if (ctx->err_ != Z_OK) {
ZCtx::Error(ctx, "Failed to reset stream");
if (err_ != Z_OK) {
Error("Failed to reset stream");
}
}

Expand Down