Fixes for unkeyed decoding #16
Open
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
When the call to decodeString failed, it returned immediately out of the function without assigning the advanced type pointer to outType. Now it breaks out of the switch and updates the pointer at the end of the function.
The call to decodeString doesn't succeed in the case when charValue is NewLabel, as it then calls decodeSharedString, which then tries to check the next character for NullLabel/NewLabel/Default, fails as it is now on the first character of the string. I'm not for sure if decodeString is supposed to be called internally in NSUarchiver. Inside NSUarchiver it is calling decodeSharedString every place else. The difference between decodeString and decodeSharedString is very minor. I opted to just call decodeShareString in readType and it fixed the described issue above.
CuriousTommy
reviewed
Jan 9, 2025
There was a problem hiding this comment.
I'll admit that this PR goes beyond my current knowledge of Darling, so there are some stuff I'm not really able to verify (such as the usage of decodeString vs decodeSharedString)
With that being said, the code and commits look good and the code compiles without issues. The type encoding (#) improvements matches what Apple says in their documentation.
I have one (optional) suggestion and a clarification on your comment.
…wrong retain count Behavior: - decodeObject should return an object that the caller has to explicitly retain - decodeValueOfObjCType:at/decodeArrayOfObjCType:count:at:/decodeValuesOfObjCTypes: implicitly cause the outputted objects to be retained See section titled When to Retain a Decoded Object from: https://preterhuman.net/macstuff/techpubs/macosx/System/Library/Frameworks/Foundation.framework/Versions/C/Resources/English.lproj/Documentation/Reference/ObjC_classic/Classes/NSCoder.html
sirnacnud
force-pushed
the
unkeyed-coding
branch
from
e384ceb to
f633ddd
Compare
CuriousTommy
approved these changes
Jan 12, 2025
There was a problem hiding this comment.
Looks good to me!
I'll hold off merging this in-case if @bugaevc wants to also review this PR.
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.
This PR contains fixes to
NSUnarchiverandNSCoderto properly decode the NIB from Apple's SimpleCocoaApp, which uses unkeyed coder.Please review commit by commit/ I added a explanation for each of the fixes in the commit body.