-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix CLEAR_HASH macro to be usable as a single statement.
As it is used in deflateParams().
- Loading branch information
Showing
1 changed file
with
5 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
38e8ce3
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.
Before this fix, on https://github.com/sam-github/zlib/blob/cacf7f1d4e3d44d871b605da3b647f07d718623f/deflate.c#L603,
s->head
would be zeroed on both branches. Does this have no unintentional side-effects? It seems like it would deflate failure, but we haven't noticed that.38e8ce3
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.
The consequence is relatively benign. It results in potentially less compression when switching from compression (level !=0), to no compression (level 0), and back to compression, when only a small amount of data was compressed at level 0 (less than 32K). What is lost is string hash information from some of the data compressed in the first portion with compression, that is not carried over to the third portion. Earlier versions of zlib threw away that information anyway, so it is only a recent enhancement to try to get some mileage out of the compression before no compression.
38e8ce3
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.
Thanks for the clarification @madler
38e8ce3
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.
I would guess the do ... while(0) is not necessary and I find it confusing.
The curly braces { ... } should already do the trick.
38e8ce3
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.
The do while permits following the use of the macro with a semicolon to make it a statement, allowing its use in an if statement. It is a standard construct for macros.