Add mappings for CoreFoundation, IOKit, DiskArbitration

[dbwiddis]

Part one of three framework mappings. Will submit DiskArbitration and IOKit mappings later, but as they depend on this one, I want to submit them separately and get any needed reviews on this PR completed.

[dbwiddis]

Part 2 of 3 mappings: IOKit. Kept as a separate commit to ease review, but may be merged.

[dbwiddis]

Part 3 of 3 mappings: DiskArbitration. Ready for review/merge.

[matthiasblaesing]

I took a quick look, but I'm totally unfamiliar with Mac OS X and I don't yet foresee a future where that will change. I just did some googling and throw in some pain points from the past.

[dbwiddis]

I've addressed most of the comments. Questions still outstanding:

• I've updated the docs for the Util class, but based on the questions, I'm thinking perhaps it's best to just remove that (and keep it in my own app). The only complex method is the String conversion and I can roll that into the CFStringRef class, perhaps as a toString() method.
• I have left the pointer-sized arguments as long for now (I couldn't find where any were int?) as OSX has been 64-bit since 10.7 (Lion, July 2011, the same month as Java 7). Total worldwide usage share for macOS of Pre-Lion versions is 0.5%, of which 0.45% is 10.6 (32- or 64-bit) and 0.05% is 32-bit 10.5 or earlier.

[matthiasblaesing]

I left some more comments inline. Sorry for the trickle of comments.

[dbwiddis]

No problem at all, this is an amazingly useful and educational conversation. Stuck on a Windows machine during the day so expect another commit while you sleep.

• I think we're okay with leaving the CFIndex types (pointer-sized) as long.
• I still think PointerType is the correct mapping for the dictionary keys.
• I'll move the Util class methods and fix the other unresolved comments.

[dbwiddis]

OK, I think I've addressed all the comments. Bring on the next phase of the review!

(There's a reason I wrote this in 2016 and have delayed submitting it....)

[dbwiddis]

One note. I re-read the comments and I ended up putting MachPort back as a subclass of IOObject so that I could leverage the IOObject#release() method. My thought was that the docs for releasing a the master port (not required but good practice) say to use mach_port_deallocate(mach_task_self(), masterPort); and the source code of IOObjectRelease is:

kern_return_t IOObjectRelease(io_object_t object) {
    return(mach_port_deallocate(mach_task_self(), object));
}

If I put the ports in the "proper" order (IOObject extends MachPort) then I lose the ability to easily call that deallocate method and to do it correctly I'd have to add mach_port_deallocate into SystemB. I could play with the pointer to cast it to IOObject to release it, but that seems a worse hack than just reversing the inheritance which I've done. Or I could allow IOObjectRelease to take a PointerType (or at least MachPort) argument. Not sure what the best solution is here.

[matthiasblaesing]

Sorry I still have some comments left inline. I hope you find them helpful.

[dbwiddis]

OK. I think this is finally ready. :)

[matthiasblaesing]

Looks mostly complete to me. I added some minor comments inline.

[matthiasblaesing]

A few final thoughts on convenient return types. I tried to image what I would like from convenience methods. Feel free to disagree with me I just wanted to get the suggestions into the open.

[dbwiddis]

Good suggestions on the return types for convenience methods.

There's only one other outstanding question I've got. I tried to put in some typecasting protection using the TypeID, and used the pointer-constructor as the most likely point of failure, as this idiom is common:

Pointer p = dictionary.getValue(key);
CFNumberRef numberRef = new CFNumberRef(p);

I actually found a CFNumber miscast as a CFBoolean in my project after adding this. However, I just noticed reviewing the test cases that I can bypass this check with a setPointer() call, e.g.,

CFMutableDictionaryRef dict = new CFMutableDictionaryRef();
Pointer p = dictionary.getValue(key);
dict.setPointer(p);

I'm trying to think of the best way to handle this or whether to allow this bypass. Not sure I want to override setPointer() everywhere but perhaps I should, and even redirect the pointer constructor to call it.

[matthiasblaesing]

Thank you for the updates - looks sane to me. For the setPointer question: I have no strong feelings either way. I'm not aware of other places, that have such tight checks, if you want to add the checks I'm ok with them, but would also merge without.

[dbwiddis]

I did something with WMI type checking in the Util when getting the values, but in that case a plain Object was stored so that was the moment of the actual typecasting.

I think I’m ok with it as is.

[matthiasblaesing]

Thank you very much for your work and taking the time to work through this.