Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit d06e92d

Browse filesBrowse files
addaleaxbengl
addaleax
authored and
bengl
committed
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: #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>
1 parent 4829a10 commit d06e92d
Copy full SHA for d06e92d

File tree

Expand file treeCollapse file tree

1 file changed

+27
-22
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

1 file changed

+27
-22
lines changed
Open diff view settings
Collapse file

‎src/node_zlib.cc‎

Copy file name to clipboardExpand all lines: src/node_zlib.cc
+27-22Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ using v8::Context;
4949
using v8::Function;
5050
using v8::FunctionCallbackInfo;
5151
using v8::FunctionTemplate;
52-
using v8::Global;
5352
using v8::HandleScope;
5453
using v8::Int32;
5554
using v8::Integer;
@@ -107,8 +106,8 @@ enum node_zlib_mode {
107106
BROTLI_ENCODE
108107
};
109108

110-
#define GZIP_HEADER_ID1 0x1f
111-
#define GZIP_HEADER_ID2 0x8b
109+
constexpr uint8_t GZIP_HEADER_ID1 = 0x1f;
110+
constexpr uint8_t GZIP_HEADER_ID2 = 0x8b;
112111

113112
struct CompressionError {
114113
CompressionError(const char* message, const char* code, int err)
@@ -127,14 +126,14 @@ struct CompressionError {
127126
inline bool IsError() const { return code != nullptr; }
128127
};
129128

130-
class ZlibContext : public MemoryRetainer {
129+
class ZlibContext final : public MemoryRetainer {
131130
public:
132131
ZlibContext() = default;
133132

134133
// Streaming-related, should be available for all compression libraries:
135134
void Close();
136135
void DoThreadPoolWork();
137-
void SetBuffers(char* in, uint32_t in_len, char* out, uint32_t out_len);
136+
void SetBuffers(const char* in, uint32_t in_len, char* out, uint32_t out_len);
138137
void SetFlush(int flush);
139138
void GetAfterWriteOffsets(uint32_t* avail_in, uint32_t* avail_out) const;
140139
CompressionError GetErrorInfo() const;
@@ -183,7 +182,7 @@ class BrotliContext : public MemoryRetainer {
183182
public:
184183
BrotliContext() = default;
185184

186-
void SetBuffers(char* in, uint32_t in_len, char* out, uint32_t out_len);
185+
void SetBuffers(const char* in, uint32_t in_len, char* out, uint32_t out_len);
187186
void SetFlush(int flush);
188187
void GetAfterWriteOffsets(uint32_t* avail_in, uint32_t* avail_out) const;
189188
inline void SetMode(node_zlib_mode mode) { mode_ = mode; }
@@ -193,7 +192,7 @@ class BrotliContext : public MemoryRetainer {
193192

194193
protected:
195194
node_zlib_mode mode_ = NONE;
196-
uint8_t* next_in_ = nullptr;
195+
const uint8_t* next_in_ = nullptr;
197196
uint8_t* next_out_ = nullptr;
198197
size_t avail_in_ = 0;
199198
size_t avail_out_ = 0;
@@ -251,6 +250,12 @@ class BrotliDecoderContext final : public BrotliContext {
251250
template <typename CompressionContext>
252251
class CompressionStream : public AsyncWrap, public ThreadPoolWork {
253252
public:
253+
enum InternalFields {
254+
kCompressionStreamBaseField = AsyncWrap::kInternalFieldCount,
255+
kWriteJSCallback,
256+
kInternalFieldCount
257+
};
258+
254259
CompressionStream(Environment* env, Local<Object> wrap)
255260
: AsyncWrap(env, wrap, AsyncWrap::PROVIDER_ZLIB),
256261
ThreadPoolWork(env),
@@ -259,7 +264,7 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork {
259264
}
260265

261266
~CompressionStream() override {
262-
CHECK_EQ(false, write_in_progress_ && "write in progress");
267+
CHECK(!write_in_progress_);
263268
Close();
264269
CHECK_EQ(zlib_memory_, 0);
265270
CHECK_EQ(unreported_allocations_, 0);
@@ -295,7 +300,7 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork {
295300
CHECK_EQ(args.Length(), 7);
296301

297302
uint32_t in_off, in_len, out_off, out_len, flush;
298-
char* in;
303+
const char* in;
299304
char* out;
300305

301306
CHECK_EQ(false, args[0]->IsUndefined() && "must provide flush value");
@@ -340,7 +345,7 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork {
340345

341346
template <bool async>
342347
void Write(uint32_t flush,
343-
char* in, uint32_t in_len,
348+
const char* in, uint32_t in_len,
344349
char* out, uint32_t out_len) {
345350
AllocScope alloc_scope(this);
346351

@@ -394,6 +399,7 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork {
394399

395400
// v8 land!
396401
void AfterThreadPoolWork(int status) override {
402+
DCHECK(init_done_);
397403
AllocScope alloc_scope(this);
398404
auto on_scope_leave = OnScopeLeave([&]() { Unref(); });
399405

@@ -416,9 +422,8 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork {
416422
UpdateWriteResult();
417423

418424
// call the write() cb
419-
Local<Function> cb = PersistentToLocal::Default(env->isolate(),
420-
write_js_callback_);
421-
MakeCallback(cb, 0, nullptr);
425+
Local<Value> cb = object()->GetInternalField(kWriteJSCallback);
426+
MakeCallback(cb.As<Function>(), 0, nullptr);
422427

423428
if (pending_close_)
424429
Close();
@@ -431,7 +436,7 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork {
431436
CHECK_EQ(env->context(), env->isolate()->GetCurrentContext());
432437

433438
HandleScope scope(env->isolate());
434-
Local<Value> args[3] = {
439+
Local<Value> args[] = {
435440
OneByteString(env->isolate(), err.message),
436441
Integer::New(env->isolate(), err.err),
437442
OneByteString(env->isolate(), err.code)
@@ -465,7 +470,7 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork {
465470

466471
void InitStream(uint32_t* write_result, Local<Function> write_js_callback) {
467472
write_result_ = write_result;
468-
write_js_callback_.Reset(AsyncWrap::env()->isolate(), write_js_callback);
473+
object()->SetInternalField(kWriteJSCallback, write_js_callback);
469474
init_done_ = true;
470475
}
471476

@@ -540,14 +545,13 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork {
540545
bool closed_ = false;
541546
unsigned int refs_ = 0;
542547
uint32_t* write_result_ = nullptr;
543-
Global<Function> write_js_callback_;
544548
std::atomic<ssize_t> unreported_allocations_{0};
545549
size_t zlib_memory_ = 0;
546550

547551
CompressionContext ctx_;
548552
};
549553

550-
class ZlibStream : public CompressionStream<ZlibContext> {
554+
class ZlibStream final : public CompressionStream<ZlibContext> {
551555
public:
552556
ZlibStream(Environment* env, Local<Object> wrap, node_zlib_mode mode)
553557
: CompressionStream(env, wrap) {
@@ -646,7 +650,8 @@ class ZlibStream : public CompressionStream<ZlibContext> {
646650
};
647651

648652
template <typename CompressionContext>
649-
class BrotliCompressionStream : public CompressionStream<CompressionContext> {
653+
class BrotliCompressionStream final :
654+
public CompressionStream<CompressionContext> {
650655
public:
651656
BrotliCompressionStream(Environment* env,
652657
Local<Object> wrap,
@@ -857,10 +862,10 @@ void ZlibContext::DoThreadPoolWork() {
857862
}
858863

859864

860-
void ZlibContext::SetBuffers(char* in, uint32_t in_len,
865+
void ZlibContext::SetBuffers(const char* in, uint32_t in_len,
861866
char* out, uint32_t out_len) {
862867
strm_.avail_in = in_len;
863-
strm_.next_in = reinterpret_cast<Bytef*>(in);
868+
strm_.next_in = const_cast<Bytef*>(reinterpret_cast<const Bytef*>(in));
864869
strm_.avail_out = out_len;
865870
strm_.next_out = reinterpret_cast<Bytef*>(out);
866871
}
@@ -1093,9 +1098,9 @@ CompressionError ZlibContext::SetParams(int level, int strategy) {
10931098
}
10941099

10951100

1096-
void BrotliContext::SetBuffers(char* in, uint32_t in_len,
1101+
void BrotliContext::SetBuffers(const char* in, uint32_t in_len,
10971102
char* out, uint32_t out_len) {
1098-
next_in_ = reinterpret_cast<uint8_t*>(in);
1103+
next_in_ = reinterpret_cast<const uint8_t*>(in);
10991104
next_out_ = reinterpret_cast<uint8_t*>(out);
11001105
avail_in_ = in_len;
11011106
avail_out_ = out_len;

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.