Repository navigation
Fix ATTR-IF-DEFINED/ATTR-IF-NOT-DEFINED handling in ippFileRead - #1779
Open
anabobillo3 wants to merge 1 commit into
Open
anabobillo3 wants to merge 1 commit into
anabobillo3 wants to merge 1 commit into
Conversation
The conditional attribute directives only tested the variable when the attribute callback rejected the name, so without a callback (ipptool) the attribute was always added and with one (ippeveprinter) a rejected name was added when the condition held. Test the variable first, then the callback, and read skipped attributes into the ignored message, which is now allocated when first used. Also reset the current attribute like the ATTR and DEFINE directives do, so a "," after an out-of-band ATTR-IF-* value no longer adds a value to the previous attribute, possibly in a different ipp_t. Add unit tests for both cases to testipp.
michaelrsweet
left a comment
michaelrsweet
left a comment
Member
There was a problem hiding this comment.
Need to spend more time looking at this change before I accept it.
This branch has not been deployed
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This is the CUPS master counterpart of OpenPrinting/libcups#185;
cups/ipp-file.chas the same ATTR-IF-DEFINED/ATTR-IF-NOT-DEFINED code.ippFileRead only tested the ATTR-IF-DEFINED/ATTR-IF-NOT-DEFINED variable when the attribute callback rejected the attribute name:
ignoredmessage, which this branch never allocated, so the line failed with "Unsupported value".The directive also left
attrpointing at the previous attribute. An out-of-band ATTR-IF-* value adds no attribute, so a following,line appended its value to the previous attribute, possibly in a differentipp_t(ignoredvs. the file's attributes). ATTR and DEFINE already resetattr.This change tests the variable first and then the callback, allocates
ignoredwhen it is first needed, and resetsattrlike the other directives.testipp gains
test_file_attr_if, run without and with an attribute callback. It checks which conditional attributes end up in the message, and that a,after an out-of-band ATTR-IF-NOT-DEFINED value is reported as an error instead of adding a value to the previous attribute. On current master all four checks fail; with this change they pass, both in a--enable-debug --enable-maintainerbuild (make testincups/) and under AddressSanitizer (--enable-sanitizer).