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

urldata: make magic be the first struct field #6484

Closed
wants to merge 1 commit into from

Conversation

bagder
Copy link
Member

@bagder bagder commented Jan 19, 2021

By making the magic identifier the same size and at the same place
within the structs (easy, multi, share), libcurl will be able to more
reliably detect and safely error out if an application passes in the
wrong handle to APIs. Easier to detect and less likely to cause crashes
if done.

Such mixups can't be detected at compile-time due to them being
typedefed void pointers - unless CURL_STRICTER is defined.

By making the `magic` identifier the same size and at the same place
within the structs (easy, multi, share), libcurl will be able to more
reliably detect and safely error out if an application passes in the
wrong handle to APIs. Easier to detect and less likely to cause crashes
if done.

Such mixups can't be detected at compile-time due to them being
typedefed void pointers - unless `CURL_STRICTER` is defined.
@jay
Copy link
Member

jay commented Jan 19, 2021

Not that I'm against this but note I have an example ListEasyHandles.c that uses those internal structs. I guarded it with a version check of course.

@bagder
Copy link
Member Author

bagder commented Jan 19, 2021

Right, and we do have opaque structs for this very reason: to allow us to modify them without breaking any user applications. At least I don't think we will change the beginning of the structs very often...

@bagder bagder closed this in 942cf12 Jan 20, 2021
@bagder bagder deleted the bagder/handle-diff branch January 20, 2021 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants