dfaure added a comment.
Right the only way to get atomic renaming is to write the tempfile next to
its final destination, NOT in /tmp.
(just like QSaveFile does ;)
INLINE COMMENTS
> katesecuretextbuffer.cpp:74
> + /**
> + * There is no atomic rename operation publicly exposed by QT.
> + *
It's Qt, not QT.
QT is QuickTime ;)
> katesecuretextbuffer.cpp:88
> + char buffer[len];
> + QDataStream dataStreamFrom(&readFile);
> + QDataStream dataStreamTo(&saveFile);
You definitely don't need QDataStream just to call read and write on QIODevices.
> katesecuretextbuffer.cpp:101
> +#ifndef Q_OS_WIN
> + // ensure that the file is written to disk
> +#ifdef HAVE_FDATASYNC
Not necessary if you use QSaveFile, which does this already.
But maybe the best solution here (to avoid the file copy) is a temp file that
you can atomically rename, so let's discuss that before you get rid of all the
stuff-that-QSaveFile-does ;)
> katetextbuffer.cpp:807
> +
> +#ifndef Q_OS_WIN
> + if (newFile) {
The thing about using Qt's setPermissions() is that it should work on Windows
too.
> katetextbuffer.cpp:815
> + // set original file's permissions
> + saveFile->setPermissions(QFile(filename).permissions());
> }
Doesn't QSaveFile do this?
> katetextbuffer.cpp:885
> #ifdef HAVE_FDATASYNC
> - fdatasync(saveFile.handle());
> + fdatasync(saveFile->handle());
> #else
(pre-existing) done by QSaveFile already
> katetextbuffer.cpp:904
> + QVariantMap args;
> + args[QLatin1String("sourceFile")] = saveFile->fileName();
> + args[QLatin1String("targetFile")] = filename;
use .insert() to avoid the creation of a temporary.
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D4847
To: martinkostolny, dhaumann, #ktexteditor
Cc: dfaure, anthonyfieroni, cullmann, ltoscano, dhaumann, graesslin,
davidedmundson, palant, kwrite-devel, #frameworks, head7, kfunk, sars
