dhaumann added a comment.
Just some minor comments, besides that, looks sane. INLINE COMMENTS > fstabdevice.cpp:40-41 > + if (m_device.startsWith(QLatin1String("//"))) { > + m_vendor = m_device.mid(2, m_device.indexOf(QLatin1String("/"), 2) - > 2); > + m_product = m_device.mid(m_device.indexOf(QLatin1String("/", 2)) + > 1); > } else { Did you intentionally switch vendor and product? > fstabdevice.cpp:51 > + if (option.startsWith(QLatin1String("x-gvfs-name="))) { > + const QString encoded = > option.mid(option.indexOf(QLatin1String("="), 11) + 1); > + m_description = QUrl::fromPercentEncoding(encoded.toLatin1()); QStringRef through midRef() > fstabdevice.cpp:54 > + } else if (option.startsWith(QLatin1String("x-gvfs-icon="))) { > + const QString encoded = > option.mid(option.indexOf(QLatin1String("="), 11) + 1); > + m_iconName = QUrl::fromPercentEncoding(encoded.toLatin1()); likewise, use QStringRef > fstabhandling.cpp:146 > const QString mountpoint = QFile::decodeName(fe->mnt_dir); > + const QStringList options = > QFile::decodeName(fe->mnt_opts).split(QLatin1Char(','), > QString::SkipEmptyParts); > If you save the decodedName, you could also call splitRef() to avoid more allocations. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D5034 To: broulik, #plasma, dfaure Cc: dhaumann, plasma-devel, #frameworks, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol