-
-
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
Extend GZIP conditional #1294
Extend GZIP conditional #1294
Conversation
If gzip support has been disabled during compilation then also consider gzip relevant states as invalid in deflateStateCheck. Also the gzip state definitions can be removed. This change leads to failure in test/example, and I am not sure what the GZIP conditional is trying to achieve. All gzip related functions are still defined in zlib.h Alternative approach is to remove the GZIP define.
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.
It may be possible to also wrap these struct fields in #ifdef GZIP
:
uint32_t gzindex; /* where in extra, name, or comment */
PREFIX(gz_headerp) gzhead; /* gzip header information to write */
The problem with doing that is that it affects cacheline alignment and grouping for the whole rest of the struct, potentially making adjacent hot variables no longer reside in the same cacheline etc, so probably not worth it unless we do a much bigger job of it and ensure these get placed at the end of the struct where they will do no harm (or replace them with dummy values of the same size, not that that would really make any difference other than potentially for readability). |
Codecov Report
@@ Coverage Diff @@
## develop #1294 +/- ##
========================================
Coverage 86.59% 86.59%
========================================
Files 124 124
Lines 10533 10533
Branches 2622 2622
========================================
Hits 9121 9121
Misses 1056 1056
Partials 356 356
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Neither the Or is it desirable to extend these build environments? |
This define is only really useful when building zlib statically into an application that does not need to support gzip at all. deflate.h has this comment:
I am not sure how much code space is saved by disabling gzip and crc support, so that would have to be investigated before any real discussion on this could take place. I feel pretty sure that it does not affect performance measurably (except by random cache alignment chance). I also think gzip support can safely be enabled even when an application expects it to be absent. |
Especially the new CRC tables from 1.2.12 are quite large, so the savings can be few kilobytes. |
@mtl1979 the code size after the new tables were incorporated decreased. See #1268 (comment) |
@mtl1979 the tables look like they might be larger in the header file but actually only a portion of those generated tables are used. |
@nmoinvaz Code size might have decreased but const variables aren't considered code. Old CRC code had no support for 64-bit processors, so it had simpler tables. When we separated little-endian and big-endian tables, we practically reduced the size of the tables by 50%. |
Changes since 2.0.6: - Fix CVE-2022-37434 #1328 - Fix chunkmemset #1196 - Fix deflateBound too small #1236 - Fix Z_SOLO #1263 - Fix ACLE variant of crc32 #1274 - Fix inflateBack #1311 - Fix deflate_quick windowsize #1431 - Fix DFLTCC bugs related to adler32 #1349 and #1390 - Fix warnings #1194 #1312 #1362 - MacOS build fix #1198 - Add invalid windowBits handling #1293 - Support for Force TZCNT #1186 - Support for aligned_alloc() #1360 - Minideflate improvements #1175 #1238 - Dont use unaligned access for memcpy #1309 - Build system #1209 #1233 #1267 #1273 #1278 #1292 #1316 #1318 #1365 - Test improvements #1208 #1227 #1241 #1353 - Cleanup #1266 - Documentation #1205 #1359 - Misc improvements #1294 #1297 #1306 #1344 #1348 - Backported zlib fixes - Backported CI workflows from Develop branch
Changes since 2.0.6: - Fix CVE-2022-37434 #1328 - Fix chunkmemset #1196 - Fix deflateBound too small #1236 - Fix Z_SOLO #1263 - Fix ACLE variant of crc32 #1274 - Fix inflateBack #1311 - Fix deflate_quick windowsize #1431 - Fix DFLTCC bugs related to adler32 #1349 and #1390 - Fix warnings #1194 #1312 #1362 - MacOS build fix #1198 - Add invalid windowBits handling #1293 - Support for Force TZCNT #1186 - Support for aligned_alloc() #1360 - Minideflate improvements #1175 #1238 - Dont use unaligned access for memcpy #1309 - Build system #1209 #1233 #1267 #1273 #1278 #1292 #1316 #1318 #1365 - Test improvements #1208 #1227 #1241 #1353 - Cleanup #1266 - Documentation #1205 #1359 - Misc improvements #1294 #1297 #1306 #1344 #1348 - Backported zlib fixes - Backported CI workflows from Develop branch
If gzip support has been disabled during compilation then also
consider gzip relevant states as invalid in deflateStateCheck.
Also the gzip state definitions can be removed.
This change leads to failure in test/example, and I am not sure
what the GZIP conditional is trying to achieve. All gzip related
functions are still defined in zlib.h
Alternative approach is to remove the GZIP define.