ahmadsamir added inline comments.
INLINE COMMENTS
> dfaure wrote in file_unix.cpp:1052
> I'm confused. We want to solve renaming 'a' to 'A' when 'A' does *not* exist.
>
> QFile::rename will not overwrite an existing file, so it will do nothing if
> dest exists.
const QByteArray dest1 = "/mnt/fat32/A";
const QByteArray dest2 = "/mnt/fat32/a";
QT_STATBUF buff_dest;
qDebug() << QT_LSTAT(dest1, &buff_dest);
qDebug() << QT_LSTAT(dest2, &buff_dest);
IIUC, from FAT32 POV, 'A' exists if 'a' exists and vice versa.
> dfaure wrote in file_unix.cpp:1074
> Wouldn't it be enough to just call QFile::rename here?
>
> The whole idea is: if QFile::rename is able to rename a file in all cases,
> including the a->A special case on FAT, then let's just delegate the renaming
> to QFile.
>
> Then we don't need to have any special case in our code.
>
> QFile::rename does not overwrite, though, so if the dest exists and the
> Overwrite flag is set, we might have to either delete the dest first (race
> condition, not sure it matters here), or in *that* case use ::rename() since
> we know it can't be a FAT32-case-change (FAT32 can't have both a and A).
Too many quirks if we don't use ::rename(). I tested with e.g. in file_unix.cpp:
bool renamed = QFile::rename(src, dest);
if (!renamed) {
if ((_flags & KIO::Overwrite) || isSrcSymLink) {
renamed = ::rename(_src.data(), _dest.data()) == 0;
}
}
- QFile::rename doesn't overwrite as you said
- renaming symlinks depends on ::rename
- some unit tests are failing due to permissions
FAIL! : JobTest::statDetailsBasic() Compared values are not the same
Actual (kioItem.permissions()): 420
Expected (438) : 438
Loc: [/home/ahmad/dev/kio/autotests/jobtest.cpp(1464)]
INFO : JobTest::statDetailsBasicSetDetails() entering
FAIL! : JobTest::statDetailsBasicSetDetails() Compared values are not the
same
Actual (kioItem.permissions()): 420
Expected (438) : 438
Loc: [/home/ahmad/dev/kio/autotests/jobtest.cpp(1503)]
That and ::rename() is used all over the place in file_unix.cpp, I am not that
comfortable using QFile::rename except for the freaky FAT32 case :)
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D29610
To: ahmadsamir, #frameworks, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
