diff options
author | Anna Henningsen <anna@addaleax.net> | 2022-03-10 14:51:12 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-03-10 14:51:12 +0300 |
commit | 72e83fcea6eadffed67b7380f7ef715b98f6727e (patch) | |
tree | 1004859cd6dfaf70d013341a5a11a7b22c6cefc8 /src/node_zlib.cc | |
parent | a30e0de0059f7d49e1adbbc2f8197b08069a20a6 (diff) |
src: perform minor cleanups on zlib code
- Use `final` to indicate the classes that we actually
instantiate
- Properly use `const` (and the necessary associated `const_cast`
for zlib because we don’t define `ZLIB_CONST` and allow shared
builds)
- Store the JS callback in an internal field rather than a `Global`
(which improves memory leak debugging capabilities, removes
a potential future memory leak footgun, and aligns the code
with the rest of the codebase more closely)
- Other minor C++ cleanup
PR-URL: https://github.com/nodejs/node/pull/42247
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Diffstat (limited to 'src/node_zlib.cc')
-rw-r--r-- | src/node_zlib.cc | 49 |
1 files changed, 27 insertions, 22 deletions
diff --git a/src/node_zlib.cc b/src/node_zlib.cc index ec0a8e90f1c..5930ffd7a8a 100644 --- a/src/node_zlib.cc +++ b/src/node_zlib.cc @@ -49,7 +49,6 @@ using v8::Context; using v8::Function; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; -using v8::Global; using v8::HandleScope; using v8::Int32; using v8::Integer; @@ -107,8 +106,8 @@ enum node_zlib_mode { BROTLI_ENCODE }; -#define GZIP_HEADER_ID1 0x1f -#define GZIP_HEADER_ID2 0x8b +constexpr uint8_t GZIP_HEADER_ID1 = 0x1f; +constexpr uint8_t GZIP_HEADER_ID2 = 0x8b; struct CompressionError { CompressionError(const char* message, const char* code, int err) @@ -127,14 +126,14 @@ struct CompressionError { inline bool IsError() const { return code != nullptr; } }; -class ZlibContext : public MemoryRetainer { +class ZlibContext final : public MemoryRetainer { public: ZlibContext() = default; // Streaming-related, should be available for all compression libraries: void Close(); void DoThreadPoolWork(); - void SetBuffers(char* in, uint32_t in_len, char* out, uint32_t out_len); + void SetBuffers(const char* in, uint32_t in_len, char* out, uint32_t out_len); void SetFlush(int flush); void GetAfterWriteOffsets(uint32_t* avail_in, uint32_t* avail_out) const; CompressionError GetErrorInfo() const; @@ -183,7 +182,7 @@ class BrotliContext : public MemoryRetainer { public: BrotliContext() = default; - void SetBuffers(char* in, uint32_t in_len, char* out, uint32_t out_len); + void SetBuffers(const char* in, uint32_t in_len, char* out, uint32_t out_len); void SetFlush(int flush); void GetAfterWriteOffsets(uint32_t* avail_in, uint32_t* avail_out) const; inline void SetMode(node_zlib_mode mode) { mode_ = mode; } @@ -193,7 +192,7 @@ class BrotliContext : public MemoryRetainer { protected: node_zlib_mode mode_ = NONE; - uint8_t* next_in_ = nullptr; + const uint8_t* next_in_ = nullptr; uint8_t* next_out_ = nullptr; size_t avail_in_ = 0; size_t avail_out_ = 0; @@ -251,6 +250,12 @@ class BrotliDecoderContext final : public BrotliContext { template <typename CompressionContext> class CompressionStream : public AsyncWrap, public ThreadPoolWork { public: + enum InternalFields { + kCompressionStreamBaseField = AsyncWrap::kInternalFieldCount, + kWriteJSCallback, + kInternalFieldCount + }; + CompressionStream(Environment* env, Local<Object> wrap) : AsyncWrap(env, wrap, AsyncWrap::PROVIDER_ZLIB), ThreadPoolWork(env), @@ -259,7 +264,7 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork { } ~CompressionStream() override { - CHECK_EQ(false, write_in_progress_ && "write in progress"); + CHECK(!write_in_progress_); Close(); CHECK_EQ(zlib_memory_, 0); CHECK_EQ(unreported_allocations_, 0); @@ -295,7 +300,7 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork { CHECK_EQ(args.Length(), 7); uint32_t in_off, in_len, out_off, out_len, flush; - char* in; + const char* in; char* out; CHECK_EQ(false, args[0]->IsUndefined() && "must provide flush value"); @@ -340,7 +345,7 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork { template <bool async> void Write(uint32_t flush, - char* in, uint32_t in_len, + const char* in, uint32_t in_len, char* out, uint32_t out_len) { AllocScope alloc_scope(this); @@ -394,6 +399,7 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork { // v8 land! void AfterThreadPoolWork(int status) override { + DCHECK(init_done_); AllocScope alloc_scope(this); auto on_scope_leave = OnScopeLeave([&]() { Unref(); }); @@ -416,9 +422,8 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork { UpdateWriteResult(); // call the write() cb - Local<Function> cb = PersistentToLocal::Default(env->isolate(), - write_js_callback_); - MakeCallback(cb, 0, nullptr); + Local<Value> cb = object()->GetInternalField(kWriteJSCallback); + MakeCallback(cb.As<Function>(), 0, nullptr); if (pending_close_) Close(); @@ -431,7 +436,7 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork { CHECK_EQ(env->context(), env->isolate()->GetCurrentContext()); HandleScope scope(env->isolate()); - Local<Value> args[3] = { + Local<Value> args[] = { OneByteString(env->isolate(), err.message), Integer::New(env->isolate(), err.err), OneByteString(env->isolate(), err.code) @@ -465,7 +470,7 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork { void InitStream(uint32_t* write_result, Local<Function> write_js_callback) { write_result_ = write_result; - write_js_callback_.Reset(AsyncWrap::env()->isolate(), write_js_callback); + object()->SetInternalField(kWriteJSCallback, write_js_callback); init_done_ = true; } @@ -540,14 +545,13 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork { bool closed_ = false; unsigned int refs_ = 0; uint32_t* write_result_ = nullptr; - Global<Function> write_js_callback_; std::atomic<ssize_t> unreported_allocations_{0}; size_t zlib_memory_ = 0; CompressionContext ctx_; }; -class ZlibStream : public CompressionStream<ZlibContext> { +class ZlibStream final : public CompressionStream<ZlibContext> { public: ZlibStream(Environment* env, Local<Object> wrap, node_zlib_mode mode) : CompressionStream(env, wrap) { @@ -646,7 +650,8 @@ class ZlibStream : public CompressionStream<ZlibContext> { }; template <typename CompressionContext> -class BrotliCompressionStream : public CompressionStream<CompressionContext> { +class BrotliCompressionStream final : + public CompressionStream<CompressionContext> { public: BrotliCompressionStream(Environment* env, Local<Object> wrap, @@ -857,10 +862,10 @@ void ZlibContext::DoThreadPoolWork() { } -void ZlibContext::SetBuffers(char* in, uint32_t in_len, +void ZlibContext::SetBuffers(const char* in, uint32_t in_len, char* out, uint32_t out_len) { strm_.avail_in = in_len; - strm_.next_in = reinterpret_cast<Bytef*>(in); + strm_.next_in = const_cast<Bytef*>(reinterpret_cast<const Bytef*>(in)); strm_.avail_out = out_len; strm_.next_out = reinterpret_cast<Bytef*>(out); } @@ -1093,9 +1098,9 @@ CompressionError ZlibContext::SetParams(int level, int strategy) { } -void BrotliContext::SetBuffers(char* in, uint32_t in_len, +void BrotliContext::SetBuffers(const char* in, uint32_t in_len, char* out, uint32_t out_len) { - next_in_ = reinterpret_cast<uint8_t*>(in); + next_in_ = reinterpret_cast<const uint8_t*>(in); next_out_ = reinterpret_cast<uint8_t*>(out); avail_in_ = in_len; avail_out_ = out_len; |