Skip to content

/ rust-layers

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

Fix crash from failing XOpenDisplay #38

Merged
merged 2 commits into from Nov 13, 2013
Merged

Fix crash from failing XOpenDisplay #38

merged 2 commits into from Nov 13, 2013
+7 −1

Conversation

@kmcallister
Copy link
Contributor

kmcallister commented Nov 13, 2013

No description provided.

kmcallister added 2 commits Nov 12, 2013
@kmcallister
Fix unsafe use of CString::with_ref
7a70694 
Sometimes the C string would get freed before XOpenDisplay was done with it,
producing a garbage display string and a null Display*.

Also switch to with_c_str because it will stack-allocate for the common case of
small strings (doesn't really matter here, but why not).
@kmcallister
Check for XOpenDisplay() failure
456f154
@pcwalton

This comment has been minimized.

Sign in to view
Copy link

pcwalton commented on 456f154 Nov 13, 2013

r+

Oops, I think that's my fault.

This comment has been minimized.

Sign in to view
Copy link
Owner Author

kmcallister replied Nov 13, 2013

Yeah, in this case. However it's an error a lot of us have made at some point or another.

I wonder if there's some lightweight (automatic?) way to wrap C functions so that they take borrowed pointers to C strings instead of raw pointers, and insist that a Rust-allocated C string at least outlive the function call. For many APIs that won't be sufficient but it is almost always necessary.

This comment has been minimized.

Sign in to view
Copy link
Owner Author

kmcallister replied Nov 13, 2013

Apparently we can just use & instead of * in the extern signature.
jdm added a commit that referenced this pull request Nov 13, 2013
@jdm
Merge pull request #38 from kmcallister/display
c86eaa8 
Fix crash from failing XOpenDisplay
@jdm jdm merged commit c86eaa8 into servo:master Nov 13, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@kmcallister @pcwalton @jdm
You can’t perform that action at this time.