bruns requested changes to this revision.
bruns added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> filesystem_entry.cpp:77
> +{
> + return m_type == FilesystemType::Unknown ? m_typeString :
> FilesystemTypeToString(m_type);
> +}
Why don't you just keep the string?
> fstabdevice.cpp:41
> m_device.remove(parentUdi() + "/");
> + const FilesystemEntry* fsEntry =
> FstabHandling::filesystemEntry(m_device);
> + if (fsEntry == nullptr) {
Ircs, raw pointers ... you newer know who owns them ... It is a pointer to an
entry in a QHash, it may be deleted/moved when you don't expect it.
Unfortunately, std::optional is C++17 only.
Return it by value, return a default constructed instance if no found, and
compare `fsEntry.device() != m_device`
> fstabhandling.cpp:264
> + }
> + return fstabEntry->mountOptions();
> + }
this looks wrong ...
> fstabhandling.cpp:316
> + if (fstabEntry != globalFstabCache->localData().m_fstabCache.end()) {
> + return &(*fstabEntry);
> + }
this is bad, bad, bad, ...
REPOSITORY
R245 Solid
REVISION DETAIL
https://phabricator.kde.org/D27152
To: hallas, #frameworks, bruns, meven
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
