-
-
Notifications
You must be signed in to change notification settings - Fork 249
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
Make sure inflateCopy() allocates window with the necessary buffer padding #1583
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #1583 +/- ##
========================================
Coverage 83.90% 83.91%
========================================
Files 133 133
Lines 10880 10873 -7
Branches 2805 2807 +2
========================================
- Hits 9129 9124 -5
Misses 1048 1048
+ Partials 703 701 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
67fdd12
to
449178b
Compare
Looks ok to me. Alternatively, we could also just call: err = inflate_ensure_window(copy);
if (err != Z_OK) {
ZFREE_STATE(source, copy);
return err;
}
ZCOPY_WINDOW(copy->window, state->window, (size_t)1U << state->wbits); |
449178b
to
9050f7b
Compare
@nmoinvaz I agree, that would be cleaner. |
9050f7b
to
513103b
Compare
As a bonus, this reduces text size quite a bit. Before:
After:
224 bytes less to potentially pollute the cache 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Good now we just have one code path for inflate window creation.
Hmm, CI is failing. |
513103b
to
0ec1c6d
Compare
@iii-i I suspect this also affects DFLTCC systems, as |
…r chunked operations. Based on Chromium bugfix https://chromium-review.googlesource.com/c/chromium/src/+/4876445
0ec1c6d
to
a02e407
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Hi, sorry, I somehow missed this. For what it's worth, the change looks good to me. |
Make sure inflateCopy() allocates window with the necessary buffer for chunked operations.
Based on Chromium bugfix https://chromium-review.googlesource.com/c/chromium/src/+/4876445