Differential D21204 Diff 65955 src/solid/devices/backends/fstab/fstabhandling.cpp

Changeset View

Standalone View

View Options

src/solid/devices/backends/fstab/fstabhandling.cpp

Show First 20 Lines • Show All 128 Lines • ▼ Show 20 Line(s)126 if (fstype == "fuse.encfs" ||
129 return true; 129​ return true;
130 } 130​ }
131 return false; 131​ return false;
132} 132​}
133 133​
134QString _k_deviceNameForMountpoint(const QString &source, const QString &fstype, 134​QString _k_deviceNameForMountpoint(const QString &source, const QString &fstype,
135 const QString &mountpoint) 135​ const QString &mountpoint)
136{ 136​{
137 if (fstype.startsWith("fuse.") || 137​ if (fstype.startsWith("fuse.") ||
138 fstype == QLatin1String("overlay")) { 138​ fstype == QLatin1String("overlay")) {
ngrahamUnsubmitted
Done

So this situation could never happen with FUSE mounts? Or should that case be checked too.

ngraham: So this situation could never happen with FUSE mounts? Or should that case be checked too.
mevenAuthorUnsubmitted
Done

Good point, I haven't thought much about this case.

meven: Good point, I haven't thought much about this case.
139 return fstype + mountpoint; 139​ return fstype + mountpoint;
140 } 140​ }
141 return source; 141​ return source;
142} 142​}
brunsUnsubmitted
Done

break @ < 80 chars

bruns: break @ < 80 chars
143 143​
144void Solid::Backends::Fstab::FstabHandling::_k_updateFstabMountPointsCache() 144​void Solid::Backends::Fstab::FstabHandling::_k_updateFstabMountPointsCache()
brunsUnsubmitted
Done

that likely breaks for "server:/" NFS mounts

bruns: that likely breaks for "server:/" NFS mounts
mevenAuthorUnsubmitted
Done

I am not familiar with those mounts.
What is the udi of those mounted filesystem ?

meven: I am not familiar with those mounts. What is the udi of those mounted filesystem ?
brunsUnsubmitted
Done

for NFS the following entries are valid:

  • 192.168.0.1:/some/dir
  • 192.168.0.1:/some/dir/
  • server:/other
  • server:/

while server: (without slash) is invalid.

The first two entries are semantically the same

bruns: for NFS the following entries are valid: - 192.168.0.1:/some/dir - 192.168.0.1:/some/dir/…
mevenAuthorUnsubmitted
Done

What is the udi of of a mounted nfs that has an umounted udi like: server:/ server:/other and server:/other/ ?

meven: What is the udi of of a mounted nfs that has an umounted udi like: server:/ server:/other and…
145{ 145​{
146 if (globalFstabCache->localData().m_fstabCacheValid) { 146​ if (globalFstabCache->localData().m_fstabCacheValid) {
147 return; 147​ return;
148 } 148​ }
149 149​
150 globalFstabCache->localData().m_fstabCache.clear(); 150​ globalFstabCache->localData().m_fstabCache.clear();
151 globalFstabCache->localData().m_fstabOptionsCache.clear(); 151​ globalFstabCache->localData().m_fstabOptionsCache.clear();
152 152​
Show All 9 Lines161 while ((fe = getmntent(fstab)) != nullptr) {
162 const QString fsname = QFile::decodeName(fe->mnt_fsname); 162​ const QString fsname = QFile::decodeName(fe->mnt_fsname);
163 const QString fstype = QFile::decodeName(fe->mnt_type); 163​ const QString fstype = QFile::decodeName(fe->mnt_type);
164 if (_k_isFstabNetworkFileSystem(fstype, fsname) || 164​ if (_k_isFstabNetworkFileSystem(fstype, fsname) ||
165 _k_isFstabSupportedLocalFileSystem(fstype)) { 165​ _k_isFstabSupportedLocalFileSystem(fstype)) {
166 const QString mountpoint = QFile::decodeName(fe->mnt_dir); 166​ const QString mountpoint = QFile::decodeName(fe->mnt_dir);
167 const QString device = _k_deviceNameForMountpoint(fsname, fstype, mountpoint); 167​ const QString device = _k_deviceNameForMountpoint(fsname, fstype, mountpoint);
168 QStringList options = QFile::decodeName(fe->mnt_opts).split(QLatin1Char(',')); 168​ QStringList options = QFile::decodeName(fe->mnt_opts).split(QLatin1Char(','));
169 169​
170 globalFstabCache->localData().m_fstabCache.insert(device, mountpoint); 170​ globalFstabCache->localData().m_fstabCache.insert(device, mountpoint);
brunsUnsubmitted
Done

This is a litlle bit scarce ...

bruns: This is a litlle bit scarce ...
171 globalFstabCache->localData().m_fstabFstypeCache.insert(device, fstype); 171​ globalFstabCache->localData().m_fstabFstypeCache.insert(device, fstype);
apolUnsubmitted
Done

mountpoint.endsWith(QLatin1Char('/'))

apol: mountpoint.endsWith(QLatin1Char('/'))
172 while (!options.isEmpty()) { 172​ while (!options.isEmpty()) {
apolUnsubmitted
Done

you can call mountpoint.chop(2) for the same effect, which should be easier to read.

apol: you can call mountpoint.chop(2) for the same effect, which should be easier to read.
brunsUnsubmitted
Done

and why "2"?

bruns: and why "2"?
173 globalFstabCache->localData().m_fstabOptionsCache.insert(device, options.takeFirst()); 173​ globalFstabCache->localData().m_fstabOptionsCache.insert(device, options.takeFirst());
174 } 174​ }
175 } 175​ }
176 } 176​ }
177 177​
178 endmntent(fstab); 178​ endmntent(fstab);
179 179​
180#else 180​#else
Show All 26 Lines
207#endif 207​#endif
208 //prevent accessing a blocking directory 208​ //prevent accessing a blocking directory
209 if (_k_isFstabNetworkFileSystem(items.at(2), items.at(0)) || 209​ if (_k_isFstabNetworkFileSystem(items.at(2), items.at(0)) ||
210 _k_isFstabSupportedLocalFileSystem(items.at(2))) { 210​ _k_isFstabSupportedLocalFileSystem(items.at(2))) {
211 const QString device = items.at(0); 211​ const QString device = items.at(0);
212 const QString mountpoint = items.at(1); 212​ const QString mountpoint = items.at(1);
213 213​
214 globalFstabCache->localData().m_fstabCache.insert(device, mountpoint); 214​ globalFstabCache->localData().m_fstabCache.insert(device, mountpoint);
215 } 215​ }
apolUnsubmitted
Done

Same as above.

apol: Same as above.
216 } 216​ }
217 217​
218 fstab.close(); 218​ fstab.close();
219#endif 219​#endif
220 globalFstabCache->localData().m_fstabCacheValid = true; 220​ globalFstabCache->localData().m_fstabCacheValid = true;
221} 221​}
222 222​
223QStringList Solid::Backends::Fstab::FstabHandling::deviceList() 223​QStringList Solid::Backends::Fstab::FstabHandling::deviceList()
224{ 224​{
225 _k_updateFstabMountPointsCache(); 225​ _k_updateFstabMountPointsCache();
226 _k_updateMtabMountPointsCache(); 226​ _k_updateMtabMountPointsCache();
227 227​
228 QStringList devices = globalFstabCache->localData().m_fstabCache.keys(); 228​ QStringList devices = globalFstabCache->localData().m_mtabCache.keys();
229 devices += globalFstabCache->localData().m_mtabCache.keys(); 229​
broulikUnsubmitted
Done

Don't create a temporary keys() list just to iterate it. Use a loop like

for (auto it = globalFstabCache->m_fstabCache.constBegin(),
     end = globalFstabCache->m_fstabCache.constEnd();
     it != end; ++it) {
    QString deviceName = it.key();
    ...
}
broulik: Don't create a temporary `keys()` list just to iterate it. Use a loop like ``` for (auto it =…
230 devices.removeDuplicates(); 230​ // Ensure that regardless an fstab device ends with a slash
231​ // it will match its eventual mounted device regardless whether or not its path
232​ // ends with a slash
233​ for (auto it = globalFstabCache->localData().m_fstabCache.constBegin(),
anthonyfieroniUnsubmitted
Done

get it by const ref.

anthonyfieroni: get it by const ref.
234​ end = globalFstabCache->localData().m_fstabCache.constEnd();
235​ it != end; ++it) {
236​
broulikUnsubmitted
Done

Excess parentheses

broulik: Excess parentheses
237​ auto device = it.key();
238​ if (devices.contains(device)) {
anthonyfieroniUnsubmitted
Done
deviceName.append('/');
anthonyfieroni: ``` deviceName.append('/'); ```
brunsUnsubmitted
Done

You should check for devices.contains(deviceName) here and continue, avoids detaching deviceName when not necessary.

bruns: You should check for `devices.contains(deviceName)` here and `continue`, avoids detaching…
239​ continue;
240​ } else {
brunsUnsubmitted
Done

No need to put this in .. else { after continue

bruns: No need to put this in `.. else {` after `continue`
241​ // deviceName will or won't end with / depending if device ended with one
242​ QString deviceName = device;
243​ if (deviceName.endsWith(QLatin1Char('/'))) {
244​ // remove trailing slash
245​ deviceName.chop(1);
246​ } else {
brunsUnsubmitted
Not Done

This comment is stating the obvious ...

bruns: This comment is stating the obvious ...
247​ // add a trailing slash
248​ deviceName.append(QLatin1Char('/'));
249​ }
brunsUnsubmitted
Not Done

dito

bruns: dito
250​ if (!devices.contains(deviceName)) {
251​ devices.append(device);
252​ }
253​ }
254​ }
231 return devices; 255​ return devices;
232} 256​}
233 257​
234QStringList Solid::Backends::Fstab::FstabHandling::mountPoints(const QString &device) 258​QStringList Solid::Backends::Fstab::FstabHandling::mountPoints(const QString &device)
235{ 259​{
236 _k_updateFstabMountPointsCache(); 260​ _k_updateFstabMountPointsCache();
237 _k_updateMtabMountPointsCache(); 261​ _k_updateMtabMountPointsCache();
238 262​
▲ Show 20 Lines • Show All 117 Lines • Show Last 20 Lines