D7401: Solid/Mac : fleshing out the skeleton IOKit backend (WIP)

[René J.V. Bertin]

rjvbb marked 10 inline comments as done.
rjvbb added inline comments.

INLINE COMMENTS

> kfunk wrote in iokitdevice.cpp:463
> Returning inside the case statements would make this code clearer as well.

I don't share that opinion and think that multiple exit points do not make the code more efficient either (judging from stepping through the code in a debugger).

But whatever...

> kfunk wrote in iokitvolume.cpp:70
> Dito, inconsistent member naming.
> 
> And given that I've noticed that three times now, this again leads to the conclusion this is very repetitive code amongst . `{IOKitStorageAccess,IOKitVolume,IOKitStorage}::Private`.
> 
> Maybe there should be helper class for accessing a `CFDictionary` instead which all these classes use?
> 
> I'm not trying to piss you off René, but this is slightly sloppy coding which easy for the initial writer to do, but will bite us any time in the future when someone unfamiliar with the code needs to do fixes to this code and potentially fixes only one copy of these snippets. Please think about your architecture more carefully.

Not really certain why I didn't decide to this myself, I'm pretty certain it did cross my mind.

> kfunk wrote in iokitvolume.h:45
> No `virtual` needed if there's already a `Q_DECL_OVERRIDE` or `override`.

I guess that's safe here because IOKitOpticalDisc doesn't override any of these methods?

REPOSITORY
  R245 Solid

REVISION DETAIL
  https://phabricator.kde.org/D7401

To: rjvbb, #frameworks, kfunk
Cc: ngraham, kfunk, anthonyfieroni, cgilles, kde-mac
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180123/df20c16e/attachment.html>