D21204: Ensure no trailing slash in mountpoint read from fstab file

ngraham retitled this revision from "Ensure no trailing slash in mountpoint read from fstab file." to "Ensure no trailing slash in mountpoint read from fstab file". REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D21204 To: meven, bruns, #frameworks Cc: dhaumann,

D21204: Ensure no trailing slash in mountpoint read from fstab file.

ngraham added a comment. In D21204#488719 , @dhaumann wrote: > Minor/general comment: given this is a KDE Frameworks change, could you improve the summary of the commit message for this and future commits? Here it just says bug, ccbug, fixedin.

D21204: Ensure no trailing slash in mountpoint read from fstab file.

dhaumann added a comment. Minor/general comment: given this is a KDE Frameworks change, could you improve the summary of the commit message for this and future commits? Here it just says bug, ccbug, fixedin. Imho a commit log should be self-explaining and self-contained: what is broken

D21204: Ensure no trailing slash in mountpoint read from fstab file.

meven marked 2 inline comments as done. meven added a comment. Thanks @anthonyfieroni REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D21204 To: meven, bruns, #frameworks Cc: anthonyfieroni, ngraham, bruns, apol, kde-frameworks-devel, LeGast00n, michaelh

D21204: Ensure no trailing slash in mountpoint read from fstab file.

meven updated this revision to Diff 60855. meven added a comment. Use a const ref in the loop use append to add a trailing / REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21204?vs=58976=60855 BRANCH arcpatch-D21204 REVISION DETAIL

D21204: Ensure no trailing slash in mountpoint read from fstab file.

anthonyfieroni added inline comments. INLINE COMMENTS > fstabhandling.cpp:231 > +// it will match its eventual mounted device regardless whether or not > it ends with a slash > +for (QString device : fstabDevices) { > +QString deviceName = device; get it by const ref. >

D21204: Ensure no trailing slash in mountpoint read from fstab file.

meven added a comment. ping @bruns REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D21204 To: meven, bruns, #frameworks Cc: ngraham, bruns, apol, kde-frameworks-devel, LeGast00n, michaelh

D21204: Ensure no trailing slash in mountpoint read from fstab file.

meven added a comment. How do you like this alternative @bruns ? The upside is that is fixes the bug and does not change stored data and keeps compatibility. The downside is that it is a little hackish. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D21204 To:

D21204: Ensure no trailing slash in mountpoint read from fstab file.

meven added a comment. I have tested the new patch. It is a bit a workaround but does not edit any udi. What do you think ? REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D21204 To: meven, bruns, #frameworks Cc: ngraham, bruns, apol, kde-frameworks-devel,

D21204: Ensure no trailing slash in mountpoint read from fstab file.

meven edited the summary of this revision. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D21204 To: meven, bruns, #frameworks Cc: ngraham, bruns, apol, kde-frameworks-devel, michaelh

D21204: Ensure no trailing slash in mountpoint read from fstab file.

meven updated this revision to Diff 58976. meven added a comment. Tweak FstabHandling::deviceList instead of editing device udi REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21204?vs=58237=58976 BRANCH D21204 REVISION DETAIL

D21204: Ensure no trailing slash in mountpoint read from fstab file.

meven added inline comments. INLINE COMMENTS > bruns wrote in fstabhandling.cpp:142 > 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

D21204: Ensure no trailing slash in mountpoint read from fstab file.

bruns added inline comments. INLINE COMMENTS > meven wrote in fstabhandling.cpp:142 > I am not familiar with those mounts. > What is the udi of those mounted filesystem ? for NFS the following entries are valid: - 192.168.0.1:/some/dir - 192.168.0.1:/some/dir/ - server:/other - server:/ while

D21204: Ensure no trailing slash in mountpoint read from fstab file.

meven marked 2 inline comments as done. meven added inline comments. INLINE COMMENTS > ngraham wrote in fstabhandling.cpp:136 > So this situation could never happen with FUSE mounts? Or should that case be > checked too. Good point, I haven't thought much about this case. > bruns wrote in

D21204: Ensure no trailing slash in mountpoint read from fstab file.

meven marked 2 inline comments as done. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D21204 To: meven, bruns, #frameworks Cc: ngraham, bruns, apol, kde-frameworks-devel, michaelh

D21204: Ensure no trailing slash in mountpoint read from fstab file.

meven updated this revision to Diff 58237. meven added a comment. Shorten comment line, ensure fuse mount do not end with / either REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21204?vs=58190=58237 BRANCH arcpatch-D21204 REVISION DETAIL

D21204: Ensure no trailing slash in mountpoint read from fstab file.

bruns added inline comments. INLINE COMMENTS > fstabhandling.cpp:140 > + > +// strips last slash so that the device matches the one returned by > getmntent when filesystem is mounted > +// even when fstab mnt_dir ends with a '/' in /etc/fstab break @ < 80 chars > fstabhandling.cpp:142

D21204: Ensure no trailing slash in mountpoint read from fstab file.

ngraham added inline comments. INLINE COMMENTS > fstabhandling.cpp:136 > { > if (fstype.startsWith("fuse.")) { > return fstype + mountpoint; So this situation could never happen with FUSE mounts? Or should that case be checked too. REPOSITORY R245 Solid REVISION DETAIL

D21204: Ensure no trailing slash in mountpoint read from fstab file.

meven updated this revision to Diff 58190. meven added a comment. Move code change to _k_deviceNameForMountpoint REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21204?vs=58189=58190 BRANCH arcpatch-D21204 REVISION DETAIL

D21204: Ensure no trailing slash in mountpoint read from fstab file.

meven marked 2 inline comments as done. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D21204 To: meven, bruns, #frameworks Cc: bruns, apol, kde-frameworks-devel, michaelh, ngraham

D21204: Ensure no trailing slash in mountpoint read from fstab file.

meven updated this revision to Diff 58189. meven marked 3 inline comments as done. meven added a comment. use chop to strip last / REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21204?vs=58188=58189 BRANCH arcpatch-D21204 REVISION DETAIL

D21204: Ensure no trailing slash in mountpoint read from fstab file.

meven updated this revision to Diff 58188. meven added a comment. Fix code, improved doc, better testing REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21204?vs=58048=58188 BRANCH arcpatch-D21204 REVISION DETAIL https://phabricator.kde.org/D21204

D21204: Ensure no trailing slash in mountpoint read from fstab file.

ngraham added reviewers: bruns, Frameworks. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D21204 To: meven, bruns, #frameworks Cc: bruns, apol, kde-frameworks-devel, michaelh, ngraham

D21204: Ensure no trailing slash in mountpoint read from fstab file.

bruns added inline comments. INLINE COMMENTS > fstabhandling.cpp:168 > > +// strips last slash > +if (mountpoint.at(mountpoint.length() -1) == '/') { This is a litlle bit scarce ... > apol wrote in fstabhandling.cpp:170 > you can call mountpoint.chop(2) for the same

D21204: Ensure no trailing slash in mountpoint read from fstab file.

apol added inline comments. INLINE COMMENTS > fstabhandling.cpp:169 > +// strips last slash > +if (mountpoint.at(mountpoint.length() -1) == '/') { > +mountpoint = mountpoint.left(mountpoint.length() - 2); mountpoint.endsWith(QLatin1Char('/')) >

D21204: Ensure no trailing slash in mountpoint read from fstab file.

meven created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. meven requested review of this revision. REVISION SUMMARY BUG: 406242 CCBUG: 390691 FIXED-IN: 5.59 REPOSITORY R245 Solid BRANCH master REVISION DETAIL