D6833: Add support for PrivilegeExecution in KIO jobs

David Faure noreply at phabricator.kde.org
Thu Jul 27 07:46:47 UTC 2017


dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> chinmoyr wrote in copyjob.cpp:1674
> Ideally it should be set when m_privilegeExecutionEnabled is true. But it will add more lines. Even though its just 2-3 lines, it doesn't look good. 
> Besides the flag is ineffective if the parent job doesn't have this flag set. Shall I remove it?

To me it looks worse to unconditionally pass a security-critical flag when it shouldn't be set, it reads like a security hole (even if, as you say, upon further research it turns out that the flag has no effect).

Maybe make it a helper method flagsForSubJob() to avoid any duplication.

BTW isn't the flag missing for the KIO::symlink case above?

> copyjob.cpp:294
> +                break;
> +            default:
> +                break;

Remove default statement, which would leave the variable uninitialized. This way the compiler will warn if a new value is added to the CopyMode enum.

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

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


More information about the Kde-frameworks-devel mailing list