D27152: Introduce FilesystemEntry class

[Stefan Brüns]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200303/c061fb6f/attachment.html>