[CoreFoundation] Add Clone method for CFArray. #7403
Merged
Conversation
This file contains hidden or 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 working on https://github.com/xamarin/maccore/issues/940 we noticed that clone the array would be a better approach. The CFArrayCreateCopy add some nice things: * The pointer values from theArray are copied into the new array. * The values are also retained by the new array. * The count of the new array is the same as theArray * The new array uses the same callbacks as theArray. [IMPORTANT] Whith this in, we can have a better fix for https://github.com/xamarin/maccore/issues/940
ghost
reviewed
Nov 11, 2019
Comment on lines
+38
to
+39
| using CFArrayRef = System.IntPtr; | ||
| using CFAllocatorRef = System.IntPtr; |
There was a problem hiding this comment.
Why do we name IntPtrs with using statements and not just use IntPtrdirectly for the parmeter types in Clone()? There's other bindings that use IntPtr directly and mention the objc/c type in an inline comment. What's the standard practice?
Contributor
Author
There was a problem hiding this comment.
It is easier to read. I would have changed all, but would have increase the diff. It does not change the output result of the binding.
Member
There was a problem hiding this comment.
Haha nice question, but yeah just for readability.
Collaborator
|
Build failure Test results1 tests failed, 86 tests passed.Failed tests
|
ghost
approved these changes
Nov 11, 2019
dalexsoto
approved these changes
Nov 11, 2019
rolfbjarne
approved these changes
Nov 12, 2019
spouliot
approved these changes
Nov 12, 2019
Contributor
spouliot
left a comment
spouliot
left a comment
There was a problem hiding this comment.
since it's a manual binding it should have a test
e.g. just check that the cloned array has a different handle but the same items (and item's handle)
stephen-hawley
approved these changes
Nov 12, 2019
Contributor
Author
|
@spouliot CFArray is not public. Do you want me to change the internals visibility to be able to write the test? |
Contributor
|
@mandel-macaque forgot about that, it will be tested w/CFBundle anyway :) |
mandel-macaque
added a commit
to mandel-macaque/xamarin-macios
that referenced
this pull request
Nov 13, 2019
When working on https://github.com/xamarin/maccore/issues/940 we noticed that clone the array would be a better approach. The CFArrayCreateCopy add some nice things: * The pointer values from theArray are copied into the new array. * The values are also retained by the new array. * The count of the new array is the same as theArray * The new array uses the same callbacks as theArray. [IMPORTANT] Whith this in, we can have a better fix for https://github.com/xamarin/maccore/issues/940
mandel-macaque
added a commit
to mandel-macaque/xamarin-macios
that referenced
this pull request
Nov 13, 2019
When working on https://github.com/xamarin/maccore/issues/940 we noticed that clone the array would be a better approach. The CFArrayCreateCopy add some nice things: * The pointer values from theArray are copied into the new array. * The values are also retained by the new array. * The count of the new array is the same as theArray * The new array uses the same callbacks as theArray. [IMPORTANT] Whith this in, we can have a better fix for https://github.com/xamarin/maccore/issues/940
mandel-macaque
added a commit
to mandel-macaque/xamarin-macios
that referenced
this pull request
Nov 13, 2019
When working on https://github.com/xamarin/maccore/issues/940 we noticed that clone the array would be a better approach. The CFArrayCreateCopy add some nice things: * The pointer values from theArray are copied into the new array. * The values are also retained by the new array. * The count of the new array is the same as theArray * The new array uses the same callbacks as theArray. [IMPORTANT] Whith this in, we can have a better fix for https://github.com/xamarin/maccore/issues/940
Contributor
Author
|
@monojenkins backport xcode11.3 |
mandel-macaque
added a commit
to monojenkins/xamarin-macios
that referenced
this pull request
Nov 14, 2019
When working on https://github.com/xamarin/maccore/issues/940 we noticed that clone the array would be a better approach. The CFArrayCreateCopy add some nice things: * The pointer values from theArray are copied into the new array. * The values are also retained by the new array. * The count of the new array is the same as theArray * The new array uses the same callbacks as theArray. [IMPORTANT] Whith this in, we can have a better fix for https://github.com/xamarin/maccore/issues/940
mandel-macaque
pushed a commit
that referenced
this pull request
Nov 14, 2019
* [CoreFoundation] CFBundle.GetAll better thread safe. The initial solution fixed the rance condition in which the index changed, but that did not guarantee that we would get the correct bundles. We now clone the CFArray (which also clones the callbacks set to the array) and iterate over it to make sure Apple does not do evil tings while we are iteraing. Better Fixes: https://github.com/xamarin/maccore/issues/940 * [CoreFoundation] Add Clone method for CFArray. (#7403) When working on https://github.com/xamarin/maccore/issues/940 we noticed that clone the array would be a better approach. The CFArrayCreateCopy add some nice things: * The pointer values from theArray are copied into the new array. * The values are also retained by the new array. * The count of the new array is the same as theArray * The new array uses the same callbacks as theArray. [IMPORTANT] Whith this in, we can have a better fix for https://github.com/xamarin/maccore/issues/940
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
When working on https://github.com/xamarin/maccore/issues/940 we noticed
that clone the array would be a better approach.
The CFArrayCreateCopy add some nice things:
With this in, we can have a better fix for https://github.com/xamarin/maccore/issues/940