-
Notifications
You must be signed in to change notification settings - Fork 60
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
Remove null terminators from other extensions #159
Remove null terminators from other extensions #159
Conversation
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.
Good catch!
I think it would make the most sense to not have null terminators in Strings.
That makes sense to me. We'll need to insert nulls in fn names
, then.
It is already doing that. |
Revisiting this, there's a problem with the proposed approach: the most natural way to check for a name is to compare with e: Maybe that's mitigated by those necessarily being the extensions that you can use the built-in |
The |
The constants are also |
Fair enough. |
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!
The field
other
inExtensionSet
currently handles null terminated strings in a weird way. It adds null terminators infn names
but doesn't remove them infn from_properties
. I got bit by this when checking for extensions by name and not having null terminators in my strings. I think it would make the most sense to not have null terminators inString
s.