Differential D7401 Diff 18366 src/solid/devices/backends/iokit/iokitstorage.cpp

Changeset View

Standalone View

View Options

src/solid/devices/backends/iokit/iokitstorage.cpp

  • This file was added.
1​/*
2​ Copyright 2017 René J.V. Bertin <rjvbertin@gmail.com>
3​
4​ This library is free software; you can redistribute it and/or
5​ modify it under the terms of the GNU Lesser General Public
6​ License as published by the Free Software Foundation; either
7​ version 2.1 of the License, or (at your option) version 3, or any
8​ later version accepted by the membership of KDE e.V. (or its
9​ successor approved by the membership of KDE e.V.), which shall
10​ act as a proxy defined in Section 6 of version 3 of the license.
11​
12​ This library is distributed in the hope that it will be useful,
13​ but WITHOUT ANY WARRANTY; without even the implied warranty of
14​ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
15​ Lesser General Public License for more details.
16​
17​ You should have received a copy of the GNU Lesser General Public
18​ License along with this library. If not, see <http://www.gnu.org/licenses/>.
19​*/
20​
21​#include "iokitstorage.h"
22​
23​#include <QDebug>
24​
25​#include <CoreFoundation/CoreFoundation.h>
26​#include <DiskArbitration/DiskArbitration.h>
27​
28​using namespace Solid::Backends::IOKit;
29​
30​class IOKitStorage::Private
31​{
32​public:
33​ Private(const IOKitDevice *device)
34​ : m_device(device)
35​ {
36​ daRef = 0;
kfunkUnsubmitted
Done

Here and one line below: Could be initialized at decl again.

kfunk: Here and one line below: Could be initialized at decl again.
kfunkUnsubmitted
Done

nullptr

kfunk: `nullptr`
rjvbbAuthorUnsubmitted
Not Done

Did you check that these are indeed pointers? ;)

rjvbb: Did you check that these are indeed pointers? ;)
kfunkUnsubmitted
Not Done

Yes. And they are, no?

kfunk: Yes. And they are, no?
37​ daDict = 0;
38​ daSession = DASessionCreate(kCFAllocatorDefault);
39​ if (daSession) {
40​ const QString devName = m_device->property(QStringLiteral("BSD Name")).toString();
41​ daRef = DADiskCreateFromBSDName(kCFAllocatorDefault, daSession, devName.toStdString().c_str());
42​ if (daRef) {
43​ daDict = DADiskCopyDescription(daRef);
44​ }
45​ }
46​ }
47​ virtual ~Private()
48​ {
49​ if (daDict) {
50​ CFRelease(daDict);
51​ daDict = 0;
kfunkUnsubmitted
Done

nullptr, etc. pp.

kfunk: `nullptr`, etc. pp.
52​ }
53​ if (daRef) {
54​ CFRelease(daRef);
55​ daRef = 0;
56​ }
57​ if (daSession) {
58​ CFRelease(daSession);
59​ daSession = 0;
60​ }
61​ }
62​
63​ const IOKitDevice *m_device;
64​ DASessionRef daSession;
65​ DADiskRef daRef;
66​ CFDictionaryRef daDict;
67​};
68​
69​IOKitStorage::IOKitStorage(IOKitDevice *device)
70​ : Block(device)
71​ , d(new Private(device))
72​{
73​}
74​
kfunkUnsubmitted
Done

Inconsistent member naming. Some with m_ prefix, some without. Choose one style. Private classes' members usually live without the m_ prefix, but we don't mind them being around (in KDE land)

kfunk: Inconsistent member naming. Some with `m_` prefix, some without. Choose one style. Private…
75​IOKitStorage::IOKitStorage(const IOKitDevice *device)
kfunkUnsubmitted
Not Done

Please just remove the constructors taking a const IOKitDevice *device and adapt uses (just use IOKitDevice *device everywhere). It's unusual for pointer types to be const in such contexts. It just adds noise.

kfunk: Please just remove the constructors taking a `const IOKitDevice *device` and adapt uses (just…
rjvbbAuthorUnsubmitted
Not Done

I don't get it, sorry, can you explain in more words how you'd want to see this changed? If I remove this extra ctor, I can no longer call IOKitStorage(this).vendor() in IOKitDevice::vendor() without extra code that's also going to add noise.

I get a warning when I remove the const attribute from IOKitDevice::vendor(), which suggests that I'd no longer be reimplementing a virtual method but adding a method instead.

All these "extra" ctors hand off the pointer to a "const this" to DeviceInterface which finally makes a deep copy. I find that cleaner than creating a deep copy of this everywhere needed and ensuring it gets deallocated (even via QPointers).
Unusual doesn't mean wrong (we're on Mac here, after all ^^)

rjvbb: I don't get it, sorry, can you explain in more words how you'd want to see this changed? If I…
kfunkUnsubmitted
Done

Okay, well, leave it like that.

I'm running out of time to properly explain how I'd envision this code to be like in a perfect world.

kfunk: Okay, well, leave it like that. I'm running out of time to properly explain how I'd envision…
76​ : Block(device)
77​ , d(new Private(device))
78​{
79​}
80​
81​IOKitStorage::~IOKitStorage()
82​{
83​ delete d;
84​}
85​
86​Solid::StorageDrive::Bus IOKitStorage::bus() const
87​{
88​// if (d->daDict) {
89​// qWarning() << Q_FUNC_INFO;
90​// CFShow(d->daDict);
91​// }
92​
93​ const QString udi = m_device->udi();
94​ // TODO: figure out how to return something useful here.
95​ if (udi.contains(QStringLiteral("/SATA@"))) {
96​ return Solid::StorageDrive::Sata;
97​ }
98​ if (udi.contains(QStringLiteral("/SDXC@"))) {
99​ // TODO: return something finer grained; the built-in card reader
100​ // is NOT connected via USB on Macs, for instance (but there's no PCI option)
101​ return Solid::StorageDrive::Usb;
102​ }
103​ if (udi.contains(QStringLiteral("/IOUSBInterface@"))) {
104​ return Solid::StorageDrive::Usb;
105​ }
106​ if (d->daDict) {
107​ const CFStringRef devProtocol = (const CFStringRef) CFDictionaryGetValue(d->daDict,
108​ kDADiskDescriptionDeviceProtocolKey);
109​ if (QString::fromCFString(devProtocol) == QStringLiteral("USB")) {
110​ return Solid::StorageDrive::Usb;
111​ }
112​ }
113​ return Solid::StorageDrive::Platform;
114​}
115​
116​Solid::StorageDrive::DriveType IOKitStorage::driveType() const
117​{
118​ const QString udi = m_device->udi();
119​ const QString type = m_device->property(QLatin1String("className")).toString();
120​
121​ if (type == QStringLiteral("IOCDMedia")
122​ || type == QStringLiteral("IOBDMedia")
123​ || type == QStringLiteral("IODVDMedia")) {
124​ return Solid::StorageDrive::CdromDrive;
125​ }
126​ if (udi.contains(QStringLiteral("/SDXC@"))) {
127​ return Solid::StorageDrive::SdMmc;
128​ }
129​ if (d->daDict) {
130​ const CFStringRef devModel = (const CFStringRef) CFDictionaryGetValue(d->daDict,
131​ kDADiskDescriptionDeviceModelKey);
132​ if (QString::fromCFString(devModel) == QStringLiteral("Compact Flash")) {
133​ return Solid::StorageDrive::CompactFlash;
134​ }
135​ }
136​ return Solid::StorageDrive::HardDisk;
137​}
138​
139​bool IOKitStorage::isRemovable() const
140​{
141​ bool isExternal = false;
142​ if (d->daDict) {
143​ const CFBooleanRef isInternal = (const CFBooleanRef) CFDictionaryGetValue(d->daDict,
144​ kDADiskDescriptionDeviceInternalKey);
145​ if (isInternal) {
146​ isExternal = !CFBooleanGetValue(isInternal);
147​ }
148​ }
149​ return isExternal || m_device->property(QLatin1String("Removable")).toBool();
150​}
151​
152​bool IOKitStorage::isHotpluggable() const
153​{
154​ const Solid::StorageDrive::DriveType type = driveType();
155​ return bus() == Solid::StorageDrive::Usb
156​ || type == Solid::StorageDrive::CdromDrive || type == Solid::StorageDrive::SdMmc;
157​}
158​
159​qulonglong IOKitStorage::size() const
160​{
161​ return m_device->property(QLatin1String("Size")).toULongLong();
162​}
163​
164​const QString IOKitStorage::vendor()
165​{
166​ if (d->daDict) {
167​ return QString::fromCFString((const CFStringRef) CFDictionaryGetValue(d->daDict,
168​ kDADiskDescriptionDeviceVendorKey));
169​ }
170​ return QString();
171​}
172​
173​const QString IOKitStorage::product()
174​{
175​ if (d->daDict) {
176​ return QString::fromCFString((const CFStringRef) CFDictionaryGetValue(d->daDict,
177​ kDADiskDescriptionDeviceModelKey));
178​ }
179​ return QString();
180​}
181​
182​const QString IOKitStorage::description()
183​{
184​ if (d->daDict) {
185​ return QString::fromCFString((const CFStringRef) CFDictionaryGetValue(d->daDict,
186​ kDADiskDescriptionMediaNameKey));
187​ }
188​ return QString();
189​}