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

Add CFPropertyList serialization #90

Merged
merged 3 commits into from Jan 5, 2017
Merged

Add CFPropertyList serialization #90

merged 3 commits into from Jan 5, 2017

Conversation

@ghost
Copy link

ghost commented Jan 2, 2017

Tests included. Reopened from #88


This change is Reviewable

Christian Howe

pub fn create_with_data(data: CFData,
options: CFPropertyListMutabilityOptions)
-> Result<(CFType, CFPropertyListFormat), CFError> {

This comment has been minimized.

@ghost

ghost Jan 3, 2017

Author

I'm not sure that using CFType here is appropriate, especially without an easy way to downcast it to a concrete type. Other parts of the API use *const c_void for these generic types instead.

This comment has been minimized.

@ghost

ghost Jan 4, 2017

Author

I've now converted this to use *const c_void, keeping it consistent with other parts of the API.

Copy link
Member

jdm left a comment

Looks good!

data.as_concrete_TypeRef(),
options,
&mut format as *mut CFPropertyListFormat,
&mut error as *mut CFErrorRef);

This comment has been minimized.

@jdm

jdm Jan 4, 2017

Member

These casts shouldn't be necessary.

property_list_ref,
format,
0,
&mut error as *mut CFErrorRef);

This comment has been minimized.

@jdm

jdm Jan 4, 2017

Member

This cast shouldn't be necessary.

@ghost
Copy link
Author

ghost commented Jan 5, 2017

Unnecessary casts removed. Thanks for the reviews!

@jdm
Copy link
Member

jdm commented Jan 5, 2017

@bors-servo: r+
Thanks!

@bors-servo
Copy link
Contributor

bors-servo commented Jan 5, 2017

📌 Commit 37e11bd has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Jan 5, 2017

Test exempted - status

@bors-servo bors-servo merged commit 37e11bd into servo:master Jan 5, 2017
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test exempted
Details
bors-servo added a commit that referenced this pull request Jan 5, 2017
Add CFPropertyList serialization

Tests included. Reopened from #88

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/core-foundation-rs/90)
<!-- Reviewable:end -->
@ghost ghost deleted the feature-cfproperty-serialization branch Jan 5, 2017
jdm pushed a commit that referenced this pull request Feb 1, 2018
Remove warnings for zero-sized structs

This change removes a bunch of warnings from the crate.

The only remaining warnings are due to the bitflags macro usage.

This will help fix servo/core-text-rs#62 when it upgrades to `core-graphics = 0.9`.

@jdm

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/core-graphics-rs/90)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.