Repair copying file to VFAT without warnings.
ClosedPublic

Authored by dfaure on Feb 7 2018, 9:35 AM.

Details

Summary

The refactoring returned the wrong value, which led to warnings,
but it was also in the wrong place. We want KIO::chown() to fail
explicitly, while we want chown during a file copy to be ignored
on e.g. VFAT systems.

Test Plan

Copying a file to a VFAT partition no longer warns/errors.

Diff Detail

Branch
silence_vfat_warnings
Lint
No Linters Available
Unit
No Unit Test Coverage
dfaure created this revision.Feb 7 2018, 9:35 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptFeb 7 2018, 9:35 AM
dfaure requested review of this revision.Feb 7 2018, 9:35 AM
jtamate requested changes to this revision.Feb 7 2018, 11:22 AM

Copying to a VFAT has no problems.
Moving to a VFAT has the following problem:
An Access Denied Information appears after the file have been created.
In both cases, there is no data loss.

In ntfs, as it is said that it supports chown (if you have the rights to do it), this patch doesn't fix the data loss.

This revision now requires changes to proceed.Feb 7 2018, 11:22 AM

OK, this shows that we need to fix a bigger issue before silencing the warnings on VFAT.

But I just noticed something. The instructions you posted to kde-frameworks-devel for reproducing the bug are somewhat broken.

sudo mount -t vfat -o uid=0000,fmask=0007,dmask=0000,rw,noexec,loop fat.fs /mnt/vfat

leads to files that don't have "owner" permission, so one can't read the files he creates.

$ echo a > /mnt/vfat/myfile
zsh: permission denied: myfile
$ cd /mnt/vfat ; touch foo ; ls -l
-rwxrwx--- 1 root root     0  7 févr. 23:05 foo
$ cat foo
cat: foo: Permission denied

Something like fmask=0000 or fmask=0111 (if no file should be executable) seems much more sensible, so that I can edit my own files.
The bug is still there though, I'll look into it.

Proper fix for the data loss in https://phabricator.kde.org/D10376.

I'll rebase this one on top of it, to keep the silencing of the other warnings over vfat (other than the "nobody shall be allowed to peek" code).

dfaure updated this revision to Diff 26728.Feb 7 2018, 10:26 PM

rebased

jtamate accepted this revision.Feb 8 2018, 8:00 AM

Even moving to a ntfs filesystem without the rights to change owner doesn't end in an empty file.
Very good job.

This revision is now accepted and ready to land.Feb 8 2018, 8:00 AM
dfaure closed this revision.Feb 8 2018, 4:30 PM