Skip to content
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 ZSTD_getDictID_fromDDict() Read DictID from DDict #3290

Merged
merged 1 commit into from
Oct 18, 2022

Conversation

felixhandte
Copy link
Contributor

@felixhandte felixhandte commented Oct 15, 2022

Currently this function actually reads the dict ID from the dictionary's header, via ZSTD_getDictID_fromDict(). But during decompression the decompressor actually compares the dict ID in the frame header with the dict ID in the DDict. Now of course the dict ID in the dictionary contents and the dict ID in the DDict struct should be the same. But in cases of memory corruption, where they can drift out of sync, it's misleading for this function to read it again from the dict buffer rather then return the dict ID that will actually be used.

Also doing it this way avoids rechecking the magic and so on and so it is a tiny bit more efficient.

Currently this function actually reads the dict ID from the dictionary's
header, via `ZSTD_getDictID_fromDict()`. But during decompression the decomp-
ressor actually compares the dict ID in the frame header with the dict ID in
the DDict. Now of course the dict ID in the dictionary contents and the dict
ID in the DDict struct *should* be the same. But in cases of memory corrupt-
ion, where they can drift out of sync, it's misleading for this function to
read it again from the dict buffer rather then return the dict ID that will
actually be used.

Also doing it this way avoids rechecking the magic and so on and so it is a
tiny bit more efficient.
@felixhandte felixhandte merged commit 99d239d into facebook:dev Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants