D6832: Integrate new file ioslave in KIO job

[David Faure]

dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  Looks good, apart from a few minor things.

INLINE COMMENTS

> job.cpp:276
> +            // Whether or not sub-jobs have PrivilegeExecution flag set doesn't matter.
> +            // If its not set in parent job then don't continue.
> +            return false;

its -> it's

> job_p.h:197
> +     *
> +     * @since 5.37
> +     */

Private header -> no need for @since doc, it's not part of the API.

> simplejob.cpp:346
> +    bool confirmed = tryAskPrivilegeOpConfirmation();
> +    m_slave->send(MSG_PRIVILEGE_EXEC, QByteArray::number(confirmed));
> +}

I'm not sure if the bool->int conversion is safe and guaranteed (AFAIK, in theory a bool can convert to 2 or anything else that is not 0, depending on how it was set in the first place)

This would be safer if it said QByteArray::number(confirmed ? 1 : 0), but then that's even better written as confirmed ? "1" : "0", no need to call int->string conversion code;

REVISION DETAIL
  https://phabricator.kde.org/D6832

To: chinmoyr, dfaure, #frameworks
Cc: #frameworks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170727/0b8d6a12/attachment.html>