Add duplicate chunk check #334
Merged
Conversation
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
The duplicate chunk error variant was unused and resulted in a warning when compiling. Only some chunk types are allowed to appear multiple times according to the [specification](https://www.w3.org/TR/PNG/#5ChunkOrdering). I have added checks for that. This also fixes the unused variant warning when compiling. The `parse_chunk` method is already checking if the info is `None` so it is safe to unwrap the info in the subsequent `parse_*` methods. Ideally the parsing should be refactored at some point so that the parsing methods only receive what they need and not a reference to the whole mutable self. For example, the header, which creates the info, must appear right after the signature. That means that the info could be passed to the subsequent `parse_*` methods and the they would not need to unwrap it. That would, however, be a large complicated change.
MichaelMcDonnell
force-pushed
the
add-duplicate-chunk-check
branch
from
6dc31f2
to
2448a07
Compare
There was a problem hiding this comment.
The rework of receiving isn't even that appealing to me. There are enough chunks that either have dependencies or conflict on each other or can benefit from having more context information in any case (e.g. gAMA–cHRM–iCCP), various animation related chunks.
I do see value in somehow statically clarifying that the info is available, though.
HeroicKatora
approved these changes
Feb 23, 2022
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The duplicate chunk error variant was unused and resulted in a warning when compiling. Only some chunk types are allowed to appear multiple times according to the specification. I have added checks for that. This also fixes the unused variant warning when compiling.
The
parse_chunkmethod is already checking if the info isNoneso it is safe to unwrap the info in the subsequentparse_*methods.Ideally the parsing should be refactored at some point so that the parsing methods only receive what they need and not a reference to the whole mutable self. For example, the header, which creates the info, must appear right after the signature. That means that the info could be passed to the subsequent
parse_*methods and the they would not need to unwrap it. That would, however, be a large complicated change.