D4847: KAuth integration in document saving

[David Faure]

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